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

Fix the problem with instant redirect #390

Merged
merged 3 commits into from
May 1, 2021

Conversation

GneyHabub
Copy link
Contributor

  • Fixed the problem with redirecting after an error
  • Fixed the problem with the height of JSONEditor
  • Slightly improved the schema edition form

@workshur workshur closed this Apr 25, 2021
@workshur workshur reopened this Apr 25, 2021
Copy link
Member

@marat-ad marat-ad left a comment

Choose a reason for hiding this comment

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

I think we should call each api request in updateSchema thunk instead of dispatching other thunks, since update is a separate user action, which should have its own request/success/failure action callers. In that case we can use try/catch inside updateSchema, throw an error in the end of catch block, and catch this error in the component to determine if we should redirect or not. That will allow us avoid using all those statuses updates and get rid of confusing useEffects.

@marat-ad marat-ad self-requested a review April 26, 2021 11:00
@marat-ad marat-ad self-requested a review April 27, 2021 20:02
@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2021

@GneyHabub GneyHabub merged commit 42a1c97 into master May 1, 2021
@GneyHabub GneyHabub deleted the fix/enhance-schema-registry-views branch May 1, 2021 06:08
javalover123 pushed a commit to javalover123/kafka-ui that referenced this pull request Dec 7, 2022
* Fix the problem with instant redirect

* Rewrite updateSchema thunk
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.

None yet

3 participants