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

Error messages aren't displaying on the remote container create form #2601

Merged

Conversation

MilanPospisil
Copy link
Contributor

@MilanPospisil MilanPospisil commented Sep 22, 2022

https://issues.redhat.com/browse/AAH-1845

This PR refactors error handling using utils. Every error (including field validation and server side validation together) is now in one associative array formErrors, which can hold error message per form field (for example formErrors['name']).

Also, validation from server in attempt to save the form is now included.

@himdel
Copy link
Collaborator

himdel commented Sep 26, 2022

This is missing a PR description now, what is this supposed to do? :)

@MilanPospisil MilanPospisil changed the title Repository form errors aah 1845 Error messages aren't displaying on the remote container create form Oct 3, 2022
@github-actions github-actions bot added backport-4.2 This PR should be backported to stable-4.2 (1.2) backport-4.4 This PR should be backported to stable-4.4 (2.1) backport-4.5 This PR should be backported to stable-4.5 (2.2) backport-4.6 This PR should be backported to stable-4.6 (2.3) labels Oct 7, 2022
@himdel himdel removed the backport-4.2 This PR should be backported to stable-4.2 (1.2) label Oct 9, 2022
@himdel
Copy link
Collaborator

himdel commented Oct 10, 2022

One more thing noticed during testing... this doesn't deal with unknown errors from the server in any way, no alerts, no messages, just a stuck form.

Try adding a EE with a name like foo/. - this passes the name validation, but the server responds with a base_path validation message .. we should be displaying unknown messages on error.
(Though maybe as a usability improvement, this should be also associated with the name field, so that onChange can unset it.)

(The response looks like...

{
  "errors": [
    {
      "status": "400",
      "code": "invalid",
      "title": "Invalid input.",
      "detail": "The provided base path contains forbidden characters.",
      "source": {
        "parameter": "base_path"
      }
    }
  ]
}

)

@MilanPospisil
Copy link
Contributor Author

TODO - delete validation errors when alert is shown and error does not belong to any field.

@MilanPospisil MilanPospisil force-pushed the repositoryFormErrors-AAH-1845 branch 2 times, most recently from 6d9789e to ec734ef Compare November 7, 2022 13:53
@MilanPospisil MilanPospisil force-pushed the repositoryFormErrors-AAH-1845 branch 2 times, most recently from 4a36a77 to 3cf6437 Compare December 13, 2022 14:27
Issue: 1845

Repair setState passing to function without lambda

PR checks
Issue: AAH-1845

Delete unecessary set functions in validation utility

PR review

Review - registry to registries

Use of form valid

Handle errors for registries and registry.

Log the rest of the errors. Catch promises on save in one place.

Alert the rest errors instead of displaying them.

Translate error string, show error only for unknown field, clear the error after showing alert

Add translation

Rename rest remoteId

PR review

Prettier

WIP

Prettier

Remove messages without field

Prettier

PR Checks

Review - rename remotePulpId and add type to isFieldValid function

Prettier

Change order of prop

WIP

WIP
@himdel
Copy link
Collaborator

himdel commented Dec 13, 2022

Looks like this works 👍

20221213223155
20221213223218
20221213223308
20221213223349
20221213223505

(just needs a rebase to get tests green, #3018 )

Copy link
Collaborator

@himdel himdel left a comment

Choose a reason for hiding this comment

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

👍 (once tests go green)

@MilanPospisil MilanPospisil merged commit 77ecd51 into ansible:master Dec 14, 2022
@patchback

This comment was marked as outdated.

@patchback

This comment was marked as outdated.

@patchback

This comment was marked as outdated.

@himdel himdel removed the backport-4.4 This PR should be backported to stable-4.4 (2.1) label Dec 22, 2022
himdel pushed a commit to himdel/ansible-hub-ui that referenced this pull request Dec 22, 2022
…nsible#2601)

* WIP

* Repair forms validation in EE form.
Issue: 1845

Repair setState passing to function without lambda

PR checks
Issue: AAH-1845

Delete unecessary set functions in validation utility

PR review

Review - registry to registries

Use of form valid

Handle errors for registries and registry.

Log the rest of the errors. Catch promises on save in one place.

Alert the rest errors instead of displaying them.

Translate error string, show error only for unknown field, clear the error after showing alert

Add translation

Rename rest remoteId

PR review

Prettier

WIP

Prettier

Remove messages without field

Prettier

PR Checks

Review - rename remotePulpId and add type to isFieldValid function

Prettier

Change order of prop

WIP

WIP

(cherry picked from commit 77ecd51)

(manual resolution of remoteId vs remotePulpId)
himdel pushed a commit to himdel/ansible-hub-ui that referenced this pull request Dec 23, 2022
…nsible#2601)

Issue: AAH-1845

(cherry picked from commit 77ecd51)

(manual resolution of remoteId vs remotePulpId)
himdel pushed a commit to himdel/ansible-hub-ui that referenced this pull request Dec 23, 2022
…nsible#2601)

Issue: AAH-1845

(cherry picked from commit 77ecd51)

(manual resolution of group field differences)
himdel added a commit that referenced this pull request Jan 2, 2023
…2601) (#3070)

Issue: AAH-1845

(cherry picked from commit 77ecd51)

(manual resolution of remoteId vs remotePulpId)

Co-authored-by: MilanPospisil <arkanus@seznam.cz>
@github-actions github-actions bot added the backported-4.6 This PR has been backported to stable-4.6 (2.3) label Jan 2, 2023
himdel added a commit that referenced this pull request Jan 2, 2023
…2601) (#3071)

Issue: AAH-1845

(cherry picked from commit 77ecd51)

(manual resolution of group field differences)

Co-authored-by: MilanPospisil <arkanus@seznam.cz>
@github-actions github-actions bot added the backported-4.5 This PR has been backported to stable-4.5 (2.2) label Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4.5 This PR should be backported to stable-4.5 (2.2) backport-4.6 This PR should be backported to stable-4.6 (2.3) backported-4.5 This PR has been backported to stable-4.5 (2.2) backported-4.6 This PR has been backported to stable-4.6 (2.3)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants