Add implementation for Members, Sources and Invitations endpoints

[GG-Yanne]

Add support for managing users

This PR implements all endpoints required for managing users on an account as well as adding support for invitations and sources handling

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 93.38677% with 33 lines in your changes missing coverage. Please review.

Project coverage is 93.99%. Comparing base (391408f) to head (5133106).

Files with missing lines	Patch %	Lines
pygitguardian/client.py	82.55%	26 Missing ⚠️
pygitguardian/models.py	98.26%	5 Missing ⚠️
pygitguardian/models_utils.py	96.72%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
- Coverage   94.55%   93.99%   -0.56%     
==========================================
  Files           6        7       +1     
  Lines         918     1382     +464     
==========================================
+ Hits          868     1299     +431     
- Misses         50       83      +33     

Flag	Coverage Δ
unittests	93.99% <93.38%> (-0.56%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Alustrat-gg]

Good Job Yanne 👍

I just think that we sometime have incoherences in the way we pass parameters/queries/bodies but it's a detail

[agateau-gg]

I usually do inline reviews, but that PR is too big to make this practical.

Regarding VCR cassettes

If it's not possible to write a script to automatically setup the account, then can you create a Markdown document in doc/dev/ (We have a similar directory in ggshield, but you would have to create it there) to list all the requirements for the account in one place? This would be more efficient than having to look for comments in different tests to gather them all.

client.py

update_*() methods

• either make them all return Optional[Detail] or Union[Detail, <ModelClass>] (update_team_source() returns Union[Detail, int]).
• be consistent with argument names because people are going to use keyword arguments for them and changing them after a release is an API breakage. So either we call them all payload or w call theom all <name_of_the_model>. I prefer payload because what they take is not an instance of the model, it's an instance of Update<Model>.

Misc

Rename list_teams_sources() to list_team_sources() for consistency.

Remove the "obj.status_code" useless lines (grep -n "^ *obj\.status_code$" client.py to find them)

models.py

@post_load methods

• Type-hint for **kwargs must be Any.

• All existing @post_load methods are called make_<something>. Can you use the same naming convention for yours?

conftest.py

get_member() is never used, can we remove it?

Instead of creating fixtures, you can create a test.utils module with plain functions and import them in the tests.

[GG-Yanne]

Regarding VCR cassettes

If it's not possible to write a script to automatically setup the account, then can you create a Markdown document in doc/dev/ (We have a similar directory in ggshield, but you would have to create it there) to list all the requirements for the account in one place? This would be more efficient than having to look for comments in different tests to gather them all.

I added doc/dev/tests.md akin to what is done for GG Shield and listed all hard requirements for the workplace to be able to run the test suite without issues

update_*() methods

• either make them all return Optional[Detail] or Union[Detail, <ModelClass>] (update_team_source() returns Union[Detail, int]).

update_team_source works in a particular way, it does not return a ModelClass since we are adding and removing sources and the endpoint does not return them, see : https://api.gitguardian.com/docs#tag/Team-Sources/operation/update-team-sources

In any case, I applied the same logic as for delete endpoints and optionally returning a Detail object if the status code is not the one we expect for a successful call 👍

• be consistent with argument names because people are going to use keyword arguments for them and changing them after a release is an API breakage. So either we call them all payload or w call theom all <name_of_the_model>. I prefer payload because what they take is not an instance of the model, it's an instance of Update<Model>.

I changed all updates to payload so they are all aligned 👍

Misc

Rename list_teams_sources() to list_team_sources() for consistency.

This was already taken care of in a later commit 🙌

Remove the "obj.status_code" useless lines (grep -n "^ *obj\.status_code$" client.py to find them)

grep -n "^ *obj\.status_code$" client.py does not return anything anymore 👍

models.py

@post_load methods

• Type-hint for **kwargs must be Any.

I changed kwargs typing to be Any 👍

• All existing @post_load methods are called make_<something>. Can you use the same naming convention for yours?

I changed all @post_load method to be respect this convention

conftest.py

get_member() is never used, can we remove it?

Indeed, I removed it 👍

Instead of creating fixtures, you can create a test.utils module with plain functions and import them in the tests.

I create the corresponding utils file and updated references for these functions

[agateau-gg]

Looks good now! Thanks for this significant addition to the API and for documenting the test setup requirements 👍🏻.