✨ feat(GHE): add issue sync support #103424
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| assert pr.merge_commit_sha == "0d1a26e67d8f5eaf1f6ba5c57fc3c7d91ac0fd1c" | ||
|
|
||
|
|
||
| @with_feature("organizations:integrations-github-project-management") |
There was a problem hiding this comment.
Bug: Fix Enterprise feature flag mismatch in webhook tests.
The webhook test class uses the wrong feature flag "organizations:integrations-github-project-management" (for GitHub) instead of "organizations:integrations-github_enterprise-project-management" (for GitHub Enterprise). This causes tests to pass under conditions that wouldn't exist in production. The should_sync_assignee_inbound function checks for the github_enterprise specific flag, and the GitHubIssueSyncSpec.check_feature_flag method also constructs the feature flag name from the provider, which would be github_enterprise. The test decorator needs to match the actual feature flag that gates the functionality.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## raj/gh-feat-par/issue-sync-spec #103424 +/- ##
====================================================================
- Coverage 80.61% 80.61% -0.01%
====================================================================
Files 9414 9414
Lines 403736 403796 +60
Branches 25662 25662
====================================================================
+ Hits 325466 325510 +44
- Misses 77801 77817 +16
Partials 469 469 |
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
30db582 to
baf8dd1
Compare
iamrajjoshi
force-pushed
the
raj/gh-feat-par/issue-sync-spec
branch
from
60a6c1a to
6547644
Compare
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
baf8dd1 to
e3d8201
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
iamrajjoshi
force-pushed
the
raj/gh-feat-par/issue-sync-spec
branch
from
6547644 to
c3280f7
Compare
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
e3d8201 to
b0e16aa
Compare
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
b0e16aa to
7d5916a
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| """ | ||
| config: list[dict[str, Any]] = [] | ||
|
|
||
| if features.has("organizations:integrations-github-project-management", self.organization): |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
7d5916a to
2ad440c
Compare
| IntegrationExternalProject.objects.filter( | ||
| organization_integration_id=self.org_integration.id | ||
| ).delete() |
There was a problem hiding this comment.
I just realized, shouldn't this be in a transaction along with the create stuff a few lines down? We'd want these project updates to be atomic whenever possible. Same with the GitHub update_configuration work.
There was a problem hiding this comment.
true, i think this is something that exists with jira and azure as well.
i can put up a followup PR for this.
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
998a559 to
9f14071
Compare
iamrajjoshi
added
the
Trigger: getsentry tests
iamrajjoshi
force-pushed
the
raj/ghe-feat-par
branch
from
9f14071 to
088f3ba
Compare
| external_id=repo_id, | ||
| resolved_status=statuses["on_resolve"], | ||
| unresolved_status=statuses["on_unresolve"], | ||
| ) |
There was a problem hiding this comment.
Config update deletes data before validation completes
Medium Severity
The update_organization_config method deletes all existing IntegrationExternalProject records at lines 460-462 before validating each new project's status values in the loop at lines 467-477. If validation fails for any project (e.g., invalid status value), the original configuration is already deleted but the new configuration is incomplete. This causes data loss—users lose their original settings and don't get their intended new configuration.
There was a problem hiding this comment.
Already flagged this in the manual review, but good catch. @iamrajjoshi, please make sure to post this please.
There was a problem hiding this comment.
yep follow pr for this.
Added support for the Issue Sync Integration for Github Enterprise. Its mostly just using the issue sync spec.
Please review the first commit because it is the only commit with the changes, the rest is just boiler plate tests.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.