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

Fixes #31985 - show sync error in UI #151

Merged
merged 1 commit into from Mar 8, 2021

Conversation

ezr-ondrej
Copy link
Member

The new Formik forms don't show the validation errors properly.
This should be fixed properly, but for now we are using temporary fix
introduced in theforeman/foreman@0ed6bc5 (theforeman/foreman#8309)

The new Formik forms don't show the validation errors properly.
This should be fixed properly, but for now we are using temporary fix
introduced in theforeman/foreman@0ed6bc5.
@xprazak2
Copy link
Contributor

xprazak2 commented Mar 4, 2021

When there is an error in form submission, user is presented with empty state which is misleading. The toast with error is shown only if user navigates back to sync form.

If there is an error, we should stay in the form and show the toast.

@ezr-ondrej
Copy link
Member Author

If there is an error, we should stay in the form and show the toast.

Agreed, do you see a way how we can fix it in a simle fix?

Proper fix should include frontend validation, but I doubt we can do it in a safe to backport fashion.

@xprazak2
Copy link
Contributor

xprazak2 commented Mar 8, 2021

The form submission relies on common infrastructure provided by Foreman core which is now broken so there is hardly anything we can do about that in a plugin.

We cannot use frontend validation to prevent preconditions for all server-side errors as we can hardly detect whether user has permissions to export into a given repository.

I think I have voiced my concerns strongly enough, here and in other places as well, so there is no point of me going on about it. Even though this does not fix the problem, it does not make it any worse, so merging. Thanks!

@xprazak2 xprazak2 merged commit b3c07cf into theforeman:master Mar 8, 2021
@ezr-ondrej ezr-ondrej deleted the fix_error_report branch March 22, 2021 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants