-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(releases): Fix environment filtering for per-project new groups count in old releases serializer #101003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(releases): Fix environment filtering for per-project new groups count in old releases serializer #101003
Changes from all commits
5180085
d7ac732
94d3251
c2e43c7
d10c805
05eef4b
a4add81
b43f15b
42ac977
292de0f
47f3496
2ab71c9
bca93ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -515,9 +515,11 @@ def __get_release_data_with_environments(self, release_project_envs): | |
last_seen[release_project_env.release.version] = release_project_env.last_seen | ||
|
||
group_counts_by_release: dict[int, dict[int, int]] = {} | ||
for project_id, release_id, new_groups in release_project_envs.annotate( | ||
aggregated_new_issues_count=Sum("new_issues_count") | ||
).values_list("project_id", "release_id", "aggregated_new_issues_count"): | ||
for project_id, release_id, new_groups in ( | ||
release_project_envs.values("project_id", "release_id") | ||
.annotate(aggregated_new_issues_count=Sum("new_issues_count")) | ||
.values_list("project_id", "release_id", "aggregated_new_issues_count") | ||
): | ||
group_counts_by_release.setdefault(release_id, {})[project_id] = new_groups | ||
|
||
return first_seen, last_seen, group_counts_by_release | ||
|
@@ -535,6 +537,18 @@ def _get_release_project_envs(self, item_list, environments, project): | |
|
||
return release_project_envs | ||
|
||
def _get_release_project_envs_unordered(self, item_list, environments, project): | ||
release_project_envs = ReleaseProjectEnvironment.objects.filter( | ||
release__in=item_list | ||
).select_related("release") | ||
|
||
if environments is not None: | ||
release_project_envs = release_project_envs.filter(environment__name__in=environments) | ||
if project is not None: | ||
release_project_envs = release_project_envs.filter(project=project) | ||
|
||
return release_project_envs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not just accessing project id? We don't access any other attributes of release_project_envs.project. |
||
|
||
def get_attrs(self, item_list, user, **kwargs): | ||
project = kwargs.get("project") | ||
|
||
|
@@ -558,7 +572,6 @@ def get_attrs(self, item_list, user, **kwargs): | |
raise TypeError("health data requires snuba") | ||
|
||
adoption_stages = {} | ||
release_project_envs = None | ||
if self.with_adoption_stages: | ||
release_project_envs = self._get_release_project_envs(item_list, environments, project) | ||
adoption_stages = self._get_release_adoption_stages(release_project_envs) | ||
|
@@ -568,10 +581,9 @@ def get_attrs(self, item_list, user, **kwargs): | |
project, item_list, no_snuba_for_release_creation | ||
) | ||
else: | ||
if release_project_envs is None: | ||
release_project_envs = self._get_release_project_envs( | ||
item_list, environments, project | ||
) | ||
release_project_envs = self._get_release_project_envs_unordered( | ||
item_list, environments, project | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Redundant Query Overwrites Ordered DataWhen |
||
( | ||
first_seen, | ||
last_seen, | ||
|
@@ -626,11 +638,21 @@ def get_attrs(self, item_list, user, **kwargs): | |
has_health_data = {} | ||
|
||
for pr in project_releases: | ||
# Use environment-filtered new groups count if available, otherwise fall back to ReleaseProject data | ||
new_groups_count_env_filtered = issue_counts_by_release.get(pr["release_id"], {}).get( | ||
pr["project__id"] | ||
) | ||
new_groups_count = ( | ||
new_groups_count_env_filtered | ||
if new_groups_count_env_filtered is not None | ||
else pr["new_groups"] | ||
) | ||
|
||
pr_rv: _ProjectDict = { | ||
"id": pr["project__id"], | ||
"slug": pr["project__slug"], | ||
"name": pr["project__name"], | ||
"new_groups": pr["new_groups"], | ||
"new_groups": new_groups_count, | ||
"platform": pr["project__platform"], | ||
"platforms": platforms_by_project.get(pr["project__id"]) or [], | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we joining the release model? This call to
select_related
seems unnecessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We access release_project_env.release.version in other parts of __get_release_data_with_environments, for example:
sentry/src/sentry/api/serializers/models/release.py
Line 507 in 6f7b903