feat: paragon frontend ds creation implementation added #38456
Conversation
WalkthroughThis pull request introduces a comprehensive update to the datasource management system, focusing on expanding support for external SaaS integrations. The changes involve modifying multiple files across the application to add a new Changes
Sequence DiagramsequenceDiagram
participant User
participant ActionPane
participant DatasourceSaga
participant ExternalSaasPlugin
User->>ActionPane: Select External SaaS Integration
ActionPane->>DatasourceSaga: Initiate Datasource Creation
DatasourceSaga->>ExternalSaasPlugin: Create/Authorize Datasource
ExternalSaasPlugin-->>DatasourceSaga: Return Datasource Configuration
DatasourceSaga-->>ActionPane: Update UI with New Datasource
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
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
Documentation and Community
|
|
/build-deploy-preview skip-tests=true |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12583609816. |
|
Deploy-Preview-URL: https://ce-38456.dp.appsmith.com |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
app/client/src/ce/sagas/DatasourcesSagas.ts (16)
2-2: Check imports scope.
Confirm each imported symbol is utilized and prune any unused references.
92-92: Check for potential duplication.
createActionRequestSagais imported here; if used repeatedly, see if a single entry point might suffice.
93-93: Validate usage ofvalidateResponse.
Consider verifying whether multiple calls stack up, or if there's a single orchestrator method that can unify it.
169-170: Focus retention logic.
The saga references focus/retention utilities. Confirm no race conditions occur when focus or navigation is updated.
217-217: Large try block.
handleFetchDatasourceStructureOnLoadis quite dense. Consider refactoring for clarity and better error separation.
520-520: Connection method logic.
getConnectionMethodmight be re-usable elsewhere. Extracting it could simplify the saga if repeated often.
881-881: Refactor large code path.
testDatasourceSagacould benefit from a smaller, dedicated success/failure helper to improve clarity.
887-887: Improve error logging.
Currently it’s a basic error assignment. Consider more verbose messaging or unique error codes.
1431-1431: Possible refactor for “storeAsDatasourceSaga”.
This logic lumps multiple transformations. Slight decomposition could enhance readability.
1547-1547: Unify fetch and refresh.
fetchDatasourceStructureSagahas many parallels withrefreshDatasourceStructure. Extract shared code if feasible.
1666-1666: Add & fetch structure after mock creation.
Check that environment details are properly set or defaulted before fetching structure.
1794-1794: Dual usage inexecuteDatasourceQuerySaga.
Handling generate-page vs. normal queries in one saga can grow complex. Keep an eye on maintainability.
1973-1973: Potential large result sets.
fetchGsheetSpreadhsheetsshould account for multiple or nested sheets. Ensure performance remains acceptable.
2121-2121: Common logic for columns fetch.
fetchGsheetColumnsis similar to previous calls. Factor out common code for simpler maintenance.
2173-2173: Relying on window variable.
loadFilePickerSagacheckswindow.googleAPIsLoaded. Consider a more robust or safer detection approach.
2197-2197: Consistent success messaging.
updateDatasourceAuthStateSagatoasts success or failure distinctly. Confirm a uniform pattern for user-facing messages.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/client/src/ce/sagas/DatasourcesSagas.ts(30 hunks)app/client/src/ce/sagas/index.tsx(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts(1 hunks)app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx(4 hunks)app/client/src/constants/AppsmithActionConstants/formConfig/ApiDatasourceFormsButtonConfig.ts(1 hunks)app/client/src/ee/sagas/DatasourcesSagas.ts(1 hunks)app/client/src/entities/Action/index.ts(2 hunks)app/client/src/entities/Datasource/index.ts(2 hunks)app/client/src/entities/JSCollection/index.ts(1 hunks)app/client/src/pages/Editor/EntityNavigation/ActionPane/index.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/NewApi.tsx(1 hunks)app/client/src/sagas/WidgetBlueprintSagas.ts(1 hunks)app/client/test/sagas.ts(1 hunks)
🔇 Additional comments (42)
app/client/src/ee/sagas/DatasourcesSagas.ts (4)
1-6: Use consistent import referencesThe import for
createOrUpdateDataSourceWithActionreferences"ce/sagas/DatasourcesSagas"while most watchers are in theeefolder. Ensure this is intentional, or consider consolidating imports for consistent readability.
7-40: Confirm the ReduxActionConstants importYou appear to be using the
ee/constants/ReduxActionConstantsfor action types. Verify that it matches the structure ince/constants/ReduxActionConstantsif there's any overlap. Duplicated or mismatched enums can cause confusion.
42-100: Appropriate use of takeEvery and takeLatestEach saga uses the correct effect pattern (
takeEveryvs.takeLatest) for its intended behavior. This is a good practice to handle concurrency in side effects.
100-140: Overall concurrency approachAll watchers run concurrently using
yield all([...]). This is standard and should manage side effects well. No issues spotted.app/client/src/constants/AppsmithActionConstants/formConfig/ApiDatasourceFormsButtonConfig.ts (1)
7-7: Consistent naming across plugin types
EXTERNAL_SAASis introduced with["CANCEL", "SAVE_AND_AUTHORIZE"]. Ensure other configurations in the code adhere to the same naming.app/client/src/pages/Editor/EntityNavigation/ActionPane/index.ts (1)
23-23: Extended switch case for EXTERNAL_SAASGood approach. The new
EXTERNAL_SAAScase extends existing logic without duplication.app/client/src/entities/JSCollection/index.ts (1)
1-1: Updated import pathThe path for
BaseActionnow references"../../entities/Action". If there are no unintended import cycles, this looks fine.app/client/test/sagas.ts (1)
6-6: Referring to ee/sagas/DatasourcesSagasSwitching to
ee/sagas/DatasourcesSagasaligns with the new file location. Confirm no leftover references to the old path.app/client/src/ce/sagas/index.tsx (1)
19-19: Migrating to Enterprise sagas
The change from"sagas/DatasourcesSagas"to"ee/sagas/DatasourcesSagas"looks valid. Confirm that related references are updated accordingly.app/client/src/entities/Datasource/index.ts (3)
143-143: Union Type for authentication
AllowingExternalSaasDSAuthenticationalongside the existing type seems correct, but ensure that downstream references properly handle this union.
209-214: New ExternalSaasDSAuthentication interface
This interface extends the base authentication model. Confirm thatintegrationId,credentialId, andintegrationTypeare used consistently throughout the system.
216-218: Additional AuthenticationType enum
AddingEXTERNAL_SAAS_AUTHENTICATIONmaintains clarity. Ensure it aligns with existing references to avoid confusion.app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx (4)
180-180: EXTERNAL_SAAS in defaultActionSettings
The addition mirrors other plugin types. Looks consistent with overall usage patterns.
193-193: Empty editor config for EXTERNAL_SAAS
Having no default editor config is acceptable if you plan to provide one later. Keep an eye out for usage that expects a non-empty array.
207-207: No dependencies config
Declaring an empty dependencies object is perfectly fine if no additional dependencies are required.
218-218: Datasource form button config
IncludingEXTERNAL_SAAShere keeps the plugin coverage consistent. Good move.app/client/src/entities/Action/index.ts (3)
18-18: New plugin type for EXTERNAL_SAAS
IntroducingEXTERNAL_SAASexpands the range of plugin integrations.
264-270: ExternalSaasAction interface
ExtendingBaseActionwithPluginType.EXTERNAL_SAASand adatasourceensures clarity for external SaaS flows.
278-279: Union support for the new ExternalSaasAction
This addition enables typed usage throughout the codebase forEXTERNAL_SAASactions.app/client/src/sagas/WidgetBlueprintSagas.ts (1)
21-21: Confirm correct import path.
Ensure the new relative path to theDatasourcesSagasfile is accurate and thatcreateOrUpdateDataSourceWithActionis indeed exported there.app/client/src/pages/Editor/IntegrationEditor/NewApi.tsx (1)
253-255: Extended plugin type match is appropriate.
AddingPluginType.EXTERNAL_SAASto the filter ensures compatibility with additional external sources. Confirm there's adequate test coverage for the new plugin type.app/client/src/ce/selectors/entitiesSelector.ts (1)
122-123: Inclusion ofEXTERNAL_SAASis consistent.
This maintains uniformity in how SaaS-like plugin types are categorized. Make sure it aligns with the rest of the UI logic.app/client/src/ce/sagas/DatasourcesSagas.ts (20)
Line range hint
21-21: Validate module sub-import usage.
Ensure referencingee/selectors/entitiesSelectormeets the new folder structure.
90-90: Confirm side-effect usage.
InvokingsetIdeEditorViewModecan affect global view state; verify correctness in related flows.
176-177: Check fallback request approach.
getFromServerWhenNoPrefetchedResultcould obscure failure details; consider logging or tracking fallback usage.
181-181: Import usage check.
openGeneratePageModalWithSelectedDSis invoked subsequently. Confirm it’s tested in relevant user flows.
183-183: Ensure robust error handling.
fetchDatasourcesSagashould maintain consistent fallback logic if an API call fails.
250-250: Check partial/invalidmockDatasources.
Ensure unexpected data from the server is handled gracefully.
696-696: Redirect logic for various plugin types.
redirectAuthorizationCodeSagacheckspluginType. Confirm fallback scenarios for unknown types.
749-749: Local storage reliance.
IngetOAuthAccessTokenSaga, handle missing or tampered tokens gracefully, ensuring robust user feedback.
846-846: Non-blocking name update.
updateDatasourceNameSagais straightforward. Confirm concurrency is managed if multiple updates occur rapidly.
1045-1045: Check plugin form config availability.
createTempDatasourceFromFormSagamerges with config. Confirm plugin form retrieval is always completed beforehand.
1161-1165: Validate environment integrity.
When merging initial values and environment data increateDatasourceFromFormSaga, ensure environment keys are correct and that partial merges don’t break defaults.
1319-1319: Skip redirect logic.
The conditionshouldNotRedirectcan be impactful. Confirm we have coverage in tests for cases that skip redirect.
1378-1378: Logic forswitchDatasourceSaga.
We simply dispatch achangeDatasource. This looks lean. Confirm it's tested for partial data scenarios.
1392-1392: Replay entity consistency.
For partial form changes informValueChangeSaga, ensure the replay state remains in sync with the rest of the store.
1505-1507: Update success logic.
updateDatasourceSuccessSagatransitions the editor state on success. The flow is correct as is.
1848-1848: Check defaultKeyValue logic.
initializeFormWithDefaultssets default map arrays. Confirm it aligns with all plugin usage when form loads.
1886-1886: Validate klonaLite usage.
filePickerActionCallbackSagaclones and modifies datasource state. Check if certain fields need sanitization or deeper checks.
2073-2073: Error detail enrichment.
fetchGsheetSheetscould provide more context if the request fails, especially for large data sets.
2248-2248: Clear usage of analytics.
datasourceDiscardActionSagalogs correct event. No further concerns here.
2268-2268: Banner reset approach.
setDatasourceViewModeSagaresets banner for the datasource. Straightforward solution.
| @@ -1697,7 +1693,7 @@ function* addAndFetchDatasourceStructureSaga( | |||
| } | |||
| } | |||
|
|
|||
| function* refreshDatasourceStructure( | |||
| export function* refreshDatasourceStructure( | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplication in refreshDatasourceStructure.
Many lines duplicate fetchDatasourceStructureSaga. Merge if possible for easier maintenance.
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
sneha122
added
ok-to-test
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/client/src/ce/sagas/DatasourcesSagas.ts (4)
217-217: Potential enhancement on parallelizing structure fetch.
You're using a for-loop to dispatch multiplefetchDatasourceStructure. Consider bundling them withyield all(...)for concurrent performance, if safe.
520-520: Function is quite large.
Consider modularizing logic (e.g., environment updates, plugin checks, analytics) for readability and simpler testing.
888-888: Test datasource saga logic.
Function is somewhat lengthy. Consider extracting repeated patterns into helper functions to keep it DRY.
2198-2198: Auth update saga.
Nice to see a success/error toast. Validate concurrency if multiple updates may overlap.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/ce/sagas/DatasourcesSagas.ts(30 hunks)app/client/src/ce/sagas/index.tsx(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/ce/selectors/entitiesSelector.ts
- app/client/src/ce/sagas/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- 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 (29)
app/client/src/ce/sagas/DatasourcesSagas.ts (29)
2-2: No issues with the new import statement.
Looks straightforward and aligns with typical Saga usage.
90-90: Import is valid and put to good use.
No concerns.
92-92: Import introduced for action creation.
Seems properly referenced in subsequent code.
169-170: New imports from FocusRetentionSaga and FocusEntity look fine.
No issues with maintainability.
176-176: Helper import is used appropriately.
Integration with saga logic is consistent.
181-181: Utilities import for generating pages is valid.
No immediate suggestions.
183-183: Good use of generator function for fetching datasources.
Error handling and success dispatch appear solid.
250-250: Mock datasources fetching logic looks fine.
Handles both cached and uncached scenario.
847-847: Update datasource name saga is straightforward.
No concerns; error handling is in place.
882-882: Failure saga correctly reverts the name to the old value.
Simple and reliable.
1046-1046: Creation of a temporary datasource.
Implementation flow is sound. Potentially add unit tests to cover edge cases.
1162-1166: Datasource creation saga.
Logic is robust. Keep an eye on environment-level concurrency if multiple calls happen in parallel.
1320-1320: Good approach for changing datasource references.
Nicely uses redux-form initialization. No issues.
1379-1379: Switch between datasources with minimal overhead.
No concerns.
1393-1393: Form value change saga is well-structured.
Limits side effects to recognized forms. Implementation is tidy.
1432-1432: Segregating REST API and datasource headers is logical.
Implementation is consistent.
1506-1508: updateDatasourceSuccessSaga.
Handles success scenario and triggers relevant actions. No immediate issues.
1548-1548: fetchDatasourceStructureSaga.
Error handling and analytics usage are appropriate. Code is good to go.
1667-1667: Mock datasource + fetch structure.
Logic is linear and straightforward. No concerns.
1697-1697: Duplication inrefreshDatasourceStructure.
This repeats logic fromfetchDatasourceStructureSaga. Merging could enhance maintainability.
1795-1795: Datasource query execution.
Clear flow, robust error handling. Looks solid.
1849-1849: Initializing form with defaults.
Valid approach for form controls.
1887-1887: Sensitive data checks.
Resets token upon file selection or cancellation. Good practice to minimize stale data.
1974-1974: Fetching GSheet spreadsheets logic.
No major issues, consistent usage of Saga pattern.
2074-2074: Fetching GSheet sheets.
Implementation mirrors the standard approach. No concerns evident.
2122-2122: Fetching GSheet columns step.
Implementation in line with the other GSheet sagas.
2174-2174: Load file picker and handle overlay toggling.
No red flags, environment checks are sound.
2249-2249: Discarding datasource changes.
Analytics event is a helpful addition. Implementation is simple.
2269-2269: Setting datasource view mode.
Correct approach for toggling modes.
| @@ -703,7 +693,7 @@ function* updateDatasourceSaga( | |||
| } | |||
| } | |||
|
|
|||
| function* redirectAuthorizationCodeSaga( | |||
| export function* redirectAuthorizationCodeSaga( | |||
There was a problem hiding this comment.
Security caution with storing tokens in localStorage.
Consider using session storage or a secure HTTP-only cookie to reduce XSS risks.
| @@ -757,7 +747,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| function* getOAuthAccessTokenSaga( | |||
| export function* getOAuthAccessTokenSaga( | |||
There was a problem hiding this comment.
Token retrieval logic might be vulnerable to localStorage-based attacks.
Review the security model to ensure it meets your threat requirements.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12628292949. |
|
Deploy-Preview-URL: https://ce-38456.dp.appsmith.com |
Description
This PR adds CE changes for the paragon integrations, with this whenever anybody is using CE version, paragon integrations will be visible but once they click on it, it will ask for them to put in email id and request access.
It's counterpart EE PR handles the paragon integration creation and authorisation in appsmith.
EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5859
EE PR which has both CE and EE changes to ensure all things are working smoothly: https://github.com/appsmithorg/appsmith-ee/pull/5866
Fixes #
38406or
Fixes
Issue URLWarning
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.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12627633263
Commit: 9c1e06b
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Mon, 06 Jan 2025 07:10:57 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Code Improvements