feat: Add api/v2/relationship-kinds/{relationship_kind_id} - BED-8757 - #3061
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a ChangesRelationship kind retrieval
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GetRelationshipKindByID
participant ExtensionsService
participant RelationshipKindStore
Client->>GetRelationshipKindByID: GET relationship-kind ID
GetRelationshipKindByID->>ExtensionsService: GetRelationshipKind(ctx, id)
ExtensionsService->>RelationshipKindStore: GetRelationshipKind(ctx, id)
RelationshipKindStore-->>ExtensionsService: RelationshipKind data
ExtensionsService-->>GetRelationshipKindByID: RelationshipKind
GetRelationshipKindByID-->>Client: JSON response or mapped error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
superlinkx
reviewed
Jul 29, 2026
Contributor
|
Structurally this doesn't seem particularly problematic, however 4k lines is quite large and you've got a lot of conflicts to resolve as a result. 21 commits tells me this PR could have been broken up into smaller atomic PRs. While I wouldn't block this PR on that, being mindful of your fellow devs (and AI reviewers for context bounding reasons) and creating atomic PRs of <1k lines will lead to faster, stronger review and improve overall velocity. |
brandonshearin
changed the title
brandonshearin
force-pushed
the
BED-8757-v2
branch
from
July 30, 2026 21:34
689afac to
501b162
Compare
coderabbitai
Bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
server/extensions/internal/appdb/relationshipkind.go (1)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale extension-join TODO.
The row model and query below already join
schema_extensionsand populate its fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/extensions/internal/appdb/relationshipkind.go` at line 31, Remove the obsolete extension-join TODO comment above the relationship row model/query, leaving the existing schema_extensions join and field population unchanged.server/extensions/internal/handlers/relationshipkind.go (1)
70-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGroup
viewandmarkdownErrinto a singlevar (...)block.Per repo Go guidelines, variable initializations should be grouped in a
var (...)block and hoisted to the top of the function. Hereviewis built via:=andmarkdownErris declared separately below it.♻️ Proposed grouping
func buildRelationshipKindView(relationshipKind services.RelationshipKind) (RelationshipKindView, error) { - - view := RelationshipKindView{ - RelationshipKindID: relationshipKind.ID, - Name: relationshipKind.Name, - Description: relationshipKind.Description, - IsTraversable: relationshipKind.IsTraversable, - Extension: ExtensionView{ - ExtensionID: relationshipKind.Extension.ID, - Name: relationshipKind.Extension.Name, - DisplayName: relationshipKind.Extension.DisplayName, - Namespace: relationshipKind.Extension.Namespace, - Version: relationshipKind.Extension.Version, - }, - Info: map[string]KindInfoView{}, - } - - var markdownErr error + var ( + markdownErr error + view = RelationshipKindView{ + RelationshipKindID: relationshipKind.ID, + Name: relationshipKind.Name, + Description: relationshipKind.Description, + IsTraversable: relationshipKind.IsTraversable, + Extension: ExtensionView{ + ExtensionID: relationshipKind.Extension.ID, + Name: relationshipKind.Extension.Name, + DisplayName: relationshipKind.Extension.DisplayName, + Namespace: relationshipKind.Extension.Namespace, + Version: relationshipKind.Extension.Version, + }, + Info: map[string]KindInfoView{}, + } + )As per coding guidelines, "When possible, group variable initializations in a
var (...)block and hoist them to the top of the function."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/extensions/internal/handlers/relationshipkind.go` around lines 70 - 88, Update buildRelationshipKindView to hoist and group view and markdownErr in a single var (...) declaration at the top of the function, preserving their existing initial values and subsequent behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/extensions/internal/routes/routes.go`:
- Around line 32-42: Update Register so router.With applies rateLimit to both
nodeKindRoute and relationshipKindRoute. Keep the existing GraphDBRead
permission requirements unchanged for each route.
In `@server/extensions/internal/services/relationshipkind.go`:
- Around line 25-26: Correct the doc comment immediately above
ErrRelationshipKindNotFound so it names ErrRelationshipKindNotFound
consistently.
---
Nitpick comments:
In `@server/extensions/internal/appdb/relationshipkind.go`:
- Line 31: Remove the obsolete extension-join TODO comment above the
relationship row model/query, leaving the existing schema_extensions join and
field population unchanged.
In `@server/extensions/internal/handlers/relationshipkind.go`:
- Around line 70-88: Update buildRelationshipKindView to hoist and group view
and markdownErr in a single var (...) declaration at the top of the function,
preserving their existing initial values and subsequent behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8ef520bf-2cd3-4693-9735-ab1bc0a37afb
📒 Files selected for processing (21)
packages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/openapi.yamlpackages/go/openapi/src/paths/opengraph.relationship-kind.yamlpackages/go/openapi/src/schemas/model.relationship-kind-response.yamlserver/extensions/internal/appdb/kindinfo_test.goserver/extensions/internal/appdb/relationshipkind.goserver/extensions/internal/appdb/relationshipkind_integration_test.goserver/extensions/internal/handlers/extension.goserver/extensions/internal/handlers/handlers.goserver/extensions/internal/handlers/kindinfo.goserver/extensions/internal/handlers/mocks/extensions.goserver/extensions/internal/handlers/nodekind.goserver/extensions/internal/handlers/nodekind_test.goserver/extensions/internal/handlers/relationshipkind.goserver/extensions/internal/handlers/relationshipkind_test.goserver/extensions/internal/routes/routes.goserver/extensions/internal/services/mocks/database.goserver/extensions/internal/services/nodekind_test.goserver/extensions/internal/services/relationshipkind.goserver/extensions/internal/services/relationshipkind_test.goserver/extensions/internal/services/services.go
brandonshearin
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/extensions/internal/services/relationshipkind.go`:
- Around line 48-50: Update the extension lookup in the relationship-kind
retrieval method to store relKind.Extension.ID in a local variable before
assigning the fetched extension, then use that preserved ID in the
fetching-extension error message. Keep the existing lookup and
ErrExtensionNotFound handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f905c338-2049-4ce2-aa1b-a0900db6e8c5
📒 Files selected for processing (6)
server/extensions/internal/appdb/relationshipkind.goserver/extensions/internal/appdb/relationshipkind_integration_test.goserver/extensions/internal/handlers/relationshipkind_test.goserver/extensions/internal/services/relationshipkind.goserver/extensions/internal/services/relationshipkind_test.goserver/extensions/internal/services/services.go
💤 Files with no reviewable changes (2)
- server/extensions/internal/appdb/relationshipkind_integration_test.go
- server/extensions/internal/services/services.go
🚧 Files skipped from review as they are similar to previous changes (2)
- server/extensions/internal/handlers/relationshipkind_test.go
- server/extensions/internal/services/relationshipkind_test.go
Comment on lines
+48
to
+50
| if relKind.Extension.ID != 0 { | ||
| if relKind.Extension, err = s.db.GetExtension(ctx, relKind.Extension.ID); err != nil && !errors.Is(err, ErrExtensionNotFound) { | ||
| return RelationshipKind{}, fmt.Errorf("fetching extension %d for relationship kind %d: %w", relKind.Extension.ID, id, err) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the requested extension ID for error reporting.
The assignment on Line 49 overwrites relKind.Extension before Line 50 formats the error. When the lookup returns an empty extension, the error reports extension 0 instead of the ID that was actually requested.
Proposed fix
- if relKind.Extension.ID != 0 {
- if relKind.Extension, err = s.db.GetExtension(ctx, relKind.Extension.ID); err != nil && !errors.Is(err, ErrExtensionNotFound) {
- return RelationshipKind{}, fmt.Errorf("fetching extension %d for relationship kind %d: %w", relKind.Extension.ID, id, err)
+ if extensionID := relKind.Extension.ID; extensionID != 0 {
+ if relKind.Extension, err = s.db.GetExtension(ctx, extensionID); err != nil && !errors.Is(err, ErrExtensionNotFound) {
+ return RelationshipKind{}, fmt.Errorf("fetching extension %d for relationship kind %d: %w", extensionID, id, err)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if relKind.Extension.ID != 0 { | |
| if relKind.Extension, err = s.db.GetExtension(ctx, relKind.Extension.ID); err != nil && !errors.Is(err, ErrExtensionNotFound) { | |
| return RelationshipKind{}, fmt.Errorf("fetching extension %d for relationship kind %d: %w", relKind.Extension.ID, id, err) | |
| if extensionID := relKind.Extension.ID; extensionID != 0 { | |
| if relKind.Extension, err = s.db.GetExtension(ctx, extensionID); err != nil && !errors.Is(err, ErrExtensionNotFound) { | |
| return RelationshipKind{}, fmt.Errorf("fetching extension %d for relationship kind %d: %w", extensionID, id, err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/extensions/internal/services/relationshipkind.go` around lines 48 -
50, Update the extension lookup in the relationship-kind retrieval method to
store relKind.Extension.ID in a local variable before assigning the fetched
extension, then use that preserved ID in the fetching-extension error message.
Keep the existing lookup and ErrExtensionNotFound handling unchanged.
AD7ZJ
reviewed
Jul 31, 2026
AD7ZJ
reviewed
Jul 31, 2026
| Description string `json:"description"` | ||
| IsTraversable bool `json:"is_traversable"` | ||
| Info map[string]KindInfoView `json:"info"` | ||
| Extension ExtensionView `json:"extension"` |
Member
There was a problem hiding this comment.
Do you think this should be a pointer, *ExtensionView like it is for NodeKind?
AD7ZJ
approved these changes
Aug 3, 2026
AD7ZJ
left a comment
AD7ZJ
left a comment
Member
There was a problem hiding this comment.
Nothing blocking that I see, send it!
LawsonWillard
approved these changes
Aug 3, 2026
LawsonWillard
left a comment
LawsonWillard
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me, great work! 🚀
Merged
3 tasks
brandonshearin
enabled auto-merge (squash)
August 3, 2026 21:48
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
Description
KindInfoViewinto dedicatedhandlers/kindinfo.gofor consumption by both nodekind and relkind endpointsMotivation and Context
Resolves BED-8757
We want users to be able to read the relationship kinds they have registered via OpenGraph in their DB.
How Has This Been Tested?
Here is an example API response when I query for the AD WriteDacl relationship:
Then, I added some kind infos to the WriteDacl relationship. I queried the endpoint again, and see that
infois populated as expected:Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit