Skip to content

Modernize tooling, fix API bugs, add type hints and generic verbs - #36

Open
tswfi wants to merge 14 commits into
masterfrom
modernize-tooling
Open

tswfi wants to merge 14 commits into
masterfrom
modernize-tooling

Conversation

@tswfi

@tswfi tswfi commented Jun 10, 2026

Copy link
Copy Markdown
Member

This is a large maintenance pass to bring the package up to date and keep it healthy. It is backwards compatible for the public surface (from procountor.client import Client, the constructor, all method names, properties, and attributes are unchanged) except for a handful of intentional correctness fixes that send different (correct) data on the wire — these are why this is slated for a 3.0.0 release.

No version bump or release is included here; that's a follow-up once this is reviewed.

Tooling / packaging

  • Migrated setup.py/setup.cfg/Pipfile/requirements.txt/tox.ini → a single PEP 621 pyproject.toml (hatchling backend), and adopted uv for env/lock management. Removed the dead .pyup.yml and MANIFEST.in.
  • Dropped Python 2; now requires Python 3.9+. Refreshed the locked deps so transitive packages (urllib3, etc.) resolve to current, non-vulnerable versions.
  • Added the previously empty LICENSE (MIT).
  • Added GitHub Actions CI (ruff + mypy + pytest matrix on 3.9–3.13) and modernized the PyPI publish workflow to build with uv.

Tests

  • The old suite required live Procountor credentials and hit the network on construction, so it could not run in CI. It now runs fully offline: the live integration tests skip unless PROCOUNTOR_RUN_INTEGRATION=1 (+ credentials), and a new mocked unit-test suite is the CI gate. 67 unit tests pass, 41 integration skip.

Code quality

  • Added type hints throughout + a py.typed marker (PEP 561). To keep the types honest, the HTTP transport/auth was extracted into procountor.transport.BaseClient, which ApiMethods now inherits (MRO: Client → ApiMethods → BaseClient).
  • Client is now importable from the top level: from procountor import Client.

Generic verbs (the strategic bit)

The Procountor API has ~192 operations; the named helpers cover ~58. Rather than hand-maintain wrappers that drift from the spec, added generic get/post/put/delete verbs so any endpoint is reachable while the client keeps handling auth + token refresh:

client.get("payrolls/salaryslips", salaryPeriodId=123)
client.post("products", json={"name": "Widget"})
client.put("invoices/123/notes", json={"note": "ok"})
client.delete("attachments/42")

request() also gained an explicit json body param (so top-level JSON arrays work). The named helpers were converted to thin wrappers over these verbs (wire-identical) to make it obvious they're just convenience.

Bug fixes (these change what is sent on the wire)

  • Request body bug: 8 write methods (update_company, update_user, update_business_partner, create_dimension_item, update_dimension_item, post_payment, payments_direct_bank_transfers, put_products_to_bank_statement) passed the body into request()'s headers arg, so no JSON body was ever sent. Fixed + regression tests.
  • get_payments queried /invoices instead of /payments.
  • get_invoice_paymentevents sent its args as a GET JSON body; they are query params (previousId, orderById, size, page).
  • Leading-slash endpoints (create_dimension_item, update_dimension_item, confirm_invoice) produced double-slash URLs.
  • update_dimension now takes a dimensionId and targets PUT /dimensions/{id} (Procountor removed PUT /dimensions around 25.11).
  • api_key was stored as a stray 1-tuple; now the documented string (wire-identical).

Deprecations

  • pay_invoice and send_one_time_pass now emit DeprecationWarning — their endpoints (/invoices/pay, /users/otp) no longer exist in the current API. They still function.

Ergonomics

  • get_bank_statements accepts date/datetime (or strings) for its date-only params. Date normalization deliberately lives in the named wrapper, not the transport — the correct format is per-parameter, so only the method that knows a field's semantics should convert it.

Docs

  • README: usage example, generic verbs, and the supported/latest/dated api_version cycle, pointing at https://dev.procountor.com/.
  • CONTRIBUTING/README release steps updated for uv + git push --follow-tags.

🤖 Generated with Claude Code

tswfi and others added 12 commits June 10, 2026 17:05
Migrate packaging from setup.py/setup.cfg to a PEP 621 pyproject.toml with
the hatchling backend, and adopt uv for dependency/environment management.
Remove Pipfile, Pipfile.lock, requirements.txt, tox.ini, MANIFEST.in and the
dead .pyup.yml. Drop Python 2 support (requires-python >=3.9) and refresh the
locked dependencies so transitive packages (urllib3 etc.) resolve to current,
non-vulnerable versions.

Make the test suite runnable offline: the live integration tests now skip
unless PROCOUNTOR_RUN_INTEGRATION=1 and credentials are present, and a new
mocked unit-test suite covers client URL/header/token/response logic and
api_methods delegation (43 passing, 41 skipped).

Add a GitHub Actions CI workflow (ruff + pytest on Python 3.9-3.13) and
modernize the PyPI publish workflow to build with uv. Add the previously
empty LICENSE (MIT). Minor hardening: fix a bare except, a dict
string-concatenation in _get_token's error path, and two test modules that
called unittest.main() without importing unittest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Non-breaking follow-ups to the tooling modernization:

