Skip to content

feat(users): add approve and reject methods to User #2061

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

Merged
merged 1 commit into from
Jun 25, 2022

Conversation

bgamari
Copy link
Contributor

@bgamari bgamari commented Jun 9, 2022

Closes #1604.

Follow-up to #2064.

@bgamari bgamari marked this pull request as ready for review June 9, 2022 15:15
@nejch
Copy link
Member

nejch commented Jun 9, 2022

Thanks a lot for this @bgamari! It'll just need a slight git commit --amend to something like feat(users): add approve and reject methods to keep the linter happy as we use it to generate the changelog.

If you can, would you be able to add tests for this in https://github.com/python-gitlab/python-gitlab/blob/main/tests/unit/objects/test_users.py?

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2022

Codecov Report

Merging #2061 (f57139d) into main (8f8611a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2061      +/-   ##
==========================================
+ Coverage   94.69%   94.71%   +0.01%     
==========================================
  Files          78       78              
  Lines        5056     5070      +14     
==========================================
+ Hits         4788     4802      +14     
  Misses        268      268              
Flag Coverage Δ
cli_func_v4 82.52% <71.42%> (-0.04%) ⬇️
py_func_v4 81.12% <71.42%> (-0.03%) ⬇️
unit 85.79% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gitlab/exceptions.py 100.00% <100.00%> (ø)
gitlab/v4/objects/users.py 96.38% <100.00%> (+0.15%) ⬆️

@bgamari
Copy link
Contributor Author

bgamari commented Jun 9, 2022

I have amended the commit message as suggested.

Unfortunately, it's not clear how to test this robustly given that the GitLab instance the tests are being run against must be configured to have new account approvals enabled. However, I am using this patch locally in managing spam account creation on https://gitlab.haskell.org/

@nejch
Copy link
Member

nejch commented Jun 9, 2022

I have amended the commit message as suggested.

Unfortunately, it's not clear how to test this robustly given that the GitLab instance the tests are being run against must be configured to have new account approvals enabled. However, I am using this patch locally in managing spam account creation on https://gitlab.haskell.org/

Thanks for the quick response @bgamari!

We run integration tests against a gitlab container, but for unit tests, we just mock the responses (see file linked above). We usually just use the example responses from the API docs for the mocked response. I guess we should document this in our contributing guide a bit more, thanks for the feedback! 😁

In this case, this would just be "Examples responses" in https://docs.gitlab.com/ee/api/users.html#approve-user - we only really need to test the success case.

It's not really the best example, but you could check https://github.com/python-gitlab/python-gitlab/blob/main/tests/unit/objects/test_users.py#L63-L72 to see how the fixtures are used because it's so similar (though ideally we'd have an individual response as a fixture). Let me know if you're happy to work on that, otherwise we can merge and do this after. Thanks again :)

@bgamari
Copy link
Contributor Author

bgamari commented Jun 9, 2022

Thanks for the feedback, @nejch. I'll add a few unit tests.

As requested in python-gitlab#1604.

Co-authored-by: John Villalovos <john@sodarock.com>
@nejch nejch changed the title users: Add approve and reject methods to User feat(users): add approve and reject methods to User Jun 25, 2022
@nejch
Copy link
Member

nejch commented Jun 25, 2022

@bgamari thanks again for this, I rebased your branch and squashed the commit co-authored by John to make pre-commit happy. @JohnVillalovos LGTM any other concerns on your side?

@JohnVillalovos
Copy link
Member

@bgamari thanks again for this, I rebased your branch and squashed the commit co-authored by John to make pre-commit happy. @JohnVillalovos LGTM any other concerns on your side?

@nejch LGTM. I'm good to merge if you are.

@nejch nejch merged commit f9b7c7b into python-gitlab:main Jun 25, 2022
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.

Add ban/unban and approve/reject methods to User
4 participants