Linear - Throw blocking configurationError for webhooks #14884
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe pull request includes updates to version numbers and dependencies in several Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
components/linear_app/sources/common/webhook.mjs (1)
27-37: Cache admin status to reduce redundant API callsThe
isAdmin()method is called in bothadditionalProps()and thedeployhook, which may lead to multiple API requests. Consider caching the admin status during the component's lifecycle to improve performance.Apply this diff to implement caching:
export default { // ...existing code... data() { return { + adminStatus: null, }; }, async additionalProps() { const props = {}; - if (!(await this.isAdmin())) { + if (!(await this.getAdminStatus())) { props.alert = { type: "alert", alertType: "error", content: "You must have an admin role to create or manage webhooks. See the Linear [documentation](https://linear.app/docs/api-and-webhooks#webhooks) for details.", }; } return props; }, methods: { + async getAdminStatus() { + if (this.adminStatus === null) { + this.adminStatus = await this.isAdmin(); + } + return this.adminStatus; + }, async isAdmin() { // existing isAdmin implementation... }, // ...existing methods... }, hooks: { async deploy() { - if (!(await this.isAdmin())) { + if (!(await this.getAdminStatus())) { throw new ConfigurationError("You must have an admin role to create or manage webhooks. See the Linear [documentation](https://linear.app/docs/api-and-webhooks#webhooks) for details."); } // ...existing code... }, // ...existing hooks... }, // ...existing code... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/linear/package.json(2 hunks)components/linear/sources/comment-created-instant/comment-created-instant.mjs(1 hunks)components/linear/sources/issue-created-instant/issue-created-instant.mjs(1 hunks)components/linear/sources/issue-updated-instant/issue-updated-instant.mjs(1 hunks)components/linear/sources/new-issue-status-updated/new-issue-status-updated.mjs(1 hunks)components/linear_app/package.json(2 hunks)components/linear_app/sources/comment-created-instant/comment-created-instant.mjs(1 hunks)components/linear_app/sources/common/webhook.mjs(4 hunks)components/linear_app/sources/issue-created-instant/issue-created-instant.mjs(1 hunks)components/linear_app/sources/issue-updated-instant/issue-updated-instant.mjs(1 hunks)components/linear_app/sources/new-issue-status-updated/new-issue-status-updated.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- components/linear_app/sources/comment-created-instant/comment-created-instant.mjs
- components/linear/sources/issue-updated-instant/issue-updated-instant.mjs
- components/linear/sources/issue-created-instant/issue-created-instant.mjs
- components/linear/sources/comment-created-instant/comment-created-instant.mjs
- components/linear_app/sources/new-issue-status-updated/new-issue-status-updated.mjs
- components/linear/sources/new-issue-status-updated/new-issue-status-updated.mjs
- components/linear_app/sources/issue-updated-instant/issue-updated-instant.mjs
- components/linear_app/sources/issue-created-instant/issue-created-instant.mjs
🔇 Additional comments (6)
components/linear/package.json (2)
3-3: LGTM: Version bump is appropriate
The patch version increment (0.5.5 -> 0.5.6) aligns well with the scope of changes (adding error handling for webhook configuration).
18-18: Verify compatibility with @pipedream/platform v3
The dependency update from ^1.3.0 to ^3.0.3 represents a major version jump which might introduce breaking changes.
Let's check the changelog and any migration guides:
components/linear_app/package.json (2)
3-3: LGTM: Changes are consistent with @pipedream/linear package
The version bump and dependency updates match the changes in the linear package, maintaining consistency across related packages.
Also applies to: 18-18
1-1: Request additional files for review
While the package updates look good, the core implementation files for the webhook changes (particularly the files implementing the blocking configurationError and admin role validation) are not included in this review set. To ensure the PR objectives are met, please include:
- The webhook implementation files
- Any new/modified GraphQL queries for admin status checking
- Any error handling implementation files
Let's check for relevant files:
✅ Verification successful
Implementation of admin role validation and configurationError blocking is verified
The core implementation files for webhook changes are present and correctly implemented in components/linear_app/sources/common/webhook.mjs. The code shows:
- Admin role validation through
isAdmin()check - Proper blocking with
ConfigurationErrorwhen non-admin users attempt to create/manage webhooks - User-friendly error messages with documentation links
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Look for webhook and GraphQL related files
# Find webhook related files
rg -l "webhook" -g '!node_modules' -g '!package*.json'
# Find GraphQL query files
rg -l "graphql|query.*admin" -g '!node_modules'
Length of output: 65650
Script:
#!/bin/bash
# Look for webhook and GraphQL implementation files in Linear app component
rg -l "webhook|configurationError|admin.*role" components/linear_app/ -g '!package*.json'
# Look for webhook implementation details
rg "configurationError|admin.*role" components/linear_app/ -A 5 -B 5
Length of output: 2441
components/linear_app/sources/common/webhook.mjs (2)
4-4: Import ConfigurationError for proper error handling
The import of ConfigurationError from @pipedream/platform is necessary to throw configuration errors when the user lacks admin permissions.
16-16: reloadProps: true ensures updated properties
Setting reloadProps: true allows the component to refresh its properties when there are changes, ensuring the UI reflects the most current data.
Resolves #14876
Summary by CodeRabbit
Release Notes
New Features
Version Updates
@pipedream/linearand@pipedream/linear_appupdated to0.5.6.comment-created-instant,issue-created-instant,issue-updated-instant, andnew-issue-status-updatedupdated to their respective new versions.