-
Notifications
You must be signed in to change notification settings - Fork 0
[BB-1836] add owner role #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes introduce support for an "owner" role to the connector system. A new owner role constant is added and included in role enumeration, GraphQL queries are extended to fetch owner information from the Linear API, data models are updated with owner fields, and user role assignment logic handles the owner flag. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/linear/client.go (1)
155-199:GetUsersquery:ownerfield is added in the right spot; consider a fallback if schema doesn’t support it.
If Linear ever returns “Cannot query fieldowner…”, this call will hard-fail; a targeted retry withoutownerwould keep older schemas working (if that matters for this connector).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.github/workflows/ci.yamlis excluded by none and included by none.github/workflows/main.yamlis excluded by none and included by none.golangci.ymlis excluded by none and included by none
📒 Files selected for processing (4)
pkg/connector/role.go(1 hunks)pkg/connector/user.go(1 hunks)pkg/linear/client.go(3 hunks)pkg/linear/models.go(2 hunks)
🔇 Additional comments (6)
pkg/linear/models.go (2)
45-61: Modeling looks fine; only compatibility risk is whetherowneris always present in Linear’s schema.
Go-side change is safe (absent field decodes tofalse), but if some Linear tenants/plans don’t expose theownerfield, the query changes elsewhere would fail.
111-116:ViewerPermissions.Owneraddition is straightforward.
No concerns on the Go model; same schema-availability caveat as above.pkg/linear/client.go (2)
274-336:GetOrganizationquery: same schema-compatibility concern asGetUsers.
The selection is minimal (id/admin/guest/owner), which is good; just ensureowneris always queryable.
448-469:Authorizequery: addingownermakes sense; confirm downstream authorization logic expectations.
This now surfaces an additional privilege bit; ensure anything consumingViewerPermissionswon’t accidentally treat “Owner but not Admin” unexpectedly (if that state exists).pkg/connector/user.go (1)
39-49: Role precedence is clear and likely correct (Owner > Admin > Guest > User).
If Linear can return overlapping flags (e.g., bothownerandadmin), this will intentionally pickowner. Worth confirming that’s the desired mapping.pkg/connector/role.go (1)
24-36: AddingroleOwner+ including it inrolesis clean and consistent with existing patterns.
Only thing to double-check is whether any tests/assertions depend on the exact enumerated set/order of roles.
[X] Bugfix
Linear updated their platform, when upgrading to enterprise ALL admin users become Owners. This is a newly created role, added to the existing ones.
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.