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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃獰 馃悰 Fix validation when setting the connection stream cursor or primary key #18933

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

edmundito
Copy link
Contributor

What

Related to https://github.com/airbytehq/oncall/issues/948

This fixes an issue related to validation not running when the connection stream primary key or cursor field is not set.

How

The validation was happening in the wrong order because the form initial values streams were being sorted when the CatalogTree component rendered. Instead, now it makes a copy of the array before sorting since sorting is one of those weird functions that is done in place.

Move changedStreams from CatalogTreeBody to CatalogTree so that the order of the changedStreams is based on the correct order
@edmundito edmundito requested a review from a team as a code owner November 3, 2022 19:02
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Nov 3, 2022
.sort(naturalComparatorBy((syncStream) => syncStream.stream?.name ?? "")),
connection.syncCatalog.streams
.filter((s) => s.config?.selected)
.sort(naturalComparatorBy((syncStream) => syncStream.stream?.name ?? ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why these no longer need the sort. The only place I see us sorting now is the catalogTree. Isn't there still a chance that the order is not preserved between the connection object and the form values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why this was sorted in the first place was that the formValues data was getting mutated by the sort call in the CatalogTree component. The formValues now contain same stream order as received in the connection object, and the order should match.

@teallarson
Copy link
Contributor

Tested locally and the bug does seem to be gone. Just one question to make sure we're covering all our bases!

@teallarson teallarson self-requested a review November 3, 2022 21:12
Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

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

tested locally and problem is resolved, code looks good.

@edmundito edmundito merged commit d35b350 into master Nov 4, 2022
@edmundito edmundito deleted the edmundito/fix-stream-pk-validation branch November 4, 2022 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants