feat(worker): Add sync all option for bitbucket server#927
Open
svedin wants to merge 7 commits intosourcebot-dev:mainfrom
Open
feat(worker): Add sync all option for bitbucket server#927svedin wants to merge 7 commits intosourcebot-dev:mainfrom
svedin wants to merge 7 commits intosourcebot-dev:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds an optional Changes
Sequence DiagramsequenceDiagram
participant User as User/Config
participant Backend as Backend
participant Bitbucket as Bitbucket Server API
User->>Backend: Submit connection config (`all: true`)
Backend->>Backend: Inspect `deploymentType`
alt deploymentType == "server"
Backend->>Bitbucket: GET /rest/api/1.0/repos?page=0
Bitbucket-->>Backend: repos batch + pagination info
loop while more pages
Backend->>Bitbucket: GET /rest/api/1.0/repos?page=n
Bitbucket-->>Backend: repos batch
end
Backend->>Backend: Aggregate repos + warnings
Backend-->>User: Return collected repos
else deploymentType == "cloud"
Backend->>Backend: Record/log warning (option ignored)
Backend-->>User: Return result with warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/bitbucket.ts (1)
77-87: Consider avoiding redundant repo fetches whenallis enabled.If
allis intended to be a full sync, the laterworkspaces/projects/reposbranches still fire and can cause duplicate entries or extra API calls. Consider short‑circuiting those selectors or de‑duplicating before filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/bitbucket.ts` around lines 77 - 87, When config.all === true we call serverGetAllRepos and still fall through to the later workspaces/projects/repos branches causing duplicate fetches; short‑circuit after handling the all:true case by returning or skipping the subsequent selectors (or deduplicate before applying filters). Update the code around the allRepos/allWarnings handling (the config.all check and serverGetAllRepos call, and the subsequent workspaces/projects/repos branches) to either: 1) immediately skip/return after concatenating repos/warnings when config.all is true, or 2) perform a deduplication step on allRepos (e.g., by repo id or slug) before any further filtering to prevent duplicate entries and extra API calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/bitbucket.ts`:
- Around line 77-87: When config.all === true we call serverGetAllRepos and
still fall through to the later workspaces/projects/repos branches causing
duplicate fetches; short‑circuit after handling the all:true case by returning
or skipping the subsequent selectors (or deduplicate before applying filters).
Update the code around the allRepos/allWarnings handling (the config.all check
and serverGetAllRepos call, and the subsequent workspaces/projects/repos
branches) to either: 1) immediately skip/return after concatenating
repos/warnings when config.all is true, or 2) perform a deduplication step on
allRepos (e.g., by repo id or slug) before any further filtering to prevent
duplicate entries and extra API calls.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/docs/connections/bitbucket-data-center.mdxdocs/snippets/schemas/v3/bitbucket.schema.mdxdocs/snippets/schemas/v3/connection.schema.mdxdocs/snippets/schemas/v3/index.schema.mdxpackages/backend/src/bitbucket.tspackages/schemas/src/v3/bitbucket.schema.tspackages/schemas/src/v3/bitbucket.type.tspackages/schemas/src/v3/connection.schema.tspackages/schemas/src/v3/connection.type.tspackages/schemas/src/v3/index.schema.tspackages/schemas/src/v3/index.type.tsschemas/v3/bitbucket.json
brendan-kellam
changed the title
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/snippets/schemas/v3/bitbucket.schema.mdx (1)
70-74: Consider schema-level enforcement forall+cloudcombination.Currently,
all: truewithdeploymentType: "cloud"passes schema validation silently. The only signal to the user is the prose description. Adding a schema constraint (e.g., via an additionalif/then) would surface a validation error at config parse time rather than silently ignoring the option.♻️ Suggested addition to the existing if/then/else block
"if": { "properties": { "deploymentType": { "const": "server" } } }, "then": { "required": [ "url" ] }, + "else": { + "properties": { + "all": { + "const": false, + "description": "The `all` option is only supported for `deploymentType: server`." + } + } + }, "additionalProperties": falseNote: since this is an auto-generated file, the change should be applied to the source schema (e.g.,
packages/schemas/src/v3/bitbucket.schema.tsandschemas/v3/bitbucket.json) and regenerated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/snippets/schemas/v3/bitbucket.schema.mdx` around lines 70 - 74, Add a JSON Schema-level conditional that rejects the combination of "all": true together with deploymentType: "cloud": insert an if that matches deploymentType equal to "cloud" and all equal to true, and in the then branch make the schema invalid for that combination (e.g., using a not or a then that forces an impossible constraint for "all") so that validation fails instead of silently ignoring the option; apply this change to the source schema and regenerate the generated JSON/MDX artifacts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/snippets/schemas/v3/bitbucket.schema.mdx`:
- Around line 70-74: Add a JSON Schema-level conditional that rejects the
combination of "all": true together with deploymentType: "cloud": insert an if
that matches deploymentType equal to "cloud" and all equal to true, and in the
then branch make the schema invalid for that combination (e.g., using a not or a
then that forces an impossible constraint for "all") so that validation fails
instead of silently ignoring the option; apply this change to the source schema
and regenerate the generated JSON/MDX artifacts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/docs/connections/bitbucket-data-center.mdxdocs/snippets/schemas/v3/bitbucket.schema.mdxdocs/snippets/schemas/v3/connection.schema.mdxdocs/snippets/schemas/v3/index.schema.mdxpackages/backend/src/bitbucket.tspackages/schemas/src/v3/bitbucket.schema.tspackages/schemas/src/v3/bitbucket.type.tspackages/schemas/src/v3/connection.schema.tspackages/schemas/src/v3/connection.type.tspackages/schemas/src/v3/index.schema.tspackages/schemas/src/v3/index.type.tsschemas/v3/bitbucket.json
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/schemas/src/v3/bitbucket.type.ts
- packages/schemas/src/v3/connection.type.ts
- packages/schemas/src/v3/index.type.ts
- packages/schemas/src/v3/index.schema.ts
- packages/backend/src/bitbucket.ts
- packages/schemas/src/v3/connection.schema.ts
- docs/snippets/schemas/v3/index.schema.mdx
- packages/schemas/src/v3/bitbucket.schema.ts
- docs/snippets/schemas/v3/connection.schema.mdx
- schemas/v3/bitbucket.json
Contributor
|
@claude add a changelog entry |
|
Claude Code is working… I'll analyze this and get back to you. |
Author
|
I have added the changelog |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #926
Implementation of an option to sync all repositories for bitbucket server
Summary by CodeRabbit
New Features
Documentation
Changelog