-
Notifications
You must be signed in to change notification settings - Fork 4.1k
chore: Adding AI agent feature flag check for audit logs upgrade page #40185
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 pull request updates dynamic messaging in the application by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant AuditPage as AuditLogsUpgradePage
participant Redux as Redux Store
participant Selector as getIsAiAgentFlowEnabled
participant Footer as Footer Component
AuditPage->>Redux: useSelector(getIsAiAgentFlowEnabled)
Redux-->>AuditPage: isAiAgentFlowEnabled (true/false)
AuditPage->>Footer: Set footer message ("enterprise" if true, "business" if false)
Footer-->>AuditPage: Render dynamic footer message
Suggested labels
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
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)
app/client/src/ce/pages/Upgrade/AuditLogsUpgradePage.tsx (1)
83-88
: Dynamic messaging based on feature flag is well-implemented.The implementation correctly passes the appropriate plan name to the message function based on the feature flag, and sets the
isEnterprise
property accordingly.One minor suggestion: The boolean expression
isAiAgentFlowEnabled ? true : false
could be simplified to justisAiAgentFlowEnabled
.- isEnterprise: isAiAgentFlowEnabled ? true : false, + isEnterprise: isAiAgentFlowEnabled,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/ce/constants/messages.ts
(2 hunks)app/client/src/ce/pages/AdminSettings/config/auditlogs.ts
(2 hunks)app/client/src/ce/pages/Upgrade/AuditLogsUpgradePage.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (5)
app/client/src/ce/pages/AdminSettings/config/auditlogs.ts (2)
8-11
: Good implementation of Redux selector for feature flag.The import of Redux store and the AI agent selector is well-structured. This allows for a centralized feature flag check across components.
22-22
: Conditional property based on feature flag looks good.The
isEnterprise
flag is properly set based on the AI agent flow state, using a clean ternary expression.app/client/src/ce/constants/messages.ts (1)
1514-1518
: Good function parameter enhancement with backward compatibility.The
EXCLUSIVE_TO_BUSINESS
function has been appropriately updated to accept aplanName
parameter, with a default value of "business" to maintain backward compatibility with existing code.app/client/src/ce/pages/Upgrade/AuditLogsUpgradePage.tsx (2)
22-23
: Good implementation of Redux hooks for feature flag.Clean implementation of the necessary imports for Redux state management.
32-32
: Correctly using selector hook to access feature flag.The
useSelector
hook is properly used to access the AI agent feature flag state.
…#40185) ## Description Adding AI agent feature flag check for audit logs upgrade page Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Settings" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!IMPORTANT] > 🟣 🟣 🟣 Your tests are running. > Tests running at: <https://github.com/appsmithorg/appsmith/actions/runs/14350813652> > Commit: 89483c5 > Workflow: `PR Automation test suite` > Tags: `@tag.Settings` > Spec: `` > <hr>Wed, 09 Apr 2025 06:49:26 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Upgrade messaging now dynamically reflects the selected plan—displaying "business" or "enterprise" based on current settings. - Enhanced audit log configurations incorporate an enterprise flag to adjust messaging contextually. - **Bug Fixes** - Corrected text capitalization for clearer plan description in the upgrade footer. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…appsmithorg#40185) ## Description Adding AI agent feature flag check for audit logs upgrade page Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Settings" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!IMPORTANT] > 🟣 🟣 🟣 Your tests are running. > Tests running at: <https://github.com/appsmithorg/appsmith/actions/runs/14350813652> > Commit: 89483c5 > Workflow: `PR Automation test suite` > Tags: `@tag.Settings` > Spec: `` > <hr>Wed, 09 Apr 2025 06:49:26 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Upgrade messaging now dynamically reflects the selected plan—displaying "business" or "enterprise" based on current settings. - Enhanced audit log configurations incorporate an enterprise flag to adjust messaging contextually. - **Bug Fixes** - Corrected text capitalization for clearer plan description in the upgrade footer. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Adding AI agent feature flag check for audit logs upgrade page
Fixes #
Issue Number
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Settings"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14350813652
Commit: 89483c5
Cypress dashboard.
Tags:
@tag.Settings
Spec:
Wed, 09 Apr 2025 07:12:00 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit