Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Reflect column and engine values for existing tables #2444

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

UI: Reflect column and engine values for existing tables #2444

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75cb903
wire exclude for already existing
Amogh-Bharadwaj Jan 14, 2025
42ed6a1
populate and disable engine select
Amogh-Bharadwaj Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions ui/app/mirrors/create/cdc/schemabox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,17 @@ export default function SchemaBox({
?.map((tableMap) => tableMap.sourceTableIdentifier)
.includes(row.source)
) {
const existingRow = alreadySelectedTables?.find(
Copy link
Contributor

@serprex serprex Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const existingRow = alreadySelectedTables?.find(
const existingRow = alreadySelectedTables.find(

can remove the ?.map above too. As is it'll hit an error about trying to call undefined as a function. You'd have to use the find?.(...) to null check the call too. Tho, in this case, we only end up here if map includes returned true, so if alreadySelectedTables was null/undefined you'd never run this code in that case

This might be related to earlier PR preferring sending empty arrays over null

(tableMap) => tableMap.sourceTableIdentifier === row.source
);
row.selected = true;
row.engine = existingRow?.engine ?? TableEngine.CH_ENGINE_REPLACING_MERGE_TREE;
row.editingDisabled = true;
row.destination =
alreadySelectedTables?.find(
(tableMap) => tableMap.sourceTableIdentifier === row.source
)?.destinationTableIdentifier ?? '';
row.exclude = new Set(existingRow?.exclude ?? []);
row.destination = existingRow?.destinationTableIdentifier ?? '';
alreadySelectedTables?.find(
(tableMap) => tableMap.sourceTableIdentifier === row.source
)?.destinationTableIdentifier ?? '';
Comment on lines +210 to +212
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
alreadySelectedTables?.find(
(tableMap) => tableMap.sourceTableIdentifier === row.source
)?.destinationTableIdentifier ?? '';

addTableColumns(row.source);
}
}
Expand Down Expand Up @@ -369,6 +374,7 @@ export default function SchemaBox({
Engine:
</p>
<ReactSelect
isDisabled={row.editingDisabled}
styles={engineOptionStyles}
options={engineOptions}
placeholder='ReplacingMergeTree (default)'
Expand Down
Loading