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

Add check that EE name is correct when typed in #1220

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

ZitaNemeckova
Copy link
Member

@ZitaNemeckova ZitaNemeckova commented Nov 9, 2021

Fixes https://issues.redhat.com/browse/AAH-968

Steps to reproduce:

(Screenshots are in French because I'm looking for possible problems with translation)
Before:
Screenshot 2021-11-09 at 12 23 36

After:
Screenshot 2021-11-09 at 12 22 11

Screenshot 2021-11-11 at 11 39 11

This is not fixed in #1115 and is not related to that PR.

@himdel please have a look, thanks :)

@himdel
Copy link
Collaborator

himdel commented Nov 15, 2021

(test failures seem relevant...)

@awcrosby
Copy link
Contributor

awcrosby commented Dec 1, 2021

(adding this to a few recently updated PRs…) We need CI in this repo that enforces this, but for now please:

  • Add a line to at least one of your commits that says Issue: AAH-#### (in the future it is best to be the first commit as it gets included in the initial PR description by default)
  • Ensure when merging that the Issue: AAH-#### is part of commit message via the “Squash ‘n’ Merge” description

@himdel
Copy link
Collaborator

himdel commented Jan 4, 2022

Needs a rebase, but LGTM :)
(I think the conflict was just about removing !!)

@ZitaNemeckova
Copy link
Member Author

@himdel rebased and ready for review :)

@himdel
Copy link
Collaborator

himdel commented Jan 4, 2022

Tested in the UI, looks like everything works, just one more thing... :)

(correct:) Namespaces > Create ...there is no error when name is empty, until you enter something and remove it again.
(here:) EE > Add execution environment ...shows the validation error right away:

20220104224438

(and we may need a better message for when the field is empty .. namespaces uses Please, provide the namespace name )

Looks like namespaces is running the validation from onChange, not sure if you want to do the same thing or not, maybe all we need is to set state.pristine = true, change to false in onChange, and then we can have validated={pristine || this.validateName(name)} on the FormGroup, without affecting the save button :)

@ZitaNemeckova
Copy link
Member Author

@himdel Fixed as suggested. Just had to move it to the validateName because PF component works with "default"/"error" instead of boolean. So it's ready for another round of review :)

</Form>
</Modal>
);
}

private validateName(name) {
const regex = /^([0-9A-Za-z._-]+\/)?[0-9A-Za-z._-]+$/;
if (name === '' || regex.test(name)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@himdel This is the change you are looking for.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Works for me :), formIsValid still ensures non-empty name 👍

Issue: https://issues.redhat.com/browse/AAH-968

Disable Save button if name is not correct

Do not check name if EE is remote

Add alert when EE is saved to let an user to know

Check that slash is not there or once

Show error messages in the form

Fix regex according to review

Do not check to validate name if editing

Remove testing changes

Move new isDisabled logic to a function. Fix the condition to be simple

Do not check length if local

Make the  formIsValid function more readable

Issue: AAH-968

Fix typo numbers -> characters

And linting issue.

Move Alert out of FormGroup

Add error handling to detail page as well

Don't show name format error message when that input is empty

Add changelog
@himdel himdel merged commit f7b1e2b into ansible:master Jan 11, 2022
himdel pushed a commit to himdel/ansible-hub-ui that referenced this pull request Jan 11, 2022
Issue: https://issues.redhat.com/browse/AAH-968

Disable Save button if name is not correct

Do not check name if EE is remote

Add alert when EE is saved to let an user to know

Check that slash is not there or once

Show error messages in the form

Fix regex according to review

Do not check to validate name if editing

Remove testing changes

Move new isDisabled logic to a function. Fix the condition to be simple

Do not check length if local

Make the  formIsValid function more readable

Issue: AAH-968

Fix typo numbers -> characters

And linting issue.

Move Alert out of FormGroup

Add error handling to detail page as well

Don't show name format error message when that input is empty

Add changelog

(cherry picked from commit f7b1e2b)
@ansible ansible deleted a comment from patchback bot Jan 11, 2022
@himdel
Copy link
Collaborator

himdel commented Jan 11, 2022

backporting manually in #1497

himdel pushed a commit to himdel/ansible-hub-ui that referenced this pull request Jan 11, 2022
Issue: https://issues.redhat.com/browse/AAH-968

Disable Save button if name is not correct

Do not check name if EE is remote

Add alert when EE is saved to let an user to know

Check that slash is not there or once

Show error messages in the form

Fix regex according to review

Do not check to validate name if editing

Remove testing changes

Move new isDisabled logic to a function. Fix the condition to be simple

Do not check length if local

Make the  formIsValid function more readable

Issue: AAH-968

Fix typo numbers -> characters

And linting issue.

Move Alert out of FormGroup

Add error handling to detail page as well

Don't show name format error message when that input is empty

Add changelog

(cherry picked from commit f7b1e2b)
himdel added a commit that referenced this pull request Jan 11, 2022
Issue: https://issues.redhat.com/browse/AAH-968

Disable Save button if name is not correct

Do not check name if EE is remote

Add alert when EE is saved to let an user to know

Check that slash is not there or once

Show error messages in the form

Fix regex according to review

Do not check to validate name if editing

Remove testing changes

Move new isDisabled logic to a function. Fix the condition to be simple

Do not check length if local

Make the  formIsValid function more readable

Issue: AAH-968

Fix typo numbers -> characters

And linting issue.

Move Alert out of FormGroup

Add error handling to detail page as well

Don't show name format error message when that input is empty

Add changelog

(cherry picked from commit f7b1e2b)

Co-authored-by: ZitaNemeckova <znemecko@redhat.com>
@github-actions github-actions bot added the backported-4.4 This PR has been backported to stable-4.4 (2.1) label Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4.4 This PR should be backported to stable-4.4 (2.1) backported-4.4 This PR has been backported to stable-4.4 (2.1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants