fix(suspect commits): Add commitId to RELEASE_BASED suspect commits #103006
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "user_id": owner_id, | ||
| "project_id": project.id, | ||
| "organization_id": project.organization_id, | ||
| "context__asjsonb__commitId": owner_commits[owner_id], |
There was a problem hiding this comment.
Bug: Owner Duplication: Lookup Prevents Record Updates
Including commitId in lookup_kwargs prevents updating existing GroupOwner records when the same author has a new suspect commit. Instead of updating the existing owner's commitId, a new GroupOwner is created, causing duplicate owners for the same user/group combination and breaking the PREFERRED_GROUP_OWNERS limit enforcement. The lookup should match on (group_id, type, user_id, project_id, organization_id) to update existing records, not create duplicates.
There was a problem hiding this comment.
I want separate GroupOwners for separate commits. This record does not represent a Group + User, it represents a Group + Commit
| "user_id": owner_id, | ||
| "project_id": project.id, | ||
| "organization_id": project.organization_id, | ||
| "context__asjsonb__commitId": owner_commits[owner_id], |
There was a problem hiding this comment.
do we have any tests which will test that this task will successfully update a group owner with the right fields?
There was a problem hiding this comment.
yes - take a look at test_update_existing_entries https://github.com/getsentry/sentry/pull/103006/files#diff-b6e0afbaeb8ea99203894d42f9693cc2acc50110771a7ded33a4115cd26c62f0R272
Just added a better check for commitId
This shows we would not create duplicates but LMK if there are other test cases I should cover 👍
…103006) ## Problem `EventFileCommittersEndpoint` uses `_get_serialized_committers_from_group_owners` which filters for GroupOwners with `commitId` in context: ```python owner = next(filter(lambda go: go.context.get("commitId"), group_owners), None) ``` RELEASE_BASED suspect commits only store `suspectCommitStrategy`, not `commitId`, so they're never displayed on the Issue detail page. ## Solution Match the SCM_BASED pattern by: 1. Tracking the highest-scoring commit for each author 2. Storing `commitId` in `context_defaults` 3. Including `commitId` in `lookup_kwargs` for proper deduplication
…103006) ## Problem `EventFileCommittersEndpoint` uses `_get_serialized_committers_from_group_owners` which filters for GroupOwners with `commitId` in context: ```python owner = next(filter(lambda go: go.context.get("commitId"), group_owners), None) ``` RELEASE_BASED suspect commits only store `suspectCommitStrategy`, not `commitId`, so they're never displayed on the Issue detail page. ## Solution Match the SCM_BASED pattern by: 1. Tracking the highest-scoring commit for each author 2. Storing `commitId` in `context_defaults` 3. Including `commitId` in `lookup_kwargs` for proper deduplication
Problem
EventFileCommittersEndpointuses_get_serialized_committers_from_group_ownerswhich filters for GroupOwners withcommitIdin context:RELEASE_BASED suspect commits only store
suspectCommitStrategy, notcommitId, so they're never displayed on the Issue detail page.Solution
Match the SCM_BASED pattern by:
commitIdincontext_defaultscommitIdinlookup_kwargsfor proper deduplication