-
Notifications
You must be signed in to change notification settings - Fork 4.1k
chore: filter out AI Datasource during import #40286
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
WalkthroughA constant named Changes
Suggested labels
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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 (2)
app/client/src/ce/sagas/ApplicationSagas.tsx (2)
752-768
: Good implementation of AI Datasource filtering in forkApplicationSaga.The code correctly filters out AI Datasource entries from the unconfigured datasource list before showing the reconnect modal. The null/undefined check with fallback to empty array is a good defensive practice.
Consider extracting this filtering logic into a utility function since it's repeated in multiple places:
+// At the module level +const filterOutAIDatasources = (datasources: Datasource[] = []): Datasource[] => { + return datasources.filter(datasource => datasource.name !== AI_DATASOURCE_NAME); +}; // Then in the saga -const filteredUnConfiguredDatasourceList = ( - response?.data?.unConfiguredDatasourceList || [] -).filter( - (datasource) => datasource.name !== AI_DATASOURCE_NAME, -) as Datasource[]; +const filteredUnConfiguredDatasourceList = filterOutAIDatasources( + response?.data?.unConfiguredDatasourceList +);
838-851
: Good implementation of AI Datasource filtering in importApplicationSaga.The code correctly filters the unconfigured datasource list and uses the filtered list when showing the reconnect modal. The implementation is consistent with the approach used in forkApplicationSaga.
As mentioned earlier, consider extracting this filtering logic into a shared utility function to reduce duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/ce/sagas/ApplicationSagas.tsx
(5 hunks)app/client/src/sagas/TemplatesSagas.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/client/src/sagas/TemplatesSagas.ts (1)
app/client/src/ce/actions/applicationActions.ts (1)
showReconnectDatasourceModal
(275-283)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (5)
app/client/src/sagas/TemplatesSagas.ts (2)
51-51
: Good addition of a constant for better maintainability.Adding this constant
AI_DATASOURCE_NAME
helps maintain consistency across the codebase and makes future changes to the name easier to implement.
100-114
: Effective implementation of the AI Datasource filtering logic.The code successfully filters out the AI Datasource entries before displaying the reconnect datasource modal. The logic is sound:
- Filter out datasources named "AI Datasource"
- Only show the modal if it's a partial import and there are remaining unconfigured datasources
- Pass the filtered list to the modal component
The null check with the fallback to an empty array (
response.data.unConfiguredDatasourceList || []
) provides good defensive programming.app/client/src/ce/sagas/ApplicationSagas.tsx (3)
127-127
: Good addition of the AI_DATASOURCE_NAME constant.This matches the constant in TemplatesSagas.ts and ensures consistent filtering across the application.
811-815
: Good type annotations for the API response.Adding explicit typing for the API response improves code clarity and helps catch potential type errors during development.
864-865
: TypeScript error suppression properly documented.The
@ts-expect-error
directive is properly documented explaining why the error suppression is needed.
Before:
CleanShot.2025-04-17.at.11.57.06.mp4
After:
CleanShot.2025-04-17.at.11.57.30.mp4
/ok-to-test tags="@tag.Templates"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14509874809
Commit: a3d4aa2
Cypress dashboard.
Tags:
@tag.Templates
Spec:
Thu, 17 Apr 2025 07:16:52 UTC
Summary by CodeRabbit