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

Make password and reset_password exclusive #262

Merged
merged 3 commits into from
Sep 11, 2020

Conversation

ilpianista
Copy link
Contributor

The documentation works when either password or reset_password is set.

@ghost ghost added the size/XS label Feb 21, 2020
@ilpianista ilpianista force-pushed the feature/optional-password branch from fb3a12c to 66cf697 Compare February 21, 2020 15:58
@roidelapluie
Copy link
Collaborator

Can we have an acceptance test with an empty password?

@ilpianista ilpianista force-pushed the feature/optional-password branch from 66cf697 to 58067bf Compare February 24, 2020 12:06
@ilpianista
Copy link
Contributor Author

@roidelapluie is this OK as test? I don't set a password during the creation, but then it's set during the user update test.

Still, I don't think we can check if the password has been set because the password is (obviously) never sent back by GitLab. Thus, I removed that part.

@ilpianista ilpianista force-pushed the feature/optional-password branch from e254509 to 3954d82 Compare February 24, 2020 17:29
@ghost ghost added documentation size/S and removed size/XS labels Feb 24, 2020
@ilpianista ilpianista force-pushed the feature/optional-password branch from 3954d82 to 94087df Compare April 7, 2020 12:37
@ilpianista ilpianista force-pushed the feature/optional-password branch from 94087df to 9432fb7 Compare June 29, 2020 12:17
@ghost ghost added size/XS and removed size/S labels Jun 29, 2020
@ilpianista ilpianista changed the title Don't require password to be set Make password and reset_password exclusive Jun 29, 2020
@ilpianista
Copy link
Contributor Author

ilpianista commented Jun 29, 2020

Making the password optional has been solved by #340, so this PR now just ensure that either password or reset_password is set, not both.

@ilpianista ilpianista force-pushed the feature/optional-password branch from 9432fb7 to 1033886 Compare August 18, 2020 15:02
@ilpianista ilpianista force-pushed the feature/optional-password branch from 1033886 to ffca640 Compare September 8, 2020 09:34
@armsnyder
Copy link
Collaborator

@ilpianista Can you add two new tests?

  • One where you only set reset_password, just to ensure that it doesn't explode if password is undefined.
  • One where you set neither field and expect an error.

@ilpianista
Copy link
Contributor Author

@armsnyder I implemented 2). To implement 1), I simply removed the password argument from the existing test.

@ilpianista ilpianista force-pushed the feature/optional-password branch from b779e7d to 46a9dbf Compare September 9, 2020 10:35
Copy link
Collaborator

@armsnyder armsnyder left a comment

Choose a reason for hiding this comment

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

Thanks!

@armsnyder armsnyder merged commit b1e2fd8 into gitlabhq:master Sep 11, 2020
@ilpianista ilpianista deleted the feature/optional-password branch September 11, 2020 09:06
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants