at-gmbh / personio-py

a lightweight Personio API client
https://at-gmbh.github.io/personio-py
Apache License 2.0
26 stars 17 forks source link

[WIP] Feature/attendances #13

Closed philipflohr closed 1 year ago

philipflohr commented 3 years ago

This implements the attendance methods.

Still missing: Mock tests, in detail raw api tests

This PR needs #11 to be merged first

klamann commented 3 years ago

I resolved the merge conflicts with master, but the tests fail. might be incomplete mock data, not sure though.

philipflohr commented 2 years ago

@klamann I'll get some time to finally integrate my changes to personio-py. Mock tests and tests using the authenticated API are available. The Sphinx build currently fails due to a dependency issue (mistune 0.8.4 / 2.0.0).

klamann commented 2 years ago

Hey @philipflohr, glad to hear it! I'm in the middle of the refactoring for personio-py 1.0, but if we merge this soon, I can integrate it into the refactored codebase.

To resolve the sphinx dependency issue, please add mistune = "0.8.4" to requirements-dev.txt

philipflohr commented 2 years ago

@klamann Pinning the package did not resolve the issue. From a very quick look at the logfile I guess that Sphinx has the same dependency issue

codecov-commenter commented 2 years ago

Codecov Report

Merging #13 (901f06f) into master (a4c8a5b) will decrease coverage by 0.03%. The diff coverage is 76.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   86.68%   86.65%   -0.04%     
==========================================
  Files           7        7              
  Lines         841      884      +43     
==========================================
+ Hits          729      766      +37     
- Misses        112      118       +6     
Impacted Files Coverage Δ
src/personio_py/client.py 78.67% <69.69%> (-0.79%) :arrow_down:
src/personio_py/models.py 90.45% <86.36%> (+0.62%) :arrow_up:

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 a4c8a5b...901f06f. Read the comment docs.

philipflohr commented 2 years ago

Hi @klamann

I had closer look at the docs-verify action. In my opinion this is the line where pinning the mistune package needs to be done pre-build-command: "pip install sphinx_rtd_theme~=0.4.3 recommonmark~=0.6.0 m2r~=0.2.1"

philipflohr commented 2 years ago

@klamann Could you have a look if the docs-verify check is fixed if you install the correct mistune package in the prebuild command?

klamann commented 2 years ago

I solved this in #24 with this set of dependencies:

Sphinx = "^4.3.2"
sphinx-rtd-theme = "^1.0.0"
recommonmark = "^0.7.1"
docutils = "0.16"
mistune = "0.8.4"
m2r = "^0.2.1"

the order of the dependencies should not matter, but I think you're missing docutils = "0.16" (I think there was an issue with the latest version, can't remember what it was though).

Also, you can change the pre-build-command in docs-pr.yml to pip install -r requirements-dev.txt if you want. All of this is changing in #24 anyways.

philipflohr commented 2 years ago

Hi @klamann the checks are passing - do you think there is anything left to do before this can be merged?