feat: Remove hardcoding of upcoming integrations from client codebase #40047 #40271
Conversation
WalkthroughThis change removes hardcoded definitions for upcoming integrations from the client codebase and migrates the logic to fetch these integrations from a backend API. It introduces a new Redux action, reducer state, and saga to manage upcoming plugins, adds a selector for accessing them, and updates components and utilities to use the dynamic list instead of static constants. The API layer is extended with a new method to retrieve upcoming integrations, and corresponding tests are added. All references to "premium" integrations are replaced with "upcoming" integrations, and the filtering logic is updated to operate on the API-driven data. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Redux
participant Saga
participant PluginsApi
participant Backend
UI->>Redux: dispatch(fetchUpcomingPlugins)
Redux->>Saga: GET_UPCOMING_PLUGINS_REQUEST
Saga->>PluginsApi: fetchUpcomingIntegrations()
PluginsApi->>Backend: GET /upcoming-integrations
Backend-->>PluginsApi: [UpcomingIntegration[]]
PluginsApi-->>Saga: Promise resolves with data
Saga->>Redux: dispatch(GET_UPCOMING_PLUGINS_SUCCESS)
Redux-->>UI: State updated with upcoming plugins list
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🪧 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
|
0e0fd40 to
844ca77
Compare
|
/ok-to-test |
|
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14492238539. |
|
Deploy-Preview-URL: https://ce-40271.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14511714648. |
|
Deploy-Preview-URL: https://ce-40271.dp.appsmith.com |
…pcoming-integrations-api
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
265-265: 🛠️ Refactor suggestionReferences to premiumPlugins should be updated
These lines still reference premiumPlugins when they should reference upcomingPlugins to maintain naming consistency throughout the codebase.
Update all occurrences with a consistent name:
- if ( - props.premiumPlugins.length === 0 && + if ( + props.upcomingPlugins.length === 0 && props.plugins.length === 0 && !props.restAPIVisible && !props.graphQLAPIVisible ) return null;And:
- {props.premiumPlugins.length > 0 && props.isIntegrationsEnabledForPaid ? ( + {props.upcomingPlugins.length > 0 && props.isIntegrationsEnabledForPaid ? ( <DatasourceSection id="upcoming-saas-integrations"> <DatasourceSectionHeading kind="heading-m"> {createMessage(UPCOMING_SAAS_INTEGRATIONS)} </DatasourceSectionHeading> <DatasourceContainer data-testid="upcoming-datasource-card-container"> <PremiumDatasources isIntegrationsEnabledForPaid - plugins={props.premiumPlugins} + plugins={props.upcomingPlugins} /> </DatasourceContainer> </DatasourceSection> ) : null}Also applies to: 283-292
🧹 Nitpick comments (5)
app/client/src/api/__tests__/PluginApi.test.ts (1)
1-74: Well-structured tests for new API methodGood implementation of tests for the
fetchUpcomingIntegrationsmethod. The tests verify both successful API calls and error handling.One minor improvement suggestion:
Consider refactoring the Api mock implementation to use simple functions instead of a class with static members:
- jest.mock("api/Api", () => { - return { - // Export a class that can be extended - __esModule: true, - default: class MockApi { - static get: jest.Mock = jest.fn(); - }, - }; - }); + jest.mock("api/Api", () => ({ + __esModule: true, + default: { + get: jest.fn() + } + }));This provides the same functionality with a simpler implementation that avoids the linter warning about classes with only static members.
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-12: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
app/client/src/sagas/PluginSagas.ts (1)
313-329: Consider adding response validation for upcoming plugins saga.The new saga correctly handles the API call and dispatches appropriate success/error actions. However, unlike other sagas in this file (like
getDefaultPluginsSaga), it doesn't use thevalidateResponsehelper to verify the API response.function* getUpcomingPluginsSaga() { try { const response: ApiResponse<UpcomingIntegration[]> = yield call( PluginsApi.fetchUpcomingIntegrations, ); + const isValid: boolean = yield validateResponse(response); - yield put({ + if (isValid) { + yield put({ type: ReduxActionTypes.GET_UPCOMING_PLUGINS_SUCCESS, payload: response.data || [], }); + } } catch (error) { yield put({ type: ReduxActionErrorTypes.GET_UPCOMING_PLUGINS_ERROR, payload: { error }, }); } }app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
236-238: Update PremiumDatasources component usageThis component is still named PremiumDatasources but works with upcoming plugins. This creates ambiguity.
Consider renaming the PremiumDatasources component to UpcomingDatasources for consistency, or update the property reference:
{!props.isIntegrationsEnabledForPaid && ( - <PremiumDatasources plugins={props.premiumPlugins} /> + <PremiumDatasources plugins={props.upcomingPlugins} /> )}app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts (2)
24-29: Consider renaming constants for consistencyThe file and function have been updated to use "upcoming" terminology, but constants like PREMIUM_INTEGRATION_CONTACT_FORM still use "premium". For naming consistency, consider updating these constants.
-export const PREMIUM_INTEGRATION_CONTACT_FORM = - "PREMIUM_INTEGRATION_CONTACT_FORM"; +export const UPCOMING_INTEGRATION_CONTACT_FORM = + "UPCOMING_INTEGRATION_CONTACT_FORM";
1-29: Inconsistent file namingThe file is still named "PremiumDatasources/Constants.ts" but primarily contains functionality for "Upcoming" integrations. Consider moving this file to a more appropriately named directory or updating the directory name.
Consider renaming the directory from "PremiumDatasources" to "UpcomingDatasources" to maintain naming consistency with the updated terminology.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/client/src/actions/pluginActions.ts(1 hunks)app/client/src/api/PluginApi.ts(3 hunks)app/client/src/api/__tests__/PluginApi.test.ts(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(1 hunks)app/client/src/ce/entities/Engine/actionHelpers.ts(2 hunks)app/client/src/ce/pages/Applications/CreateNewAppsOption.test.tsx(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts(1 hunks)app/client/src/entities/Plugin/index.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(5 hunks)app/client/src/pages/Editor/IntegrationEditor/EmptySearchedPlugins.tsx(3 hunks)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx(3 hunks)app/client/src/reducers/entityReducers/pluginsReducer.ts(4 hunks)app/client/src/sagas/PluginSagas.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
app/client/src/ce/selectors/entitiesSelector.ts (1)
app/client/src/ce/reducers/index.tsx (1)
AppState(98-191)
app/client/src/api/PluginApi.ts (2)
app/client/src/api/types.ts (1)
ApiResponse(14-18)app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)
app/client/src/ce/entities/Engine/actionHelpers.ts (1)
app/client/src/actions/pluginActions.ts (1)
fetchUpcomingPlugins(88-90)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (1)
app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (3)
app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)app/client/src/ce/selectors/entitiesSelector.ts (1)
getUpcomingPlugins(1803-1806)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts (1)
getFilteredUpcomingIntegrations(11-22)
app/client/src/pages/Editor/IntegrationEditor/EmptySearchedPlugins.tsx (2)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getUpcomingPlugins(1803-1806)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts (1)
getFilteredUpcomingIntegrations(11-22)
app/client/src/sagas/PluginSagas.ts (3)
app/client/src/api/types.ts (1)
ApiResponse(14-18)app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1282-1325)ReduxActionErrorTypes(1327-1354)
app/client/src/api/__tests__/PluginApi.test.ts (1)
app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts (1)
app/client/src/entities/Plugin/index.ts (1)
UpcomingIntegration(95-98)
🪛 Biome (1.9.4)
app/client/src/api/__tests__/PluginApi.test.ts
[error] 10-12: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-unit-tests / client-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-prettier / prettier-check
🔇 Additional comments (26)
app/client/src/entities/Plugin/index.ts (1)
95-98: Good addition of the UpcomingIntegration interface.The interface is well-defined with clear property names. This will provide proper type safety for the upcoming integrations feature across the application.
app/client/src/ce/pages/Applications/CreateNewAppsOption.test.tsx (1)
25-28: Test state properly updated for the new feature.The mock store now includes the
upcomingPluginsstate slice with appropriate default values, ensuring tests will work correctly with components that depend on this state.app/client/src/ce/selectors/entitiesSelector.ts (1)
1803-1806: Well-implemented selector for upcoming plugins.Good use of
createSelectorfor memoization, which will prevent unnecessary re-renders when the state is accessed multiple times without changes. The selector correctly extracts just the list from the state object.app/client/src/actions/pluginActions.ts (1)
88-90: Action creator properly implemented.The
fetchUpcomingPluginsaction creator follows the established pattern in this file, creating an action without payload that will trigger the request to fetch upcoming plugins.app/client/src/ce/entities/Engine/actionHelpers.ts (2)
11-11: Added import for fetchUpcomingPlugins actionThe import statement has been updated to include the
fetchUpcomingPluginsaction creator alongside the existingfetchPluginsfunction.
40-40: Added upcoming plugins fetch to initialization flowThe
fetchUpcomingPlugins()action has been added to the initialization actions array. Good that you've included a comment explaining why it doesn't have corresponding success and error actions in the arrays below.app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (3)
10-10: Switched to centralized interface definitionChanged from using a locally defined
PremiumIntegrationtype to the centralizedUpcomingIntegrationinterface from the entities module.
29-29: Updated prop type to use UpcomingIntegrationThe props type has been updated to use the imported
UpcomingIntegrationinterface, maintaining type safety after the refactoring.
52-52: Updated property access for icon URLChanged from using
integration.icontointegration.iconLocationto match the property name in the newUpcomingIntegrationinterface.app/client/src/ce/constants/ReduxActionConstants.tsx (2)
754-755: Added action types for upcoming plugins request and successAdded the necessary Redux action types for requesting and successfully fetching upcoming plugins.
762-762: Added error action type for upcoming pluginsAdded the Redux error action type for handling failures when fetching upcoming plugins.
app/client/src/pages/Editor/IntegrationEditor/EmptySearchedPlugins.tsx (1)
13-14: Implementation correctly uses dynamic upcoming plugins.The changes appropriately migrate from hardcoded premium integrations to dynamically fetched upcoming integrations:
- Updated imports to use
getUpcomingPluginsselector andgetFilteredUpcomingIntegrationsfunction- Added Redux selector hook to retrieve upcoming plugins from state
- Updated the filter function call to use the dynamically fetched plugins
This aligns with the PR objective to centralize transformation logic and remove hardcoded values.
Also applies to: 37-38, 64-68
app/client/src/api/PluginApi.ts (3)
6-11: Import statements correctly organized.Imports have been properly structured to include the
UpcomingIntegrationtype and theobjectKeysutility function which will be used in the file.
63-67: Well-implemented API method for fetching upcoming integrations.The new
fetchUpcomingIntegrationsmethod follows the existing API pattern and structure:
- Returns appropriate Promise type with
UpcomingIntegration[]as the generic parameter- Uses the correct endpoint path format consistent with other API methods
- Properly leverages the existing API utility methods
This implementation effectively supports the PR goal of moving integration data fetching to the API layer.
84-86: Improved object key iteration with utility function.Replacing
Object.keyswith theobjectKeysutility function provides more consistent behavior across environments and better type safety.app/client/src/sagas/PluginSagas.ts (2)
34-39: Import statements correctly updated.The imports are properly organized to include the
UpcomingIntegrationtype needed for the new saga functionality.
346-349: Correctly wired up saga to action type.The saga is properly connected to the Redux action type using
takeEvery, consistent with the pattern used for other sagas in this file.app/client/src/reducers/entityReducers/pluginsReducer.ts (2)
7-11: Import statements correctly updated.The imports are properly organized to include the
UpcomingIntegrationtype needed for the state interface.
37-40: Well-structured state for upcoming plugins.The state structure for upcoming plugins follows the existing patterns in the reducer:
- Properly typed with
UpcomingIntegration[]for the list- Includes loading state for tracking API call status
- Initial state correctly initialized with empty array and loading flag set to false
This implementation provides a solid foundation for managing upcoming plugins in the Redux store.
Also applies to: 54-57
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (4)
14-14: Type import updated to UpcomingIntegrationThe type import has been updated from PremiumIntegration to UpcomingIntegration, aligning with the new naming convention throughout the application.
20-20: Selector updated to getUpcomingPluginsThe selector import has been updated from a premium plugins selector to getUpcomingPlugins, maintaining consistency with the type renaming.
48-48: Updated utility import to getFilteredUpcomingIntegrationsThe import has been updated to use getFilteredUpcomingIntegrations instead of the previous premium integration filtering utility.
312-313: Updated to use the new upcomingPlugins selectorThe code now correctly retrieves upcoming plugins from the Redux state using the getUpcomingPlugins selector.
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts (3)
1-1: Updated import to UpcomingIntegrationThe import has been updated from PremiumIntegration to UpcomingIntegration, aligning with the new naming convention.
3-10: Added clear JSDoc documentationGood addition of comprehensive JSDoc comments explaining the function's purpose, parameters, and return value.
6D40
11-22: Refactored integration filtering with dynamic data sourceThe function has been updated to accept upcomingPlugins as a parameter instead of using a hardcoded array. This is a good improvement that:
- Removes hardcoded values
- Makes the function more testable
- Centralizes the filtering logic
The filtering logic itself remains sound - it filters out plugins that are already installed when external SaaS is enabled.
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx
Outdated
Show resolved
Hide resolved
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx
Outdated
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14517298286. |
|
Deploy-Preview-URL: https://ce-40271.dp.appsmith.com |
Premium Integrations Refactoring
Problem
The existing implementation for handling premium integrations had several issues:
Constants.tsandPluginApi.tsPREMIUM_INTEGRATIONSwas a mutable variable instead of a constantSolution
This PR refactors the premium integrations handling to:
PluginApi.ts)PREMIUM_INTEGRATIONSto a constant array with proper update methodsKey Changes
In
app/client/src/api/PluginApi.ts:PremiumIntegrationinterface to define the structure for premium integrationsfetchPremiumIntegrations()that:PremiumIntegrationformatIn
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/Constants.ts:DEFAULT_PREMIUM_INTEGRATIONS)PREMIUM_INTEGRATIONSfrom a variable to a constant array.length = 0and.push()instead of reassignment)Testing
The changes maintain the existing functionality while improving code organization:
PREMIUM_INTEGRATIONSarray is still populated from the APIgetFilteredPremiumIntegrationsfunction works as beforeImpact Areas
Type of Change
Further Improvements
In the future, we could consider:
Fixes #40047
/ok-to-test tags="@tag.All"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14516648891
Commit: 79bd56d
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 17 Apr 2025 14:38:39 UTC
Summary by CodeRabbit