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

[User Accounts] Hide UI option allowing users to edit their own approval status #5353

Merged
merged 5 commits into from
Nov 20, 2019
Merged

[User Accounts] Hide UI option allowing users to edit their own approval status #5353

merged 5 commits into from
Nov 20, 2019

Conversation

johnsaigle
Copy link
Contributor

Brief summary of changes

Fix #4743 where a user could accidentally lock themselves out but editing their own approval status.

Testing instructions (if applicable)

  1. Checkout this branch, go to user accounts.
  2. You will no longer see the Pending Approval fields when editing your own accounts

@johnsaigle johnsaigle added Category: Bug PR or issue that aims to report or fix a bug Cleanup PR or issue introducing/requiring at least one clean-up operation State: Needs work PR awaiting additional work by the author to proceed labels Oct 22, 2019
@johnsaigle
Copy link
Contributor Author

I think I'll have to update the integration tests

@ridz1208 ridz1208 added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 6, 2019
@johnsaigle johnsaigle removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 13, 2019
@johnsaigle johnsaigle changed the base branch from minor to 22.0-release November 13, 2019 16:02
@ridz1208
Copy link
Collaborator

@johnsaigle i made an attempt to fix the failing test, however I have a question.

why not also hide the active field ? this way you cant lock urself out by setting it to no.

@johnsaigle
Copy link
Contributor Author

@ridz1208 I updated this PR to also hide the active status as well as the active window.

I'm not really clear on why you updated the integration test the way you did. Can you explain the changes?

@ridz1208
Copy link
Collaborator

@johnsaigle The logged in user for testingg is UnitTester and the changes are being tested on the user itself. so obviously it's failing when it gets to testing the pending approval dropdown.

I tried adding an other user and editing that one instead but I'm getting a weird duplicated value error when there should not be any other 999991 user from what I can tell.

@kongtiaowang is looking into that particular error

@kongtiaowang
Copy link
Contributor

I will reopen this to debug Travis.

@johnsaigle johnsaigle removed the State: Needs work PR awaiting additional work by the author to proceed label Nov 19, 2019
@driusan driusan merged commit efda4ff into aces:22.0-release Nov 20, 2019
@ridz1208 ridz1208 added this to the 22.0.0 milestone Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug PR or issue that aims to report or fix a bug Cleanup PR or issue introducing/requiring at least one clean-up operation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants