Conversation
WalkthroughSupport for including and excluding repository connections in search contexts was added across schema definitions, TypeScript types, and documentation. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SyncJob
participant DB
participant ConnectionService
User->>SyncJob: Define search context (include, exclude, includeConnections, excludeConnections)
SyncJob->>DB: Fetch all repositories
alt include pattern specified
SyncJob->>DB: Filter repos by include pattern
end
alt includeConnections specified
SyncJob->>ConnectionService: Fetch connections by name/org
ConnectionService-->>SyncJob: Return associated repositories
SyncJob->>DB: Add these repos to inclusion list
end
alt exclude pattern specified
SyncJob->>DB: Remove repos by exclude pattern
end
alt excludeConnections specified
SyncJob->>ConnectionService: Fetch connections by name/org
ConnectionService-->>SyncJob: Return associated repositories
SyncJob->>DB: Remove these repos from inclusion list
end
SyncJob->>DB: Upsert search context with resolved repos
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment has been minimized.
This comment has been minimized.
msukkari
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/snippets/schemas/v3/index.schema.mdx (1)
245-251: Same duplication concern forexcludeConnectionsSee the previous comment – the same reasoning applies here.
🧹 Nitpick comments (4)
packages/shared/src/ee/syncSearchContexts.ts (1)
46-71: Consider deduplicating repositories to avoid potential issues.The logic correctly fetches repositories from included connections, but concatenating without deduplication could result in duplicate entries if the same repository belongs to multiple connections or was already included via patterns.
Consider deduplicating the repository list:
for (const connection of connections) { - newReposInContext = newReposInContext.concat(connection.repos.map(repo => repo.repo)); + const connectionRepos = connection.repos.map(repo => repo.repo); + for (const repo of connectionRepos) { + if (!newReposInContext.find(existing => existing.id === repo.id)) { + newReposInContext.push(repo); + } + } }Or use a more efficient approach with a Set:
+ const existingIds = new Set(newReposInContext.map(repo => repo.id)); for (const connection of connections) { - newReposInContext = newReposInContext.concat(connection.repos.map(repo => repo.repo)); + const connectionRepos = connection.repos.map(repo => repo.repo); + newReposInContext.push(...connectionRepos.filter(repo => !existingIds.has(repo.id))); + connectionRepos.forEach(repo => existingIds.add(repo.id)); }docs/snippets/schemas/v3/index.schema.mdx (3)
95-101: Strengthen validation forincludeConnectionsarray itemsThe items are currently validated as generic strings.
Everywhere else, connection identifiers are constrained by the regex^[a-zA-Z0-9_-]+$. Adding the same pattern here (and optionally aminItems: 1plus an example) will prevent typos and empty arrays from silently passing validation."includeConnections": { "type": "array", "description": "List of connections to include in the search context.", - "items": { - "type": "string" - } + "items": { + "type": "string", + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "minItems": 1, + "examples": [["github-enterprise", "gitlab-prod"]] }
115-121: Mirror the tighter validation inexcludeConnectionsFor symmetry with the previous suggestion, apply the same
pattern,minItems, andexamplesattributes here.
225-231: Duplication between definitions – consider a$ref
includeConnectionsis now declared twice (rootSearchContextand the per-contextpatternProperties). Keeping two in-place copies invites drift in future edits.
Using a$refto a singleSearchContextdefinition (or generating the file via$defsin the code-gen step) would eliminate this risk.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)docs/snippets/schemas/v3/index.schema.mdx(4 hunks)docs/snippets/schemas/v3/searchContext.schema.mdx(2 hunks)packages/schemas/src/v3/index.schema.ts(4 hunks)packages/schemas/src/v3/index.type.ts(1 hunks)packages/schemas/src/v3/searchContext.schema.ts(2 hunks)packages/schemas/src/v3/searchContext.type.ts(1 hunks)packages/shared/src/ee/syncSearchContexts.ts(2 hunks)schemas/v3/searchContext.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
CHANGELOG.mdpackages/schemas/src/v3/index.type.tspackages/schemas/src/v3/searchContext.type.tspackages/shared/src/ee/syncSearchContexts.tspackages/schemas/src/v3/index.schema.tspackages/schemas/src/v3/searchContext.schema.tsdocs/snippets/schemas/v3/index.schema.mdxschemas/v3/searchContext.jsondocs/snippets/schemas/v3/searchContext.schema.mdx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
CHANGELOG.md (1)
12-12: LGTM! Changelog entry properly documents the new feature.The entry accurately describes the functionality and follows the standard changelog format with appropriate PR reference.
packages/schemas/src/v3/index.type.ts (1)
117-129: LGTM! Interface changes correctly implement the new connection filtering feature.The changes properly:
- Make the
includeproperty optional for more flexible configurations- Add well-documented optional properties for connection inclusion and exclusion
- Maintain type safety with appropriate string array types
schemas/v3/searchContext.json (1)
20-26: LGTM! Schema additions are well-structured and consistent.The new properties are properly defined with:
- Correct array type definitions for string values
- Clear and descriptive documentation
- Consistent structure with existing properties
Also applies to: 40-46
packages/schemas/src/v3/searchContext.schema.ts (1)
21-27: LGTM! Schema definitions are consistent and well-structured.The new connection properties follow the established schema patterns with proper type definitions and descriptive documentation.
Also applies to: 41-47
packages/schemas/src/v3/searchContext.type.ts (1)
10-14: LGTM! Interface changes maintain consistency across schema files.The SearchContext interface correctly implements the new optional properties with proper typing and documentation that matches the main schema definitions.
Also applies to: 19-22
docs/snippets/schemas/v3/searchContext.schema.mdx (2)
22-28: Schema definition looks correct.The
includeConnectionsproperty is properly defined as an optional array of strings with a clear description that aligns with the PR objectives for connection-based filtering.
42-48: Schema definition looks correct.The
excludeConnectionsproperty is properly defined as an optional array of strings with a clear description that complements theincludeConnectionsfunctionality.packages/schemas/src/v3/index.schema.ts (4)
94-100: Schema definition is consistent and correct.The
includeConnectionsproperty in the SearchContext definition matches the documentation schema and maintains consistency across the codebase.
114-120: Schema definition is consistent and correct.The
excludeConnectionsproperty in the SearchContext definition matches the documentation schema and maintains consistency across the codebase.
224-230: Consistent schema definition in contexts pattern.The
includeConnectionsproperty is correctly duplicated in the contexts pattern properties to maintain schema consistency between standalone and nested SearchContext definitions.
244-250: Consistent schema definition in contexts pattern.The
excludeConnectionsproperty is correctly duplicated in the contexts pattern properties to maintain schema consistency between standalone and nested SearchContext definitions.packages/shared/src/ee/syncSearchContexts.ts (3)
39-44: Correct implementation of optional include property.The initialization change properly supports making the
includeproperty optional, allowing search contexts to rely solely on connection-based filtering. The conditional check and non-null assertion are appropriate.
80-107: Exclusion logic is correctly implemented.The implementation properly filters out repositories belonging to excluded connections. The database query pattern is consistent with the inclusion logic, and the filtering approach using repo IDs is accurate.
39-107: Well-structured implementation of connection-based filtering.The overall flow correctly implements the feature requirements:
- Maintains backwards compatibility with existing include/exclude patterns
- Properly integrates connection-based filtering
- Follows a logical sequence: include → exclude for both patterns and connections
- Preserves existing database synchronization logic
The implementation successfully extends search context functionality while maintaining code clarity and consistency.
1 participant
Summary by CodeRabbit
New Features
Documentation
includeConnectionsandexcludeConnectionsoptions and the optional nature of theincludeproperty in search contexts.