feat: update client logic for DS form #24474
Conversation
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/5319146695. |
|
Deploy-Preview-URL: https://ce-24474.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true recreate=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/5320027081. |
|
Deploy-Preview-URL: https://ce-24474.dp.appsmith.com |
|
/ok-to-test |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5320856349. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5320856349.
|
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5320856349.
|
|
/build-deploy-preview skip-tests=true recreate=true |
|
/ok-to-test |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/5330162577. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5330163410. |
|
Deploy-Preview-URL: https://ce-24474.dp.appsmith.com |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5330163410. |
ayushpahwa
changed the title
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5330163410. |
|
/build-deploy-preview skip-tests=true recreate=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/5331142924. |
|
Closing since fix is to be done with #23920 |
| @@ -192,7 +193,7 @@ class DatasourceEditorRouter extends React.Component<Props, State> { | |||
| requiredFields: {}, | |||
| configDetails: {}, | |||
| filterParams: { | |||
| id: "", | |||
| id: DEFAULT_ENV_ID, | |||
There was a problem hiding this comment.
Why is the envId added within the filterParams? And if this is the best place to put it, can we rename the id to envId
| @@ -460,8 +464,8 @@ class DatasourceSaaSEditor extends JSONtoForm<Props, State> { | |||
| pageId={pageId} | |||
| /> | |||
| ) : null} | |||
| {!_.isNil(sections) | |||
| ? _.map(sections, this.renderMainSection) | |||
| {!isNil(sections) | |||
There was a problem hiding this comment.
why is isNil preferred over isEmpty? If posible can we use isEmpty
| @@ -474,17 +475,25 @@ function ReconnectDatasourceModal() { | |||
| useEffect(() => { | |||
| if (isModalOpen && !isTesting) { | |||
| const id = selectedDatasourceId; | |||
| const pending = datasources.filter((ds: Datasource) => !ds.isConfigured); | |||
| const pending = datasources.filter((ds: Datasource) => | |||
| ds.datasourceStorages ? !isEnvironmentConfigured(ds) : true, | |||
There was a problem hiding this comment.
the condition here could be better since ds.datasourceStorages could be an empty Object which is still truthy. If it is fine, then please ignore
|
|
||
| return datasource; | ||
| }), | ||
| recentDatasources: [ |
There was a problem hiding this comment.
What are recentDatasources?
| if (!!queryParams && queryParams.hasOwnProperty(ENVIRONMENT_QUERY_KEY)) { | ||
| return queryParams[ENVIRONMENT_QUERY_KEY].toLowerCase(); | ||
| } | ||
| return "unused_env"; |
There was a problem hiding this comment.
Can we create an enum/constant for this variable?
Description
PR fixes following issue(s)
Fixes # (issue number)
Media
Type of change
Testing
How Has This Been Tested?
Test Plan
Issues raised during DP testing
Checklist:
Dev activity
QA activity:
Test Plan Approvedlabel after Cypress tests were reviewedTest Plan Approvedlabel after JUnit tests were reviewed