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

Highlight invalid cursor or primary key in new connection streams table #20756

Conversation

matter-q
Copy link
Contributor

@matter-q matter-q commented Dec 21, 2022

What

Closes #19918

How

Added error state for required fields for new connection streams table

For test purposes, use the environment variable: REACT_APP_NEW_STREAMS_TABLE=true

Loom

https://www.loom.com/share/4c8c6dac261d48728bbd2803315dcbcb

@matter-q matter-q requested a review from a team as a code owner December 21, 2022 14:47
@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Dec 21, 2022
@matter-q matter-q assigned dizel852 and unassigned dizel852 Dec 21, 2022
@edmundito edmundito requested review from teallarson and edmundito and removed request for a team December 21, 2022 15:08
@matter-q matter-q marked this pull request as draft December 21, 2022 15:27
@matter-q matter-q changed the title Highlight invalid cursor or primary key in new connection streams table [WIP] Highlight invalid cursor or primary key in new connection streams table Dec 21, 2022
@matter-q matter-q force-pushed the mark/highlight-invalid-cursor-or-primary-key-in-new-connection-streams-table branch from 4d09e37 to 26d8c8d Compare December 21, 2022 23:00
Mark Berger added 2 commits January 3, 2023 13:57
- Added error handling for Cursor and Primary Key
- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
@matter-q matter-q force-pushed the mark/highlight-invalid-cursor-or-primary-key-in-new-connection-streams-table branch from a7f0b6d to b6e118d Compare January 3, 2023 11:57
@matter-q matter-q marked this pull request as ready for review January 3, 2023 11:59
@matter-q matter-q changed the title [WIP] Highlight invalid cursor or primary key in new connection streams table Highlight invalid cursor or primary key in new connection streams table Jan 3, 2023
- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
@edmundito edmundito removed the request for review from teallarson January 3, 2023 14:57
@edmundito
Copy link
Contributor

@matter-q The storybook failed to build

- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
@edmundito
Copy link
Contributor

@matter-q While testing, I noticed an edge case that I didn't realize when creating the original ticket. You can use a hubspot source to reproduce this:

  1. Use bulk edit to select all the streams
  2. Toggle the streams to be enabled, select "Incremental | Deduped + History"

Notice that the sync mode is empty in one of the rows and should also be highlighted red:

image

Does it make sense to fix it in this PR?

Copy link
Contributor

@edmundito edmundito left a comment

Choose a reason for hiding this comment

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

I had a few questions about the code. Tested locally and the functionality is good.

@matter-q
Copy link
Contributor Author

matter-q commented Jan 3, 2023

@matter-q While testing, I noticed an edge case that I didn't realize when creating the original ticket. You can use a hubspot source to reproduce this:

  1. Use bulk edit to select all the streams
  2. Toggle the streams to be enabled, select "Incremental | Deduped + History"

Notice that the sync mode is empty in one of the rows and should also be highlighted red:

image

Does it make sense to fix it in this PR?

I think it's better to fix in this PR

Mark Berger added 3 commits January 4, 2023 01:41
- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
- Added individual error handling for Cursor and Primary Key
- Added feature variable to encapsulate new changes with new streams table
@edmundito
Copy link
Contributor

Followed up over standup - The sync mode issue will be skipped for now and resolved separately as part of the larger bulk edit mode behavior

Copy link
Contributor

@edmundito edmundito 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 with new and old table. Code looks good. 🚢

@matter-q matter-q merged commit 2d0d30c into master Jan 5, 2023
@matter-q matter-q deleted the mark/highlight-invalid-cursor-or-primary-key-in-new-connection-streams-table branch January 5, 2023 17:32
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.

Highlight invalid cursor or primary key in new connection streams table
4 participants