Skip to content

chore(integrations): Add canonical way to determine the external id for a repo#112327

Merged
wedamija merged 6 commits intomasterfrom
danf/repo-external-id-refactor
Apr 8, 2026
Merged

chore(integrations): Add canonical way to determine the external id for a repo#112327
wedamija merged 6 commits intomasterfrom
danf/repo-external-id-refactor

Conversation

@wedamija
Copy link
Copy Markdown
Member

@wedamija wedamija commented Apr 7, 2026

This is prep work for a pr to genericise the automatic repo syncing pr. To make it easy to genericise, I needed a way to easily get the external id for a repo from the client. As part of adding this, I ran into a bunch of type errors, so this pr also improves typing in this area.

@wedamija wedamija requested a review from a team April 7, 2026 01:05
@wedamija wedamija requested review from a team as code owners April 7, 2026 01:05
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 7, 2026
@github-actions

This comment was marked as outdated.

@wedamija wedamija force-pushed the danf/repo-sync-audit-logs branch from 59f330b to f466517 Compare April 7, 2026 16:25
@wedamija wedamija requested review from a team as code owners April 7, 2026 16:25
Base automatically changed from danf/repo-sync-audit-logs to master April 7, 2026 16:59
@wedamija wedamija force-pushed the danf/repo-external-id-refactor branch from 02bee7a to b6936f6 Compare April 7, 2026 17:57
@github-actions

This comment was marked as outdated.

wedamija added 2 commits April 7, 2026 15:02
…or a repo

This is prep work for a pr to genericise the automatic repo syncing pr. To make it easy to genericise, I needed a way to easily get the external id for a repo from the client. As part of adding this, I ran into a bunch of type errors, so this pr also improves typing in this area.
@github-actions

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 64fc2df. Configure here.

@jaydgoss
Copy link
Copy Markdown
Member

jaydgoss commented Apr 8, 2026

I think it would be useful to have external_id added here as well

isInstalled=repo["identifier"] in installed_repo_names,
.

When working on SCM onboarding I needed to match a repo from OrganizationIntegrationReposEndpoint to a repo from OrganizationRepositoriesEndpoint, and there wasn't an obvious shared key between the two responses. externalId (already returned by RepositorySerializer) would solve that.

Ended up using IntegrationRepository.identifier === Repository.name, which works, but the relationship isn't obvious.

@wedamija
Copy link
Copy Markdown
Member Author

wedamija commented Apr 8, 2026

I think it would be useful to have external_id added here as well

isInstalled=repo["identifier"] in installed_repo_names,

.
When working on SCM onboarding I needed to match a repo from OrganizationIntegrationReposEndpoint to a repo from OrganizationRepositoriesEndpoint, and there wasn't an obvious shared key between the two responses. externalId (already returned by RepositorySerializer) would solve that.

Ended up using IntegrationRepository.identifier === Repository.name, which works, but the relationship isn't obvious.

Ok, I can return that, I'll add it as a follow up

Comment on lines +38 to +40
default_branch: NotRequired[str | None] # GitHub, GitHub Enterprise
project: NotRequired[str] # Bitbucket Server
repo: NotRequired[str] # Bitbucket Server
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's no longer common with these here now right 😬

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we should have a extended version of this type with a discriminate field, so github and ghe get the default_branch and then bitbucket server gets project and repo

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is just what we already pass around... I might merge it as is and try to improve it in follow ups, because this stuff is hairy in general

@wedamija wedamija merged commit 4cef60f into master Apr 8, 2026
62 checks passed
@wedamija wedamija deleted the danf/repo-external-id-refactor branch April 8, 2026 20:26
wedamija added a commit that referenced this pull request Apr 8, 2026
wedamija added a commit that referenced this pull request Apr 8, 2026
…eposEndpoint (#112529)

As requested in
#112327 (comment)

<!-- Describe your PR here. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants