codeant-ai-for-open-source[bot] commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3481934122
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -2345,299 +2359,289 @@ class DatasourceEditor extends PureComponent<
/>
</div>
);
- }
+ }, [datasource, sortMetrics, onDatasourcePropChange, metricSearchTerm]);
- render() {
- const { datasource, activeTabKey } = this.state;
- const { metrics } = datasource;
- const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : [];
+ const sortedMetrics = useMemo(
+ () => (datasource.metrics?.length ? sortMetrics(datasource.metrics) : []),
+ [datasource.metrics, sortMetrics],
+ );
- return (
- <DatasourceContainer data-test="datasource-editor">
- {this.renderErrors()}
- <Alert
- css={theme => ({ marginBottom: theme.sizeUnit * 4 })}
- type="warning"
- message={
- <>
- {' '}
- <strong>{t('Be careful.')} </strong>
- {t(
- 'Changing these settings will affect all charts using this
dataset, including charts owned by other people.',
- )}
- </>
- }
+ // Retained to mirror the canonical (class-based) component on master: the
+ // Spatial tab definition is kept available even though it is not wired into
+ // the rendered tab list. Removing it would also drop its translatable
strings
+ // and regress existing translations. It is referenced in the `tabItems`
+ // dependency list below so it is not reported as an unused local.
+ const renderSpatialTab = useCallback(() => {
+ const { spatials, all_cols: allCols } = datasource;
+
+ return {
+ key: TABS_KEYS.SPATIAL,
+ label: <CollectionTabTitle collection={spatials} title={t('Spatial')} />,
+ children: (
+ <CollectionTable
+ tableColumns={['name', 'config']}
+ sortColumns={['name']}
+ onChange={value => onDatasourcePropChange('spatials', value)}
+ itemGenerator={() => ({
+ name: t('<new spatial>'),
+ type: t('<no type>'),
+ config: null,
+ })}
+ collection={spatials ?? []}
+ allowDeletes
+ itemRenderers={{
+ name: (d, onChange) => (
+ <EditableTitle
+ canEdit
+ title={d as string}
+ onSaveTitle={onChange}
+ />
+ ),
+ config: (v, onChange) => (
+ <SpatialControl
+ value={
+ v as {
+ type: 'latlong' | 'delimited' | 'geohash';
+ }
+ }
+ onChange={onChange}
+ choices={allCols?.map(col => [col, col] as [string, string])}
+ />
+ ),
+ }}
/>
- <StyledTableTabs
- id="table-tabs"
- data-test="edit-dataset-tabs"
- onChange={this.handleTabSelect}
- defaultActiveKey={activeTabKey}
- items={[
- {
- key: TABS_KEYS.SOURCE,
- label: t('Source'),
- children: this.renderSourceFieldset(),
- },
- {
- key: TABS_KEYS.METRICS,
- label: (
- <CollectionTabTitle
- collection={sortedMetrics}
- title={t('Metrics')}
- />
- ),
- children: this.renderMetricCollection(),
- },
- {
- key: TABS_KEYS.COLUMNS,
- label: (
- <CollectionTabTitle
- collection={this.state.databaseColumns}
- title={t('Columns')}
- />
- ),
- children: (
- <StyledTableTabWrapper>
- {this.renderDefaultColumnSettings()}
- <ColumnButtonWrapper>
- <StyledButtonWrapper>
- <Button
- buttonSize="small"
- buttonStyle="tertiary"
- onClick={this.syncMetadata}
- className="sync-from-source"
- disabled={this.state.isEditMode}
- >
- <Icons.DatabaseOutlined iconSize="m" />
- {t('Sync columns from source')}
- </Button>
- </StyledButtonWrapper>
- </ColumnButtonWrapper>
- <Input.Search
- placeholder={t('Search columns by name')}
- value={this.state.columnSearchTerm}
- onChange={e =>
- this.setState({ columnSearchTerm: e.target.value })
- }
- style={{ marginBottom: 16, width: 300 }}
- allowClear
- />
- <ColumnCollectionTable
- className="columns-table"
- columns={this.state.databaseColumns}
- filterTerm={this.state.columnSearchTerm}
- filterFields={['column_name']}
- datasource={datasource}
- onColumnsChange={databaseColumns =>
- this.setColumns({ databaseColumns })
- }
- onDatasourceChange={this.onDatasourceChange}
- />
- {this.state.metadataLoading && <Loading />}
- </StyledTableTabWrapper>
- ),
- },
+ ),
+ };
+ }, [datasource, onDatasourcePropChange]);
+
+ const tabItems = useMemo(
+ () => [
+ {
+ key: TABS_KEYS.SOURCE,
+ label: t('Source'),
+ children: renderSourceFieldset(),
+ },
+ {
+ key: TABS_KEYS.METRICS,
+ label: (
+ <CollectionTabTitle collection={sortedMetrics} title={t('Metrics')}
/>
+ ),
+ children: renderMetricCollection(),
+ },
+ {
+ key: TABS_KEYS.COLUMNS,
+ label: (
+ <CollectionTabTitle
+ collection={databaseColumns}
+ title={t('Columns')}
+ />
+ ),
+ children: (
+ <StyledTableTabWrapper>
+ {renderDefaultColumnSettings()}
+ <DefaultColumnSettingsTitle>
+ {t('Column Settings')}
+ </DefaultColumnSettingsTitle>
+ <ColumnButtonWrapper>
+ <StyledButtonWrapper>
+ <Button
+ buttonSize="small"
+ buttonStyle="tertiary"
+ onClick={syncMetadata}
+ className="sync-from-source"
+ disabled={isEditMode}
Review Comment:
**Suggestion:** The lock/edit gating is inverted for the "Sync columns from
source" action: it is disabled when edit mode is enabled and enabled when the
form is locked. This blocks users from syncing metadata while actively editing
and allows sync while the UI says changes are locked. Flip the condition so
sync is disabled when editing is locked. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Sync columns allowed while editor lock indicates no changes.
- ⚠️ Unable to sync metadata while dataset is editable.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dataset in edit mode so `DatasourceModal` renders
(`superset-frontend/src/components/Datasource/DatasourceModal/index.tsx:85-92`),
which
mounts `DatasourceEditor` with the current datasource.
2. In `DatasourceEditor` (`DatasourceEditor.tsx:1810-1839`), observe the
lock UI: when
`isEditMode` is false, the message is "Click the lock to make changes." and
the lock icon
is closed.
3. Navigate to the Columns tab; the "Sync columns from source" button is
defined at
`DatasourceEditor.tsx:2447-2456` with `disabled={isEditMode}`, so it is
enabled while
`isEditMode` is false (form locked) and disabled when `isEditMode` is true
(form
editable).
4. Click the lock to enter edit mode via `onChangeEditMode`
(`DatasourceEditor.tsx:1112-1115`): the message changes to "Click the lock
to prevent
further changes." but the sync button becomes disabled, preventing metadata
sync during
editing and allowing it while the UI indicates "locked."
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f141b10b32f7436b9aeb5f9167ebfe33&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f141b10b32f7436b9aeb5f9167ebfe33&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
**Line:** 2452:2452
**Comment:**
*Incorrect Condition Logic: The lock/edit gating is inverted for the
"Sync columns from source" action: it is disabled when edit mode is enabled and
enabled when the form is locked. This blocks users from syncing metadata while
actively editing and allows sync while the UI says changes are locked. Flip the
condition so sync is disabled when editing is locked.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=b4eb214f715e72156bbe004a91995f7c46f57bd237d28061509f538d1435c900&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=b4eb214f715e72156bbe004a91995f7c46f57bd237d28061509f538d1435c900&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
