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.

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

Conversation

Amogh-Bharadwaj
Copy link
Contributor

@Amogh-Bharadwaj Amogh-Bharadwaj commented Jan 14, 2025

Currently, excluded columns and engine selection are not populated in the table box for an existing table in the mirror (where the destination field, engine, columns are disabled)

This PR wires those through. Tested

@@ -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

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

Comment on lines +210 to +212
alreadySelectedTables?.find(
(tableMap) => tableMap.sourceTableIdentifier === row.source
)?.destinationTableIdentifier ?? '';
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 ?? '';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants