hughhhh commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4319659878
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1250,6 +1356,124 @@ function DatasourceEditor({
[],
);
+ // Which column's row expand to open, for the "map a different column" links.
+ // Consumed by the Columns tab, which scrolls the row into view and expands
it.
+ //
+ // Carries a nonce because the request is an event, not a state: clicking the
+ // same link twice -- after collapsing the row by hand in between -- asks for
+ // the same column name, and a bare string would make the second
+ // `setColumnToReveal` a no-op. React would bail out of the render, nothing
+ // downstream would see a change, and the row would stay shut.
+ const [columnToReveal, setColumnToReveal] = useState<{
+ name: string;
+ nonce: number;
+ } | null>(null);
+
+ const handlePartitionColumnChange = useCallback(
+ (columnName: string | null) => {
+ setDatasource(prev => ({
+ ...prev,
+ partition_column: columnName,
+ partition_mapped_column: nextMappedColumnOverride(
+ prev.partition_mapped_column,
+ columnName,
+ ),
+ }));
+ if (columnName) {
+ setDatabaseColumns(prev =>
+ applyPartitionColumnDefaults(prev, columnName),
Review Comment:
Fixed. Everything past the `await` reads `currentColumnsRef.current` and
`datasourceRef.current` rather than the closure, so the response merges against
the live column state instead of the snapshot taken when the sync started.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1250,6 +1373,143 @@ function DatasourceEditor({
[],
);
+ // Which column's row expand to open, for the "map a different column" links.
+ // Consumed by the Columns tab, which scrolls the row into view and expands
it.
+ //
+ // Carries a nonce because the request is an event, not a state: clicking the
+ // same link twice -- after collapsing the row by hand in between -- asks for
+ // the same column name, and a bare string would make the second
+ // `setColumnToReveal` a no-op. React would bail out of the render, nothing
+ // downstream would see a change, and the row would stay shut.
+ const [columnToReveal, setColumnToReveal] = useState<{
+ name: string;
+ nonce: number;
+ } | null>(null);
+
+ // What the mapped column's own preview said, relayed up from its row expand
+ // so the dataset-level banner cannot claim a mirror that panel has already
+ // refused. Reported rather than lifted: the transform the preview judges is
+ // the debounced local one that panel owns. `null` means no verdict yet.
+ const [previewMirrors, setPreviewMirrors] = useState<boolean | null>(null);
+
+ const handlePartitionColumnChange = useCallback(
+ (columnName: string | null) => {
+ setDatasource(prev => ({
+ ...prev,
+ partition_column: columnName,
+ partition_mapped_column: nextMappedColumnOverride(
+ prev.partition_mapped_column,
+ columnName,
+ ),
+ }));
+ if (columnName) {
+ setDatabaseColumns(prev =>
+ applyPartitionColumnDefaults(prev, columnName),
+ );
+ }
+ },
+ [],
+ );
+
+ const handleNavigateToColumn = useCallback((columnName: string) => {
+ setActiveTabKey(TABS_KEYS.COLUMNS);
Review Comment:
Fixed. The tabs are controlled with `activeKey={activeTabKey}` rather than
`defaultActiveKey`, so `handleNavigateToColumn` actually moves the user to the
Columns tab.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1250,6 +1356,124 @@ function DatasourceEditor({
[],
);
+ // Which column's row expand to open, for the "map a different column" links.
+ // Consumed by the Columns tab, which scrolls the row into view and expands
it.
+ //
+ // Carries a nonce because the request is an event, not a state: clicking the
+ // same link twice -- after collapsing the row by hand in between -- asks for
+ // the same column name, and a bare string would make the second
+ // `setColumnToReveal` a no-op. React would bail out of the render, nothing
+ // downstream would see a change, and the row would stay shut.
+ const [columnToReveal, setColumnToReveal] = useState<{
+ name: string;
+ nonce: number;
+ } | null>(null);
+
+ const handlePartitionColumnChange = useCallback(
+ (columnName: string | null) => {
+ setDatasource(prev => ({
+ ...prev,
+ partition_column: columnName,
+ partition_mapped_column: nextMappedColumnOverride(
+ prev.partition_mapped_column,
+ columnName,
+ ),
+ }));
+ if (columnName) {
+ setDatabaseColumns(prev =>
+ applyPartitionColumnDefaults(prev, columnName),
+ );
+ }
+ },
+ [],
+ );
+
+ const handleNavigateToColumn = useCallback((columnName: string) => {
+ setActiveTabKey(TABS_KEYS.COLUMNS);
+ // Filter to the column as well as expanding it: on a wide table the row
+ // would otherwise open somewhere below the fold, which reads as the link
+ // having done nothing.
+ setColumnSearchTerm(columnName);
+ setColumnToReveal(previous => ({
+ name: columnName,
+ nonce: (previous?.nonce ?? 0) + 1,
+ }));
+ }, []);
+
+ const handleMoveMappingHere = useCallback(
+ (columnName: string) => {
+ setDatabaseColumns(prev =>
+ applyMappingMove(
+ prev,
+ columnName,
+ defaultTransformFor(
+ datasource,
+ prev.find(column => column.column_name === columnName),
+ ),
+ ),
+ );
+ // Always explicit from here: the owner picked this column, so the
mapping
+ // must not drift back the next time the default datetime column moves.
+ setDatasource(prev => ({ ...prev, partition_mapped_column: columnName
}));
+ },
+ [datasource],
+ );
+
+ const handleRemoveMapping = useCallback(() => {
+ // Every column, not just the one that resolved as mapped: with the
override
+ // gone the mapping falls back to the default datetime column, so a
+ // transform left anywhere else would come straight back to life -- which
is
+ // the one thing "remove" has to rule out. `main_dttm_col` can name a
+ // calculated column, so both lists are cleared.
+ setDatabaseColumns(prev => clearMappingTransforms(prev));
+ setCalculatedColumns(prev => clearMappingTransforms(prev));
+ // The partition column stays designated; only the mapping goes away, which
+ // is the 1g state -- hidden from Explore, nothing mirrored onto it, and
the
+ // panel's warning saying so.
+ setDatasource(prev => ({ ...prev, partition_mapped_column: null }));
+ }, []);
+
+ const handleMonotonicChange = useCallback(
+ (columnName: string, isMonotonic: boolean) => {
+ setDatabaseColumns(prev =>
+ prev.map(column =>
+ column.column_name === columnName
+ ? { ...column, partition_transform_is_monotonic: isMonotonic }
+ : column,
+ ),
+ );
+ },
+ [],
+ );
+
+ const handleMainDttmColChange = useCallback(
+ (value?: string) => {
+ // Without an override the mapped column *is* the default datetime
column,
+ // so re-pointing it moves the mapping. The value transform stays behind
+ // and is cleared: it was written about the old column, and the mapping
+ // arrives inert rather than mirroring an expression nobody checked
+ // against its new home. Tested on the partition column plus the absent
+ // override rather than on `mappedColumnIsImplicit`, which needs a
+ // datetime column already set and so misses the transition that sets the
+ // first one.
+ if (datasource.partition_column && !datasource.partition_mapped_column) {
Review Comment:
Fixed. The clearing branch is now behind
`isFeatureEnabled(FeatureFlag.PartitionFilterMapping)`, so with the feature off
a datetime-column change leaves the stored transform and monotonicity flag
alone.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -617,100 +674,160 @@ function ColumnCollectionTable({
/>
);
- return (
- <CollectionTable
- tableColumns={tableColumns}
- sortColumns={tableColumns}
- allowDeletes
- allowAddItem={allowAddItem}
- itemGenerator={itemGenerator}
- collection={columns}
- columnLabelTooltips={columnLabelTooltips}
- filterTerm={filterTerm}
- filterFields={filterFields}
- stickyHeader
- expandFieldset={
- <FormContainer>
- <Fieldset compact>
- {showExpression && (
- <Field
- fieldKey="expression"
- label={t('SQL expression')}
- control={
- <TextAreaControl
- language="sql"
- offerEditInModal={false}
- maxLines={25}
- debounceDelay={300}
- />
- }
- />
- )}
- <Field
- fieldKey="verbose_name"
- label={t('Label')}
- control={
- <TextControl
- controlId="verbose_name"
- placeholder={t('Label')}
- />
- }
+ const partitionMappingEnabled =
+ isFeatureEnabled(FeatureFlag.PartitionFilterMapping) &&
Boolean(datasource);
+ const partitionColumn = partitionMappingEnabled
+ ? datasource?.partition_column
+ : null;
+
+ // The two `itemRenderers` variants below differ only in which widget edits
+ // the name, so the certified badge and the PARTITION tag are shared here
+ // rather than written out four times.
+ const renderColumnName =
+ (EditControl: 'editableTitle' | 'textControl') =>
+ (
+ v: unknown,
+ onItemChange: (value: any) => void,
+ _: unknown,
+ record: Column,
+ ): ReactNode => (
+ <StyledLabelWrapper>
+ {record.is_certified && (
+ <CertifiedBadge
+ certifiedBy={record.certified_by}
+ details={record.certification_details}
+ />
+ )}
+ {editableColumnName ? (
+ EditControl === 'editableTitle' ? (
+ <EditableTitle
+ canEdit
+ title={v as string}
+ onSaveTitle={onItemChange}
/>
- <Field
- fieldKey="description"
- label={t('Description')}
- control={
- <TextControl
- controlId="description"
- placeholder={t('Description')}
+ ) : (
+ <TextControl value={v as string} onChange={onItemChange} />
+ )
+ ) : (
+ (v as ReactNode)
+ )}
+ {partitionColumn === record.column_name && (
+ <Tooltip
+ title={t(
+ 'Filters on the mapped column are mirrored onto this column so
the engine can prune partitions.',
+ )}
+ >
+ <Label data-test="partition-tag">
+ <Icons.FilterOutlined iconSize="s" /> {t('PARTITION')}
+ </Label>
+ </Tooltip>
+ )}
+ </StyledLabelWrapper>
+ );
+
+ return (
+ <StyledColumnsTableWrapper>
+ <CollectionTable
+ tableColumns={tableColumns}
+ sortColumns={tableColumns}
+ allowDeletes
+ allowAddItem={allowAddItem}
+ itemGenerator={itemGenerator}
+ collection={columns}
+ columnLabelTooltips={columnLabelTooltips}
+ filterTerm={filterTerm}
+ filterFields={filterFields}
+ rowClassName={record =>
+ partitionColumn === record.column_name ? 'partition-column-row' : ''
+ }
+ expandItemWhere={
Review Comment:
Fixed. The `expandItemWhere` and reveal props are back on the collection
table, so Customize the value transform opens the mapped column's row rather
than only filtering to it.
--
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]