* docs/source/conf.py now reads the version from the package instead of a
  hardcoded 0.0.1.
* CONTRIBUTING.rst updated for the uv-based workflow (drop virtualenv/setup.py/
  tox/travis/bumpversion references, Python 2.7/3.6 support note).
* Replace the mutable default argument in Client._create_endpoint with a None
  sentinel.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client.__init__ stored the API key as a 1-tuple (`(api_key,)`), almost
certainly an accidental trailing comma. The form-encoded token request happened
to produce identical output, so this is wire-identical, but the public
`client.api_key` attribute is now the string it is documented to be.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix eight write methods (update_company, update_user,
update_business_partner, update_dimension, create_dimension_item,
update_dimension_item, post_payment, payments_direct_bank_transfers) that
passed the request body into request()'s `headers` positional argument, so no
JSON body was ever sent. They now forward the body as keyword arguments like
the other write methods. This changes what these methods send on the wire and
is covered by new regression tests.

Add type hints across the package and a py.typed marker so the types are
exposed to downstream users (PEP 561). To make the mixin types honest without
TYPE_CHECKING stubs, the HTTP transport/authentication is extracted into a new
procountor.transport.BaseClient that ApiMethods now inherits from; the public
procountor.client.Client (and all its methods/attributes) is unchanged, and
Client is additionally exported from the top-level package.

Wire mypy + types-requests into the dev dependencies and the CI lint job, and
update the package author/maintainer metadata.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* get_payments queried the "invoices" endpoint instead of "payments".
* create_dimension_item, update_dimension_item and confirm_invoice had a
  leading slash in the endpoint path, producing a double slash in the URL.
* Add regression tests for all three and a Usage/Installation section to the
  README.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Procountor API exposes ~192 operations; the named helpers cover only a
subset. Rather than hand-maintain wrappers that drift from the spec, add
generic verbs on the transport layer so any endpoint is reachable while the
client keeps handling auth and token refresh:

    client.get("payrolls/salaryslips", salaryPeriodId=123)
    client.post("products", json={"name": "Widget"})
    client.put("invoices/123/notes", json={"note": "ok"})
    client.delete("attachments/42")

request() now accepts an explicit `json` body so top-level JSON arrays can be
sent (the kwargs path can only build an object).

Three methods target endpoints that no longer exist in the current API
(pay_invoice -> /invoices/pay, send_one_time_pass -> /users/otp,
update_dimension -> PUT /dimensions); they now emit a DeprecationWarning.

Document the generic verbs and the supported/latest/dated api_version cycle in
the README, pointing at https://dev.procountor.com/.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the 60 simple endpoint helpers to call get/post/put/delete instead of
request() directly, making it explicit that they are thin convenience wrappers.
This is wire-identical: get(endpoint) == request("GET", endpoint) and
post(endpoint, json=data) == request("POST", endpoint, **data).

post_attachment (multipart upload with custom headers) and
get_invoice_paymentevents (sends a GET body) keep calling request() directly to
preserve their exact behaviour.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_bank_statements now accepts date/datetime objects (or strings) for its
date-only startDate/endDate parameters, formatting them to yyyy-MM-dd via a
small _as_date_string helper.

The conversion lives in the named wrapper, not the transport layer, on purpose:
the correct format is per-parameter (some Procountor fields are date-only,
others are full datetime), so only the method that knows a field's semantics
should normalise it. A blanket transport-level conversion would silently send a
full datetime to a date-only field.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Procountor removed PUT /dimensions (around version 25.11); updating a dimension
is now PUT /dimensions/{dimensionId}. Give update_dimension a dimensionId
argument and point it at the correct endpoint, replacing the earlier
deprecation. Signature change is acceptable for the upcoming 3.0.0.

The default-dimension endpoints added in 25.11
(/products|businesspartners|persons/{id}/defaults/dimensions[/{dimensionId}])
are intentionally left to the generic verbs rather than dedicated helpers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per the Procountor OpenAPI description, previousId, orderById, size and page on
GET /invoices/{invoiceId}/paymentevents are query parameters. The method was
sending them as a JSON body on the GET request; build them into the query
string instead. post_attachment is now the only method that calls request()
directly (multipart upload).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bump-my-version creates an annotated tag, so the commit and tag can be pushed
together with `git push --follow-tags` instead of separate `git push` and
`git push --tags` commands. Clarify the release steps in CONTRIBUTING.rst and
README.rst.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
astral-sh/setup-uv publishes major aliases only up to v7; v8 exists only as
exact tags (v8.2.0). Pin the exact version so the workflows resolve.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tswfi
tswfi requested a review from samaij June 10, 2026 20:06
Comment thread procountor/transport.py Outdated
Replace the "TODO: This text" placeholder docstring on _handleResponse with a
proper description of the returned dict shape. Remove the long commented-out,
never-finished test_invoice draft in test_010_invoices.py (it no longer matches
the current response shape and references a now-deprecated method); git history
keeps it. The remaining "write tests for X" markers are left as they flag real
missing integration tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants