Skip to content

Commit

Permalink
fix: Environments metadata n+1 for project admin (#3101)
Browse files Browse the repository at this point in the history
  • Loading branch information
zachaysan committed Dec 6, 2023
1 parent c1c45cb commit 093fa3a
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
5 changes: 4 additions & 1 deletion api/permissions/permission_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ def get_permitted_environments_for_user(
"""

if is_user_project_admin(user, project):
return project.environments.all()
queryset = project.environments.all()
if prefetch_metadata:
return queryset.prefetch_related("metadata")
return queryset

base_filter = get_base_permission_filter(
user, Environment, permission_key, tag_ids=tag_ids
Expand Down
42 changes: 41 additions & 1 deletion api/tests/unit/environments/test_unit_environments_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,8 @@ def test_view_environment_with_staff__query_count_is_expected(

with_environment_permissions([VIEW_ENVIRONMENT], environment_id=environment_2.id)

# One additional query for an unrelated, unfixable N+1 issue.
# One additional query for an unrelated, unfixable N+1 issue that deals with
# the defer logic around filtered environments.
expected_query_count += 1

# Then
Expand All @@ -649,6 +650,45 @@ def test_view_environment_with_staff__query_count_is_expected(
assert response.status_code == status.HTTP_200_OK


def test_view_environment_with_admin__query_count_is_expected(
admin_client: APIClient,
environment: Environment,
project: Project,
django_assert_num_queries: Callable[[int], None],
environment_metadata_a: Metadata,
environment_metadata_b: Metadata,
required_a_environment_metadata_field: MetadataModelField,
environment_content_type: ContentType,
) -> None:
# Given
url = reverse("api-v1:environments:environment-list")
data = {"project": project.id}

expected_query_count = 5
# When
with django_assert_num_queries(expected_query_count):
response = admin_client.get(url, data=data, content_type="application/json")

assert response.status_code == status.HTTP_200_OK

# Add an environment to make sure the query count is the same.
environment_2 = Environment.objects.create(
name="Second Environment", project=project
)
Metadata.objects.create(
object_id=environment_2.id,
content_type=environment_content_type,
model_field=required_a_environment_metadata_field,
field_value="10",
)

# Then
with django_assert_num_queries(expected_query_count):
response = admin_client.get(url, data=data, content_type="application/json")

assert response.status_code == status.HTTP_200_OK


@pytest.mark.parametrize(
"client",
[lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],
Expand Down

0 comments on commit 093fa3a

Please sign in to comment.