From ce98be05f0d547592a7f75cdeb27bef2433b969b Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Thu, 21 Nov 2024 07:24:36 -0500 Subject: [PATCH] ref(releases): Function to handle resolved in release (#80983) This is the largest code block in update_groups. This moves all related logic into a single function. --- src/sentry/api/helpers/group_index/update.py | 628 ++++++++++--------- src/sentry/issues/status_change.py | 9 +- 2 files changed, 319 insertions(+), 318 deletions(-) diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index 94651042b5a7fc..e31dcb140138bd 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -216,13 +216,7 @@ def update_groups( project_lookup = {p.id: p for p in projects} acting_user = user if user.is_authenticated else None - self_assign_issue = "0" - if acting_user: - user_options = user_option_service.get_many( - filter={"user_ids": [acting_user.id], "keys": ["self_assign_issue"]} - ) - if user_options: - self_assign_issue = user_options[0].value + if search_fn and not group_ids: try: cursor_result, _ = search_fn( @@ -237,8 +231,6 @@ def update_groups( group_list = list(cursor_result) group_ids = [g.id for g in group_list] - is_bulk = len(group_ids) > 1 - group_project_ids = {g.project_id for g in group_list} # filter projects down to only those that have groups in the search results projects = [p for p in projects if p.id in group_project_ids] @@ -251,11 +243,7 @@ def update_groups( status_details = result.pop("statusDetails", result) status = result.get("status") - release = None - commit = None res_type = None - activity_type = None - activity_data: MutableMapping[str, Any | None] | None = None if "priority" in result: handle_priority( priority=result["priority"], @@ -264,329 +252,349 @@ def update_groups( project_lookup=project_lookup, ) if status in ("resolved", "resolvedInNextRelease"): - res_status = None - if status == "resolvedInNextRelease" or status_details.get("inNextRelease"): - # TODO(jess): We may want to support this for multi project, but punting on it for now - if len(projects) > 1: - return Response( - {"detail": "Cannot set resolved in next release for multiple projects."}, - status=400, - ) - # may not be a release yet - release = status_details.get("inNextRelease") or get_release_to_resolve_by(projects[0]) + result, res_type = handle_resolve_in_release( + status, + status_details, + group_list, + projects, + project_lookup, + acting_user, + user, + result, + ) + if isinstance(result, Response): + return result + elif status: + result = handle_other_status_updates( + result, + group_list, + group_ids, + projects, + project_lookup, + status_details, + acting_user, + user, + ) + + return prepare_response( + result, + group_list, + group_ids, + project_lookup, + projects, + acting_user, + data, + res_type, + request.META.get("HTTP_REFERER", ""), + ) - activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value - activity_data = { - # no version yet - "version": "" - } - serialized_user = user_service.serialize_many( - filter=dict(user_ids=[user.id]), as_user=user +def handle_resolve_in_release( + status: str, + status_details: Mapping[str, Any], + group_list: Sequence[Group], + projects: Sequence[Project], + project_lookup: Mapping[int, Project], + acting_user: User | None, + user: User | RpcUser, + result: MutableMapping[str, Any], +) -> tuple[dict[str, Any], GroupResolution.Type | None] | Response: + res_type = None + release = None + commit = None + self_assign_issue = "0" + if acting_user: + user_options = user_option_service.get_many( + filter={"user_ids": [acting_user.id], "keys": ["self_assign_issue"]} + ) + if user_options: + self_assign_issue = user_options[0].value + res_status = None + if status == "resolvedInNextRelease" or status_details.get("inNextRelease"): + # TODO(jess): We may want to support this for multi project, but punting on it for now + if len(projects) > 1: + return Response( + {"detail": "Cannot set resolved in next release for multiple projects."}, + status=400, ) - new_status_details = { - "inNextRelease": True, - } - if serialized_user: - new_status_details["actor"] = serialized_user[0] - res_type = GroupResolution.Type.in_next_release - res_type_str = "in_next_release" - res_status = GroupResolution.Status.pending - elif status_details.get("inUpcomingRelease"): - if len(projects) > 1: - return Response( - {"detail": "Cannot set resolved in upcoming release for multiple projects."}, - status=400, - ) - release = status_details.get("inUpcomingRelease") or most_recent_release(projects[0]) - activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value - activity_data = {"version": ""} - - serialized_user = user_service.serialize_many( - filter=dict(user_ids=[user.id]), as_user=user + # may not be a release yet + release = status_details.get("inNextRelease") or get_release_to_resolve_by(projects[0]) + + activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value + activity_data = { + # no version yet + "version": "" + } + + serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user) + new_status_details = { + "inNextRelease": True, + } + if serialized_user: + new_status_details["actor"] = serialized_user[0] + res_type = GroupResolution.Type.in_next_release + res_type_str = "in_next_release" + res_status = GroupResolution.Status.pending + elif status_details.get("inUpcomingRelease"): + if len(projects) > 1: + return Response( + {"detail": "Cannot set resolved in upcoming release for multiple projects."}, + status=400, ) - new_status_details = { - "inUpcomingRelease": True, - } - if serialized_user: - new_status_details["actor"] = serialized_user[0] - res_type = GroupResolution.Type.in_upcoming_release - res_type_str = "in_upcoming_release" - res_status = GroupResolution.Status.pending - elif status_details.get("inRelease"): - # TODO(jess): We could update validation to check if release - # applies to multiple projects, but I think we agreed to punt - # on this for now - if len(projects) > 1: - return Response( - {"detail": "Cannot set resolved in release for multiple projects."}, status=400 - ) - release = status_details["inRelease"] - activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value - activity_data = { - # no version yet - "version": release.version - } - - serialized_user = user_service.serialize_many( - filter=dict(user_ids=[user.id]), as_user=user + release = status_details.get("inUpcomingRelease") or most_recent_release(projects[0]) + activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value + activity_data = {"version": ""} + + serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user) + new_status_details = { + "inUpcomingRelease": True, + } + if serialized_user: + new_status_details["actor"] = serialized_user[0] + res_type = GroupResolution.Type.in_upcoming_release + res_type_str = "in_upcoming_release" + res_status = GroupResolution.Status.pending + elif status_details.get("inRelease"): + # TODO(jess): We could update validation to check if release + # applies to multiple projects, but I think we agreed to punt + # on this for now + if len(projects) > 1: + return Response( + {"detail": "Cannot set resolved in release for multiple projects."}, status=400 ) - new_status_details = { - "inRelease": release.version, - } - if serialized_user: - new_status_details["actor"] = serialized_user[0] + release = status_details["inRelease"] + activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value + activity_data = { + # no version yet + "version": release.version + } + + serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user) + new_status_details = { + "inRelease": release.version, + } + if serialized_user: + new_status_details["actor"] = serialized_user[0] + res_type = GroupResolution.Type.in_release + res_type_str = "in_release" + res_status = GroupResolution.Status.resolved + elif status_details.get("inCommit"): + # TODO(jess): Same here, this is probably something we could do, but + # punting for now. + if len(projects) > 1: + return Response( + {"detail": "Cannot set resolved in commit for multiple projects."}, status=400 + ) + commit = status_details["inCommit"] + activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value + activity_data = {"commit": commit.id} + serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user) + + new_status_details = { + "inCommit": serialize(commit, user), + } + if serialized_user: + new_status_details["actor"] = serialized_user[0] + res_type_str = "in_commit" + else: + res_type_str = "now" + activity_type = ActivityType.SET_RESOLVED.value + activity_data = {} + new_status_details = {} + + now = django_timezone.now() + metrics.incr("group.resolved", instance=res_type_str, skip_internal=True) + + # if we've specified a commit, let's see if its already been released + # this will allow us to associate the resolution to a release as if we + # were simply using 'inRelease' above + # Note: this is different than the way commit resolution works on deploy + # creation, as a given deploy is connected to an explicit release, and + # in this case we're simply choosing the most recent release which contains + # the commit. + if commit and not release: + # TODO(jess): If we support multiple projects for release / commit resolution, + # we need to update this to find the release for each project (we shouldn't assume + # it's the same) + try: + release = most_recent_release_matching_commit(projects, commit) res_type = GroupResolution.Type.in_release - res_type_str = "in_release" res_status = GroupResolution.Status.resolved - elif status_details.get("inCommit"): - # TODO(jess): Same here, this is probably something we could do, but - # punting for now. - if len(projects) > 1: - return Response( - {"detail": "Cannot set resolved in commit for multiple projects."}, status=400 - ) - commit = status_details["inCommit"] - activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value - activity_data = {"commit": commit.id} - serialized_user = user_service.serialize_many( - filter=dict(user_ids=[user.id]), as_user=user - ) + except IndexError: + release = None + for group in group_list: + with transaction.atomic(router.db_for_write(Group)): + resolution = None + created = None + if release: + resolution_params = { + "release": release, + "type": res_type, + "status": res_status, + "actor_id": user.id if user.is_authenticated else None, + } - new_status_details = { - "inCommit": serialize(commit, user), - } - if serialized_user: - new_status_details["actor"] = serialized_user[0] - res_type_str = "in_commit" - else: - res_type_str = "now" - activity_type = ActivityType.SET_RESOLVED.value - activity_data = {} - new_status_details = {} - - now = django_timezone.now() - metrics.incr("group.resolved", instance=res_type_str, skip_internal=True) - - # if we've specified a commit, let's see if its already been released - # this will allow us to associate the resolution to a release as if we - # were simply using 'inRelease' above - # Note: this is different than the way commit resolution works on deploy - # creation, as a given deploy is connected to an explicit release, and - # in this case we're simply choosing the most recent release which contains - # the commit. - if commit and not release: - # TODO(jess): If we support multiple projects for release / commit resolution, - # we need to update this to find the release for each project (we shouldn't assume - # it's the same) - try: - release = most_recent_release_matching_commit(projects, commit) - res_type = GroupResolution.Type.in_release - res_status = GroupResolution.Status.resolved - except IndexError: - release = None - for group in group_list: - with transaction.atomic(router.db_for_write(Group)): - resolution = None - created = None - if release: - resolution_params = { - "release": release, - "type": res_type, - "status": res_status, - "actor_id": user.id if user.is_authenticated else None, - } - - # We only set `current_release_version` if GroupResolution type is - # in_next_release, because we need to store information about the latest/most - # recent release that was associated with a group and that is required for - # release comparisons (i.e. handling regressions) - if res_type == GroupResolution.Type.in_next_release: - # Check if semver versioning scheme is followed - follows_semver = follows_semver_versioning_scheme( - org_id=group.organization.id, - project_id=group.project.id, - release_version=release.version, - ) + # We only set `current_release_version` if GroupResolution type is + # in_next_release, because we need to store information about the latest/most + # recent release that was associated with a group and that is required for + # release comparisons (i.e. handling regressions) + if res_type == GroupResolution.Type.in_next_release: + # Check if semver versioning scheme is followed + follows_semver = follows_semver_versioning_scheme( + org_id=group.organization.id, + project_id=group.project.id, + release_version=release.version, + ) - current_release_version = get_current_release_version_of_group( - group, follows_semver + current_release_version = get_current_release_version_of_group( + group, follows_semver + ) + + if current_release_version: + resolution_params.update( + {"current_release_version": current_release_version} ) - if current_release_version: - resolution_params.update( + # Sets `current_release_version` for activity, since there is no point + # waiting for when a new release is created i.e. + # clear_expired_resolutions task to be run. + # Activity should look like "... resolved in version + # >current_release_version" in the UI + if follows_semver: + activity_data.update( {"current_release_version": current_release_version} ) - # Sets `current_release_version` for activity, since there is no point - # waiting for when a new release is created i.e. - # clear_expired_resolutions task to be run. - # Activity should look like "... resolved in version - # >current_release_version" in the UI - if follows_semver: - activity_data.update( - {"current_release_version": current_release_version} + # In semver projects, and thereby semver releases, we determine + # resolutions by comparing against an expression rather than a + # specific release (i.e. >current_release_version). Consequently, + # at this point we can consider this GroupResolution as resolved + # in release + resolution_params.update( + { + "type": GroupResolution.Type.in_release, + "status": GroupResolution.Status.resolved, + } + ) + else: + # If we already know the `next` release in date based ordering + # when clicking on `resolvedInNextRelease` because it is already + # been released, there is no point in setting GroupResolution to + # be of type in_next_release but rather in_release would suffice + + try: + # Get current release object from current_release_version + current_release_obj = Release.objects.get( + version=current_release_version, + organization_id=projects[0].organization_id, ) - # In semver projects, and thereby semver releases, we determine - # resolutions by comparing against an expression rather than a - # specific release (i.e. >current_release_version). Consequently, - # at this point we can consider this GroupResolution as resolved - # in release + date_order_q = Q(date_added__gt=current_release_obj.date_added) | Q( + date_added=current_release_obj.date_added, + id__gt=current_release_obj.id, + ) + + # Find the next release after the current_release_version + # i.e. the release that resolves the issue + resolved_in_release = ( + Release.objects.filter( + date_order_q, + projects=projects[0], + organization_id=projects[0].organization_id, + ) + .extra(select={"sort": "COALESCE(date_released, date_added)"}) + .order_by("sort", "id")[:1] + .get() + ) + + # If we get here, we assume it exists and so we update + # GroupResolution and Activity resolution_params.update( { + "release": resolved_in_release, "type": GroupResolution.Type.in_release, "status": GroupResolution.Status.resolved, } ) - else: - # If we already know the `next` release in date based ordering - # when clicking on `resolvedInNextRelease` because it is already - # been released, there is no point in setting GroupResolution to - # be of type in_next_release but rather in_release would suffice - - try: - # Get current release object from current_release_version - current_release_obj = Release.objects.get( - version=current_release_version, - organization_id=projects[0].organization_id, - ) - - date_order_q = Q( - date_added__gt=current_release_obj.date_added - ) | Q( - date_added=current_release_obj.date_added, - id__gt=current_release_obj.id, - ) - - # Find the next release after the current_release_version - # i.e. the release that resolves the issue - resolved_in_release = ( - Release.objects.filter( - date_order_q, - projects=projects[0], - organization_id=projects[0].organization_id, - ) - .extra( - select={"sort": "COALESCE(date_released, date_added)"} - ) - .order_by("sort", "id")[:1] - .get() - ) - - # If we get here, we assume it exists and so we update - # GroupResolution and Activity - resolution_params.update( - { - "release": resolved_in_release, - "type": GroupResolution.Type.in_release, - "status": GroupResolution.Status.resolved, - } - ) - activity_data.update({"version": resolved_in_release.version}) - except Release.DoesNotExist: - # If it gets here, it means we don't know the upcoming - # release yet because it does not exist, and so we should - # fall back to our current model - ... - - resolution, created = GroupResolution.objects.get_or_create( - group=group, defaults=resolution_params - ) - if not created: - resolution.update(datetime=django_timezone.now(), **resolution_params) - - if commit: - GroupLink.objects.create( - group_id=group.id, - project_id=group.project_id, - linked_type=GroupLink.LinkedType.commit, - relationship=GroupLink.Relationship.resolves, - linked_id=commit.id, - ) - - affected = Group.objects.filter(id=group.id).update( - status=GroupStatus.RESOLVED, resolved_at=now, substatus=None + activity_data.update({"version": resolved_in_release.version}) + except Release.DoesNotExist: + # If it gets here, it means we don't know the upcoming + # release yet because it does not exist, and so we should + # fall back to our current model + ... + + resolution, created = GroupResolution.objects.get_or_create( + group=group, defaults=resolution_params ) - if not resolution: - created = affected - - group.status = GroupStatus.RESOLVED - group.substatus = None - group.resolved_at = now - if affected and not options.get("groups.enable-post-update-signal"): - post_save.send( - sender=Group, - instance=group, - created=False, - update_fields=["resolved_at", "status", "substatus"], - ) - remove_group_from_inbox( - group, action=GroupInboxRemoveAction.RESOLVED, user=acting_user + if not created: + resolution.update(datetime=django_timezone.now(), **resolution_params) + + if commit: + GroupLink.objects.create( + group_id=group.id, + project_id=group.project_id, + linked_type=GroupLink.LinkedType.commit, + relationship=GroupLink.Relationship.resolves, + linked_id=commit.id, ) - result["inbox"] = None - - assigned_to = self_subscribe_and_assign_issue(acting_user, group, self_assign_issue) - if assigned_to is not None: - result["assignedTo"] = assigned_to - - if created: - activity = Activity.objects.create( - project=project_lookup[group.project_id], - group=group, - type=activity_type, - user_id=acting_user.id, - ident=resolution.id if resolution else None, - data=activity_data, - ) - record_group_history_from_activity_type(group, activity_type, actor=acting_user) - - # TODO(dcramer): we need a solution for activity rollups - # before sending notifications on bulk changes - if not is_bulk: - transaction.on_commit( - lambda: activity.send_notification(), router.db_for_write(Group) - ) - issue_resolved.send_robust( - organization_id=organization_id, - user=(acting_user or user), - group=group, - project=project_lookup[group.project_id], - resolution_type=res_type_str, - sender=update_groups, + affected = Group.objects.filter(id=group.id).update( + status=GroupStatus.RESOLVED, resolved_at=now, substatus=None ) + if not resolution: + created = affected + + group.status = GroupStatus.RESOLVED + group.substatus = None + group.resolved_at = now + if affected and not options.get("groups.enable-post-update-signal"): + post_save.send( + sender=Group, + instance=group, + created=False, + update_fields=["resolved_at", "status", "substatus"], + ) + remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED, user=acting_user) + result["inbox"] = None - kick_off_status_syncs.apply_async( - kwargs={"project_id": group.project_id, "group_id": group.id} - ) + assigned_to = self_subscribe_and_assign_issue(acting_user, group, self_assign_issue) + if assigned_to is not None: + result["assignedTo"] = assigned_to - result.update({"status": "resolved", "statusDetails": new_status_details}) + if created: + activity = Activity.objects.create( + project=project_lookup[group.project_id], + group=group, + type=activity_type, + user_id=acting_user.id, + ident=resolution.id if resolution else None, + data=activity_data, + ) + record_group_history_from_activity_type(group, activity_type, actor=acting_user) - elif status: - # The previous if statement handles the resolved and resolvedInNextRelease status updates - activity_type, activity_data, result = handle_other_status_updates( - result, - group_list, - group_ids, - projects, - project_lookup, - status_details, - acting_user, - user, + # TODO(dcramer): we need a solution for activity rollups + # before sending notifications on bulk changes + if not len(group_list) > 1: + transaction.on_commit( + lambda: activity.send_notification(), router.db_for_write(Group) + ) + + issue_resolved.send_robust( + organization_id=projects[0].organization_id, + user=(acting_user or user), + group=group, + project=project_lookup[group.project_id], + resolution_type=res_type_str, + sender=update_groups, ) - return prepare_response( - result, - group_list, - group_ids, - project_lookup, - projects, - acting_user, - data, - res_type, - request.META.get("HTTP_REFERER", ""), - ) + kick_off_status_syncs.apply_async( + kwargs={"project_id": group.project_id, "group_id": group.id} + ) + + result.update({"status": "resolved", "statusDetails": new_status_details}) + + return result, res_type def merge_groups( @@ -629,9 +637,7 @@ def handle_other_status_updates( status_details: Mapping[str, Any], acting_user: User, user: User, -): - activity_type = None - activity_data: MutableMapping[str, Any | None] | None = None +) -> dict[str, Any]: queryset = Group.objects.filter(id__in=group_ids) new_status = STATUS_UPDATE_CHOICES[result["status"]] new_substatus = ( @@ -661,7 +667,7 @@ def handle_other_status_updates( result["statusDetails"] = {} if group_list and status_updated: - activity_type, activity_data = handle_status_update( + handle_status_update( group_list=group_list, projects=projects, project_lookup=project_lookup, @@ -672,11 +678,11 @@ def handle_other_status_updates( status_details=result.get("statusDetails", {}), sender=update_groups, ) - return activity_type, activity_data, result + return result def prepare_response( - result: dict[str, Any], + result: Mapping[str, Any], group_list: Sequence[Group], group_ids: Sequence[Group], project_lookup: Mapping[int, Project], diff --git a/src/sentry/issues/status_change.py b/src/sentry/issues/status_change.py index cfda68a6ef8384..94f2a614da7ce1 100644 --- a/src/sentry/issues/status_change.py +++ b/src/sentry/issues/status_change.py @@ -1,7 +1,7 @@ from __future__ import annotations import logging -from collections import defaultdict, namedtuple +from collections import defaultdict from collections.abc import Sequence from datetime import datetime, timedelta, timezone from typing import Any @@ -25,7 +25,6 @@ from sentry.utils import json logger = logging.getLogger(__name__) -ActivityInfo = namedtuple("ActivityInfo", ("activity_type", "activity_data")) def infer_substatus( @@ -74,12 +73,10 @@ def handle_status_update( status_details: dict[str, Any], acting_user: User | None, sender: Any, -) -> ActivityInfo: +) -> None: """ Update the status for a list of groups and create entries for Activity and GroupHistory. This currently handles unresolving or ignoring groups. - - Returns a tuple of (activity_type, activity_data) for the activity that was created. """ activity_data = {} activity_type = ( @@ -173,5 +170,3 @@ def handle_status_update( created=False, update_fields=["status", "substatus"], ) - - return ActivityInfo(activity_type, activity_data)