-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Expose iif and beta QBD #1131
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/integrations/landing/landing.component.html
(1 hunks)src/app/integrations/landing/landing.component.ts
(1 hunks)
🔇 Additional comments (3)
src/app/integrations/landing/landing.component.html (3)
35-35
: LGTM! Improved responsive layout.
The addition of flex-wrap and gap utilities ensures consistent spacing and better responsive behavior for integration cards.
37-89
: LGTM! Consistent styling across integration cards.
The implementation maintains consistent:
- Vertical spacing (tw-mt-20-px)
- Shadow classes based on brand
- Image dimensions and positioning
62-75
: Verify feature flag configuration for QBD integrations.
The implementation correctly separates QBD integrations into IIF and Web Connector (Beta) options. However, we should verify the feature flag setup.
Let's check the feature flag configuration:
✅ Verification successful
Let me gather more information about the feature flag configuration.
Let me try a different search pattern to find the feature flag usage.
Let me try one more search to find the organization model and feature flag definition.
Let me try a different search pattern to find the org model and feature flag.
Based on the search results, I can now provide a final response about the feature flag configuration.
Feature flag configuration for QBD integrations is correctly implemented
The feature flags are properly defined in the organization model:
allow_qbd_direct_integration
is a boolean flag in the Org interface that controls access to QBD Direct (Web Connector) integration- The conditional rendering in the template uses both
exposeApps.QBD
andorg.allow_qbd_direct_integration
appropriately to show/hide respective QBD integration cards
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify QBD feature flag configuration
# Search for QBD feature flag definitions
rg "QBD|QBD_DIRECT" --type ts
# Search for allow_qbd_direct_integration usage
rg "allow_qbd_direct_integration" --type ts
Length of output: 102251
Script:
#!/bin/bash
# Search for allow_qbd_direct_integration in models and interfaces
rg -t ts "interface.*\{[^}]*allow_qbd_direct_integration" --multiline
# Search for allow_qbd_direct_integration initialization
rg -t ts "allow_qbd_direct_integration.*=.*" -B 2 -A 2
Length of output: 128
Script:
#!/bin/bash
# Search for org.allow_qbd_direct_integration usage
rg -t ts "org.*allow_qbd_direct_integration"
# Search for allow_qbd_direct_integration in interfaces/types
rg -t ts "type|interface.*allow_qbd_direct_integration"
# Search for files containing both org and allow_qbd_direct_integration
rg -l "org" --type ts | xargs rg "allow_qbd_direct_integration"
Length of output: 66646
Script:
#!/bin/bash
# Search for org model interface/type definition
rg -A 10 "interface.*Org.*{|type.*Org.*=" --type ts
# Search for org property in component files
rg -B 3 -A 3 "@Input.*org" --type ts
Length of output: 222
Script:
#!/bin/bash
# Search for org interface definition
rg "interface Org" --type ts -A 10
# Search for allow_qbd_direct_integration in org model and components
rg "allow_qbd_direct_integration" -B 5 -A 5 --type ts
Length of output: 1495
@@ -72,7 +72,7 @@ export class LandingComponent implements OnInit { | |||
|
|||
readonly brandingConfig = brandingConfig; | |||
|
|||
readonly isINCluster = this.storageService.get('cluster-domain').includes('in1'); | |||
readonly isINCluster = this.storageService.get('cluster-domain')?.includes('in1'); |
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.
💡 Codebase verification
Inconsistent null-checking patterns found for 'cluster-domain' access
The codebase shows inconsistent handling of potential null/undefined values when accessing 'cluster-domain':
helper.service.ts
: Uses fallback with||
operatorlogin.component.ts
: Uses optional chaining for reading (similar to the reviewed change)auth.service.ts
: Direct removal without checks (safe operation)login.component.ts
: Direct setting without checks (safe operation)
Consider standardizing the null-checking approach across the codebase:
src/app/core/services/common/helper.service.ts
: Consider using optional chaining for consistency
🔗 Analysis chain
LGTM! Defensive programming improvement.
The addition of optional chaining operator prevents potential runtime errors when accessing the cluster domain.
Let's verify the cluster-domain storage key usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other usages of cluster-domain to ensure consistent null checking
# Search for cluster-domain usage
rg "cluster-domain" --type ts
Length of output: 647
Description
Please add PR description here, add screenshots if needed
Clickup
https://app.clickup.com/
Summary by CodeRabbit
New Features
Bug Fixes
isINCluster
property to prevent runtime errors.Style