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

Jump to bottom

Fix numerous create calls #7659

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Fix numerous create calls #7659

merged 1 commit into from
Nov 5, 2021

Conversation

jamakase
Copy link
Contributor

@jamakase jamakase commented Nov 4, 2021 •
edited
Loading

What

  • Handles all errors during login ( noticed due to API key expiration )
  • Fixes creating numerous users accidentally. It was due to not unsubscribing callback correctly

@jamakase jamakase self-assigned this Nov 4, 2021
@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels Nov 4, 2021
@jamakase jamakase added the type/bug Something isn't working label Nov 4, 2021
@jamakase jamakase temporarily deployed to more-secrets November 4, 2021 23:18 Inactive
davinchia
davinchia approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

I'm not a Javascript expert, but this looks okay to me. Is it worth having tests so we don't regress?

@jamakase
Copy link
Contributor Author

jamakase commented Nov 5, 2021

I'm not a Javascript expert, but this looks okay to me. Is it worth having tests so we don't regress?

Not sure it's easy to cover it with tests. We could add separate task to add them or try to cover them by e2e testing.

I would prefer to have this fix now and add any required tests later.

@jamakase jamakase merged commit 7fd1804 into master Nov 5, 2021
@jamakase jamakase deleted the jamakase/fix-user-auth branch November 5, 2021 10:52
@davinchia
Copy link
Contributor

okay! thanks

@jrhizor jrhizor mentioned this pull request Nov 6, 2021
Merged
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
@jamakase @schlattk
Fix numerous create calls (airbytehq#7659)
3bd9bd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davinchia davinchia davinchia approved these changes

@jrhizor jrhizor Awaiting requested review from jrhizor

Assignees

@jamakase jamakase

Labels
area/platform issues related to the platform type/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jamakase @davinchia