Skip to content

Commit

Permalink
Avoid unnecessary joins to filter entities by organization IDs
Browse files Browse the repository at this point in the history
  • Loading branch information
marcospri committed Aug 8, 2024
1 parent 012f998 commit 08c8913
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 12 deletions.
4 changes: 1 addition & 3 deletions lms/services/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
Course,
Grouping,
LTIRole,
Organization,
RoleScope,
RoleType,
User,
Expand Down Expand Up @@ -267,8 +266,7 @@ def get_assignments( # noqa: PLR0913
select(Assignment.id)
.join(Course)
.join(ApplicationInstance)
.join(Organization)
.where(Organization.id.in_(admin_organization_ids))
.where(ApplicationInstance.organization_id.in_(admin_organization_ids))
)
# instructor_h_userid and admin_organization_ids are about access rather than filtering.
# we apply them both as an or to fetch assignments where the users is either an instructor or an admin
Expand Down
6 changes: 3 additions & 3 deletions lms/services/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ def get_courses( # noqa: PLR0913
)
if admin_organization_ids:
admin_organization_ids_clause = Course.application_instance_id.in_(
select(ApplicationInstance.id)
.join(Organization)
.where(Organization.id.in_(admin_organization_ids))
select(ApplicationInstance.id).where(
ApplicationInstance.organization_id.in_(admin_organization_ids)
)
)
# instructor_h_userid and admin_organization_ids are about access rather than filtering.
# we apply them both as an or to fetch courses where the users is either an instructor or an admin
Expand Down
10 changes: 4 additions & 6 deletions lms/services/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
AssignmentMembership,
LTIRole,
LTIUser,
Organization,
RoleScope,
RoleType,
User,
Expand Down Expand Up @@ -152,11 +151,10 @@ def get_users( # noqa: PLR0913, PLR0917
)

if admin_organization_ids:
admin_organization_ids_clause = User.id.in_(
select(User.id)
.join(ApplicationInstance)
.join(Organization)
.where(Organization.id.in_(admin_organization_ids))
admin_organization_ids_clause = User.application_instance_id.in_(
select(ApplicationInstance.id).where(
ApplicationInstance.organization_id.in_(admin_organization_ids)
)
)

# instructor_h_userid and admin_organization_ids are about access rather than filtering.
Expand Down

0 comments on commit 08c8913

Please sign in to comment.