-
Notifications
You must be signed in to change notification settings - Fork 97
fix: put add blueprint plus button behind FF - AB-824 #1254
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
base: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds per-section add-button visibility to the Builder sidebar component and centralizes the shared_blueprints feature-flag check into a new computed property Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (8)
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 |
HackerOne Code Security Review🟢 Scan Complete: 1 Issue(s) Here's how the code changes were interpreted and info about the tools used for scanning. 📖 Summary of ChangesThe changes involve enhancing the BuilderSidebarComponentTree Vue component by introducing a computed property to check shared blueprint feature availability. The modification adjusts the blueprint sections computation to conditionally render add buttons based on a feature flag, providing more flexible section configuration with a new `showAddButton` property.
ℹ️ Issues DetectedNOTE: These may not require action! Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem. How will I know if something is a problem?
🧰 Analysis tools
⏱️ Latest scan covered changes up to commit 580a8b1 (latest) |
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)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (1)
22-34: Indentation inconsistency in template.The section header block (lines 22-33) uses 3 tabs of indentation, but it should use 4 tabs to match
section__contentat line 34, since both are children of the<div class="section">element.🔎 Suggested fix
- <div class="section__header"> - <span class="section__title">{{ section.title }}</span> - <WdsButton - v-if="section.showAddButton" - variant="neutral" - size="smallIcon" - :data-automation-action="section.addAction" - @click="section.onAdd" - > - <WdsIcon name="plus" /> - </WdsButton> - </div> + <div class="section__header"> + <span class="section__title">{{ section.title }}</span> + <WdsButton + v-if="section.showAddButton" + variant="neutral" + size="smallIcon" + :data-automation-action="section.addAction" + @click="section.onAdd" + > + <WdsIcon name="plus" /> + </WdsButton> + </div> <div class="section__content">
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue
⏰ 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). (8)
- GitHub Check: tests (firefox)
- GitHub Check: tests (webkit)
- GitHub Check: tests (chromium)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
🔇 Additional comments (3)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (3)
63-72: Verify: Footer "Add blueprint" button not behind feature flag.The section header add button is correctly gated by
isSharedBlueprintsEnabled(viashowAddButton), but the footer "Add blueprint" button at lines 63-72 remains visible regardless of the feature flag. Is this intentional to provide a fallback, or should this button also be hidden when theshared_blueprintsflag is disabled?Additionally, there's an indentation inconsistency here (3 tabs vs 4 tabs expected to match line 62).
124-126: LGTM!Good refactor to centralize the feature flag check. The optional chaining safely handles cases where
featureFlags.valuemight be null or undefined.
128-154: LGTM!The section configuration correctly implements the feature flag gating:
- Regular "Blueprints" section shows the add button only when the FF is enabled
- "Shared Blueprints" section (which only exists when FF is enabled) always shows its add button
This aligns with the PR objective to control the add button visibility via the feature flag.
|
✅ Graham C reviewed all the included code changes and associated automation findings and determined that there were no immediately actionable security flaws. Note that they will continue to be notified of any new commits or comments and follow up as needed throughout the duration of this pull request's lifecycle. Reviewed with ❤️ by PullRequest |
|
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard. |
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.