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

Edit remote repo #2064

Merged
merged 7 commits into from
Jun 14, 2022
Merged

Edit remote repo #2064

merged 7 commits into from
Jun 14, 2022

Conversation

ShaiahWren
Copy link
Contributor

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

Moving 'edit remote repo' testing from #1833 to a separate pr.

Noting that there is very limited error handling with regard to input length, spaces, etc on all inputs except the URL.

@ShaiahWren ShaiahWren force-pushed the edit-remote-repo branch 2 times, most recently from d925f55 to cd997d2 Compare May 16, 2022 14:12
@himdel
Copy link
Collaborator

himdel commented May 19, 2022

is this about #1833 (comment) ?

If so, that comment was just about testing the "registry" part of the form in the (existing) registry tests.

If not .. what's the difference here?

@himdel himdel removed their request for review May 23, 2022 13:35
@ShaiahWren
Copy link
Contributor Author

@himdel I moved the 'edit' portion of the repo management pr over here, per @ZitaNemeckova's request.

@himdel
Copy link
Collaborator

himdel commented May 26, 2022

Ah, so just splitting a big pr, makes sense :)

LGTM, except this seems to break the same task* tests

@himdel
Copy link
Collaborator

himdel commented May 26, 2022

..although, is there one type missing?

the form works in 4 modes .. certified, community, none and registry .. registry is handled in #1833, but I can only see community and certified here?

cy.get('input[id="username"]').should('have.value', 'test');
cy.get('input[id="proxy_url"]').should('have.value', 'https://example.org');
cy.get('input[id="proxy_username"]').should('have.value', 'test');
cy.intercept(
Copy link
Member

Choose a reason for hiding this comment

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

No need to define it again. You could use @editCommunityRemote again.

.click({ multiple: true });
cy.get('input[id="proxy_url"]').clear();
cy.get('input[id="proxy_username"]').clear();
cy.contains('Save').click();
Copy link
Member

Choose a reason for hiding this comment

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

There's no check the save was a success?

cy.get('input[id="proxy_username"]').should('have.value', 'test');
cy.intercept(
'PUT',
Cypress.env('prefix') + 'content/community/v3/sync/config/',
Copy link
Member

Choose a reason for hiding this comment

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

Is it intentional that it's a community URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - any reason it can't be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ZitaNemeckova now it is using rh-certified.

@ZitaNemeckova
Copy link
Member

@ShaiahWren Great job! I added few comments.

@ShaiahWren ShaiahWren force-pushed the edit-remote-repo branch 2 times, most recently from 614f813 to 4fa96ca Compare June 1, 2022 14:30
.click({ multiple: true });
cy.get('input[id="proxy_url"]').clear();
cy.get('input[id="proxy_username"]').clear();
cy.contains('Save').click();
Copy link
Member

@ZitaNemeckova ZitaNemeckova Jun 6, 2022

Choose a reason for hiding this comment

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

There's no check that the save is a success. There should be a bit of code that checks that username, proxy_url, proxy_username are empty after the save. This applies to the community test as well.

Otherwise LGTM 👍

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
No-Issue

Verified

This commit was signed with the committer’s verified signature.
ShaiahWren Shaiah Emigh-Doyle
Issue: AAH-618
Copy link
Member

@ZitaNemeckova ZitaNemeckova left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ShaiahWren
Copy link
Contributor Author

Pr closed.

@ShaiahWren ShaiahWren merged commit 5f463e4 into ansible:master Jun 14, 2022
@ShaiahWren ShaiahWren deleted the edit-remote-repo branch June 14, 2022 13:50
drodowic pushed a commit to drodowic/ansible-hub-ui that referenced this pull request Jun 16, 2022

Partially verified

This commit is signed with the committer’s verified signature.
drodowic’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
* edit remote repo

Issue: AAH-618

* edit a remote repo

Issue: AAH-618

* adjust tests for rh-certified

Issue: AAH-618

* add to changelog

Issue: AAH-618

* check for visibility on cy.eq() calls

Issue: AAH-618

* remove config button test - it won't appear anyway

No-Issue

* confirm all values are cleared

Issue: AAH-618
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