Skip to content

Commit

Permalink
fix: iterate over prefetched taxonomyorgs instead of filtering
Browse files Browse the repository at this point in the history
to avoid re-querying.

Changes TaxonomyOrg.get_organizations to return the "all orgs" flag too,
which makes it useful in the rules.
  • Loading branch information
pomegranited committed Feb 7, 2024
1 parent af6e41d commit 286230a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 33 deletions.
27 changes: 16 additions & 11 deletions openedx/core/djangoapps/content_tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,24 @@ def get_relationships(

@classmethod
def get_organizations(
cls, taxonomy: Taxonomy, rel_type: RelType
) -> list[Organization]:
cls, taxonomy: Taxonomy, rel_type=RelType.OWNER,
) -> tuple[bool, list[Organization]]:
"""
Returns the list of Organizations which have the given relationship to the taxonomy.
Returns a tuple containing:
* bool: flag indicating whether "all organizations" have the given relationship to the taxonomy
* orgs: list of Organizations which have the given relationship to the taxonomy
"""
rels = cls.objects.filter(
taxonomy=taxonomy,
rel_type=rel_type,
)
# A relationship with org=None means all Organizations
if rels.filter(org=None).exists():
return list(Organization.objects.all())
return [rel.org for rel in rels]
is_all_org = False
orgs = []
# Iterate over the taxonomyorgs instead of filtering to take advantage of prefetched data.
for taxonomy_org in taxonomy.taxonomyorg_set.all():
if taxonomy_org.rel_type == rel_type:
if taxonomy_org.org is None:
is_all_org = True
else:
orgs.append(taxonomy_org.org)

return (is_all_org, orgs)


class ContentObjectTag(ObjectTag):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ def get_all_orgs(self, obj) -> bool:
"""
Return True if the taxonomy is associated with all orgs.
"""
return obj.taxonomyorg_set.filter(org__isnull=True).exists()
is_all_orgs = False
for taxonomy_org in obj.taxonomyorg_set.all():
if taxonomy_org.org_id is None:
return True
return False

class Meta:
model = TaxonomySerializer.Meta.model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ def test_list_taxonomy_query_count(self):
url = TAXONOMY_ORG_LIST_URL + f'?org={self.orgA.short_name}&enabled=true'

self.client.force_authenticate(user=self.staff)
with self.assertNumQueries(15): # TODO Why so many queries?
with self.assertNumQueries(11):
response = self.client.get(url)

assert response.status_code == 200
Expand Down
22 changes: 2 additions & 20 deletions openedx/core/djangoapps/content_tagging/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,12 @@ def can_view_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool:
if oel_tagging.is_taxonomy_admin(user):
return True

is_all_org = TaxonomyOrg.objects.filter(
taxonomy=taxonomy,
org=None,
rel_type=TaxonomyOrg.RelType.OWNER,
).exists()
is_all_org, taxonomy_orgs = TaxonomyOrg.get_organizations(taxonomy)

# Enabled all-org taxonomies can be viewed by any registred user
if is_all_org:
return taxonomy.enabled

taxonomy_orgs = TaxonomyOrg.get_organizations(
taxonomy=taxonomy,
rel_type=TaxonomyOrg.RelType.OWNER,
)

# Org-level staff can view any taxonomy that is associated with one of their orgs.
if is_org_admin(user, taxonomy_orgs):
return True
Expand Down Expand Up @@ -191,21 +182,12 @@ def can_change_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool:
if oel_tagging.is_taxonomy_admin(user):
return True

is_all_org = TaxonomyOrg.objects.filter(
taxonomy=taxonomy,
org=None,
rel_type=TaxonomyOrg.RelType.OWNER,
).exists()
is_all_org, taxonomy_orgs = TaxonomyOrg.get_organizations(taxonomy)

# Only taxonomy admins can edit all org taxonomies
if is_all_org:
return False

taxonomy_orgs = TaxonomyOrg.get_organizations(
taxonomy=taxonomy,
rel_type=TaxonomyOrg.RelType.OWNER,
)

# Org-level staff can edit any taxonomy that is associated with one of their orgs.
if is_org_admin(user, taxonomy_orgs):
return True
Expand Down

0 comments on commit 286230a

Please sign in to comment.