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

ui: Robot Accounts tab fixes (PROJQUAY-5914) #2097

Merged
merged 9 commits into from Aug 16, 2023

Conversation

Sunandadadi
Copy link
Contributor

@Sunandadadi Sunandadadi commented Aug 3, 2023

This PR fixes:

  1. Clearing states on closing Create Robot account wizard
  2. Footer for all modal (teams, robot token creds and set repo perms)
  3. Save repo perms
  4. Bulk change repo perms

@dmage
Copy link
Member

dmage commented Aug 8, 2023

@Sunandadadi is this PR ready for review or is it WIP (the 4th item is WIP in the description)?

@Sunandadadi Sunandadadi changed the title ui: Robot Accounts tab fixes (PROJQUAY-5914) WIP: ui: Robot Accounts tab fixes (PROJQUAY-5914) Aug 8, 2023
@Sunandadadi
Copy link
Contributor Author

Sunandadadi commented Aug 8, 2023

@dmage it is WIP, updated the title of the PR.

@Sunandadadi Sunandadadi changed the title WIP: ui: Robot Accounts tab fixes (PROJQUAY-5914) ui: Robot Accounts tab fixes (PROJQUAY-5914) Aug 14, 2023
bcaton85
bcaton85 previously approved these changes Aug 16, 2023
Copy link
Contributor

@bcaton85 bcaton85 left a comment

Choose a reason for hiding this comment

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

LGTM. I think there should be success/failure alerts when the settings change but that can be done in a follow up PR.

{
onSuccess: () => {
onSuccess: async (result) => {
await Promise.allSettled([updateRobotData(result)]);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if these requests fail? Is that caught and handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, pushed handing for onError. Verified that OnError is defined in all the places useRobotAccounts hook is used.

Copy link
Contributor

@bcaton85 bcaton85 left a comment

Choose a reason for hiding this comment

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

LGTM

@Sunandadadi Sunandadadi merged commit 36a78ad into quay:master Aug 16, 2023
13 checks passed
@Sunandadadi
Copy link
Contributor Author

/cherry-pick redhat-3.9

@Sunandadadi
Copy link
Contributor Author

/cherrypick redhat-3.9

@Sunandadadi Sunandadadi deleted the PROJQUAY-5914 branch August 17, 2023 16:47
@openshift-cherrypick-robot

@Sunandadadi: new pull request created: #2135

In response to this:

/cherry-pick redhat-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@Sunandadadi: new pull request could not be created: failed to create pull request against quay/quay#redhat-3.9 from head openshift-cherrypick-robot:cherry-pick-2097-to-redhat-3.9: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for openshift-cherrypick-robot:cherry-pick-2097-to-redhat-3.9."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

In response to this:

/cherrypick redhat-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Sunandadadi
Copy link
Contributor Author

/cherrypick redhat-3.9

@openshift-cherrypick-robot

@Sunandadadi: #2097 failed to apply on top of branch "redhat-3.9":

Applying: UI: Robot Accounts tab fixes (PROJQUAY-5914)
Using index info to reconstruct a base tree...
M	web/src/components/modals/CreateRobotAccountModal.tsx
M	web/src/components/modals/robotAccountWizard/DisplayModal.tsx
M	web/src/hooks/useRobotAccounts.ts
M	web/src/routes/RepositoriesList/RobotAccountsList.tsx
Falling back to patching base and 3-way merge...
Auto-merging web/src/hooks/useRobotAccounts.ts
CONFLICT (content): Merge conflict in web/src/hooks/useRobotAccounts.ts
Auto-merging web/src/components/modals/robotAccountWizard/DisplayModal.tsx
Auto-merging web/src/components/modals/CreateRobotAccountModal.tsx
CONFLICT (content): Merge conflict in web/src/components/modals/CreateRobotAccountModal.tsx
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 UI: Robot Accounts tab fixes (PROJQUAY-5914)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick redhat-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants