[WIP] Implement missing API functions

[philipflohr]

As written in #7 my work is not yet finished and is not ready to be pulled.

Besides the missing functionality there are no tests. Also I'm actually not sure if the remote_query_id flag should default to True or False - I chose False for a simple reason: without explicitly allowing a query with this default value it's only possible to update an attendance record which already exists on the Personio servers.

[codecov-io]

Codecov Report

Merging #8 into master will decrease coverage by 9.31%. The diff coverage is 17.18%.

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
- Coverage   84.49%   75.18%   -9.32%     
==========================================
  Files           6        6              
  Lines         729      826      +97     
==========================================
+ Hits          616      621       +5     
- Misses        113      205      +92     

Impacted Files	Coverage Δ
src/personio_py/client.py	53.02% <11.49%> (-26.15%)	:arrow_down:
src/personio_py/models.py	81.08% <29.26%> (-3.85%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2bb8a03...f4476cb. Read the comment docs.

[klamann]

@philipflohr thanks for your work on this PR. I'm gonna make a short review of the code now and add a few comments.

One thing that would be really important though are tests. The most useful and thorough tests we have right now are in test_mock_api.py, where the response from the server for certain inputs is mocked. Would you be willing to add tests for the API methods you have implemented?

[philipflohr]

@klamann are the test provided for absences ok like this?

[philipflohr]

@klamann can you give any update on this or do you have an idea when you'll have a little time for it?

[klamann]

Hey @philipflohr, sorry for letting you wait. I plan to review your changes on the weekend, maybe earlier if I get the chance. Are you finished with your additions or do you want to add more to this PR?

[philipflohr]

There are still test missing for attendances. Could have a quick look into the absences tests? If they are ok I'll provide similar tests for attendances

[klamann]

I had a look at the tests; what I like is that there are many tests that work with a live Personio instance and where many edge cases are covered. What I'm missing however are tests that can run as part of the CI pipeline without a live Personio instance, like those defined in test_mock_api.py. I know those are more difficult to define, but they are really important to ensure that the API functions are still working when other parts of the implementation are changed.

Also, I'm starting to worry that this PR is getting out of hand, there's too much structural change in here (e.g. the refactoring of the tests in multiple files) and it's becoming increasingly more difficult to review. I think it would be wise to split this into several smaller PRs that are easier to edit and review. We could easily merge the small changes to client.py and if we have focused PRs for attendances & absences, I could help you with the mock API tests. With the current state of the PR, I don't feel comfortable to merge the whole package.

[philipflohr]

I'm closing this PR in favor of multiple smaller PRs.