Skip to content

feat: add Assets/Computer and Management/Contract endpoint support - #38

Open
baraline wants to merge 19 commits into
mainfrom
worktree-assets-and-contracts
Open

baraline wants to merge 19 commits into
mainfrom
worktree-assets-and-contracts

Conversation

@baraline

Copy link
Copy Markdown
Owner

Adds GLPI asset and contract endpoint support to the client, with an agent
skill for each family. 28 new public methods, additive throughout — no
existing signature or behaviour changes.

What's in scope

Assets/Computer is the only asset itemtype implemented, of the 24 the API
exposes. That was a deliberate choice: prove the shape on one itemtype, with
its documentation, skill and tests, before repeating it 23 times. The
contract linkage is the part most likely to generalise badly, so it is
included rather than deferred.

  • Assets/Computer — flat CRUD plus search/iter_search.
  • Computer-to-contract links — the Contract_Item join, exposed as
    list_/get_/link_/update_/unlink_computer_contract. Per-type method names
    rather than a generic list_asset_contracts, so the second asset type
    can't silently inherit a signature that doesn't fit it.
  • Management/Contract — full CRUD plus the Cost sub-resource.
  • Dropdowns/ContractType.
  • Skillsglpi-asset-workflow and glpi-contract-workflow.

The client stamps the link's itemtype itself on both the create and update
paths, because the API types that field as a free string and will happily
store Monitor on a computer's link. There's a test that passes a
deliberately wrong itemtype to prove the override holds.

Verification

The gate: 1555 tests, ruff, mypy --strict, unasync_build.py --check, and a
zero-warning Sphinx build.

All 20 models were checked field-for-field against the OpenAPI document, and
then against a live GLPI 11 instance — this repo's history is largely a
record of the two disagreeing, so the document alone isn't evidence. Diffing
each served payload against its model across Computer, Contract,
ContractCost, Contract_Item and ContractType found nothing served that isn't
modelled. Three behaviours were measured and are now pinned as tests:

  • Contract.alert is validated by nothing. The document's enum lists
    64/72 where its own prose numbers the same meanings 16/24. The server
    stores all eight of 0/4/8/12/16/24/64/72 unchanged and rejects none, so
    neither listing describes an enforced set. Modelled as a plain int — an
    enum would reject values the server accepts, and would have to pick one of
    two contradictory numberings to name them by.
  • force=True genuinely hard-deletes even though this client sends it in
    the request body while the document declares it a query parameter. Every
    teardown in the suite rests on this; a server that ignored the body would
    leave soft-deleted records accumulating while still reporting success.
  • Computer.entity carries completename and Contract.entity does
    not
    , so they use different reference types.

Contract.date_begin is datetime.date, not datetime, so it stays out of
the server-clock conversion that rewrites aware timestamps — on a date-only
field that could roll the value to the neighbouring day.

Reviewer notes

  • The integration tests skip silently without credentials, so they will
    not run in CI. They were run locally against preprod; to reproduce, run
    pytest integration_tests/ -m integration from a checkout with secrets/
    populated and confirm the count rather than trusting a green result.
  • _sync/ is generated. Review _async/ and let unasync_build.py --check
    cover the rest.
  • Known and accepted, not fixed: the three new client modules have no
    FailingTransportRecorder tests (raise paths are covered generically), and
    the list_* helpers default to limit=50 without saying so in the
    signature.

🤖 Generated with Claude Code

baraline and others added 19 commits September 16, 2026 17:11
The OpenAPI document moved from docs/glpi_api_contract.json to
docs/api_contract/api.json, but only the working copy was updated -- the
committed ignore still named the old path, so a fresh clone or worktree
does not ignore the 14 MB document at all.

pyproject.toml carries the same stale path in its sdist exclude; that one
is fixed with the rest of the packaging work.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add test_iter_search_contract_types_yields_every_page, mirroring
test_iter_search_locations_yields_every_page, so the iter_search_contract_types
start += batch_size branch is exercised. Task 1 review found the only
iteration test broke on the first page, leaving that line dead -- the same
shape every later task (2-5) would otherwise have copied.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the Contract models (Get/Post/Patch/Delete), the
GlpiContractRenewalType enum, and ContractMixin CRUD to the
Management/Contract endpoint, following the Task 1 dropdown template.
date_begin is modelled as a plain date (not datetime) since the
contract's format: date field carries no time-of-day and must stay
outside the server-clock conversion applied to aware timestamps.
costs is read-only on the client side; cost lines get their own
endpoints in a later task.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add GetContractCost/PostContractCost/PatchContractCost/DeleteContractCost
and list_contract_costs/get_contract_cost/create_contract_cost/
update_contract_cost/delete_contract_cost on ContractMixin, covering
/Management/Contract/{id}/Cost. ContractCost.date_begin/date_end are
datetime (format: date-time), unlike Contract.date_begin (format: date).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…docstrings

The multi-page pagination test docstrings in test_computer.py and
test_contract_type.py described their named-function stub as an
``async def`` guarding against a ``coroutine function``, which ships
verbatim into the generated _sync tree where the stub is a plain def.
Reworded to match the wording already used in test_contract.py, which
conveys the same warning without an async/coroutine reference.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add the /Assets/Computer/{id}/Contract sub-resource to ComputerMixin:
list_computer_contracts, get_computer_contract, link_computer_contract,
update_computer_contract, unlink_computer_contract, backed by new
GetContractItem/PostContractItem/PatchContractItem/DeleteContractItem
models mirroring Contract_Item.

link_computer_contract and update_computer_contract stamp itemtype and
items_id themselves via model_copy, overriding any caller-supplied
values, because Contract_Item.itemtype is a free string rather than an
enum and a typo there would silently mis-link the contract.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…path

test_update_computer_contract only checked the endpoint, and had no
counterpart to test_link_computer_contract_overrides_a_caller_itemtype.
A regression dropping the model_copy stamp from update_computer_contract
alone would have passed the suite undetected. Add the same stamping
assertions used on the link path, plus a dedicated override test, so
both write paths are guarded identically. Test-only; the production
code was already correct on both paths.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Also update two stale references to the API contract path from
docs/glpi_api_contract.json to docs/api_contract/api.json:
- pyproject.toml exclude list
- glpi_python_client/models/api_schema module docstring
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Replace stale docs/glpi_api_contract.json with docs/api_contract/api.json
in all module docstrings and comments:

- glpi_python_client/models/__init__.py
- glpi_python_client/_async/clients/api/__init__.py
- glpi_python_client/_async/clients/commons/_constants.py
- glpi_python_client/_sync/ (regenerated)
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add API reference sections for Computer, Contract, ContractCost,
ContractType, and GlpiContractRenewalType; add user-guide Assets and
Contracts sections covering the CRUD helpers, the computer-contract
link sub-resource, the contract-cost sub-resource, and the
date/datetime asymmetry on Contract.date_begin; add a CHANGELOG entry
and a CONTRIBUTING note on fetching the GLPI OpenAPI contract.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Added section is a claim about what is in the tree, and nothing
validates changelog prose against file existence. glpi-asset-workflow
and glpi-contract-workflow don't exist at this commit; the bullet
belongs in the task that actually creates them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds skills/glpi-asset-workflow and skills/glpi-contract-workflow,
teaching an agent the Computer, Contract, ContractCost, and
ContractType endpoint families added in Tasks 1-5, including the
itemtype/items_id override on the computer-contract link helpers and
the Contract.date_begin (date) vs ContractCost.date_begin/date_end
(datetime) asymmetry. Adds both to the skills/README.md index and
announces them in the CHANGELOG so the commit creating the skills is
the commit announcing them.

This closes the branch's last structural debt: every public method is
now named by some skill, so
test_every_public_method_is_named_by_some_skill passes with nothing
deselected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
skills/glpi-asset-workflow/SKILL.md folded a separate "update a link"
example back into the link/list/get/unlink lifecycle block, ahead of
the unlink call: the previous ordering deleted the link with
force=True and then reused link_id in a follow-on update example,
which would raise on a live server. Restored the full
`IdNameRef, PatchContractItem, PostContractItem` import list on that
block too -- it only imported PatchContractItem while also
constructing an IdNameRef, so copying the block alone raised
NameError.

skills/glpi-contract-workflow/SKILL.md was checked for the same two
problems (destructive call followed by reuse, partial import list) and
found sound -- no changes needed there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add three integration tests exercising the Task 1-8 endpoints against a
live GLPI for the first time: test_computer_contract_round_trip links a
throwaway computer and contract and asserts the join's itemtype comes
back "Computer"; test_contract_date_begin_is_stored_as_sent guards the
date-vs-datetime modelling choice on Contract.date_begin; and
test_contract_cost_round_trip exercises the Cost sub-resource create/
list/delete cycle. All three skip cleanly here for want of secrets/ and
were not run against a live server, so no divergence was found or
recorded in CHANGELOG.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dent

Round 1 review found three related teardown gaps in the previous commit's
tests, all inherited from the task brief's code:

- test_computer_contract_round_trip created the computer and the contract
  above a single try, so a raise from create_contract after
  create_computer succeeded left the computer orphaned; its finally also
  ran two deletes in sequence, so a raise from delete_computer skipped
  delete_contract.
- test_contract_cost_round_trip deleted its cost line inside the try body
  instead of a finally, so a failed assertion above it leaked the cost
  line.

Nest each resource's create/try/finally so its teardown runs regardless
of what happens to any other resource in the test, without assuming
GLPI cascades a contract's delete to its cost lines or links -- that
assumption is exactly what the nesting removes the need for.
test_contract_date_begin_is_stored_as_sent was already correct (one
resource, one try/finally) and is unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…le references

Whole-branch review found three Important issues and one Minor across
docs and code, none touching control flow:

- Computer.entity was typed IdNameRef but the contract gives it a
  completename slot like Ticket.entity; retype both GetComputer and
  PostComputer as IdNameCompletenameRef. Contract/ContractCost/Location
  entity refs were verified against the contract and left as IdNameRef.
- The Contract.alert docstring stated an unsourced 64/72 -> periodic
  alert mapping as fact; the contract's enum and description actually
  disagree on the top two values. Rewrite the docstring to state the
  contradiction plainly, give both candidate numberings, and note
  neither has been confirmed against a live server. The field stays a
  plain int.
- Drop four "(a later task)"/"(added in a later task)" parentheticals
  from _contract.py docstrings that reach published Sphinx output --
  ContractCost already exists in this branch, so the phrasing was
  stale.
- Fix a skill miscount ("four" search helpers that take no sort, when
  two of the six listed methods are search helpers) and an undefined
  contract_id used before definition in the user guide's asset-linking
  example.

Also close a real gap in the iter_search_* pagination tests for
computers and contracts: the fake_search stubs accepted sort but never
recorded or asserted it, so deleting sort=sort from either generator
would have left the suite green. Both stubs now record every sort they
receive and the tests assert it lands on every page.

Regenerated _sync via unasync_build.py for the two test file changes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ran the suite against preprod for the first time. The models were only
ever checked against the OpenAPI document, and this project's history is
largely a record of GLPI disagreeing with it.

Measured, and now pinned as tests:

- Contract.alert is validated by nothing. The document's enum lists
  64/72 where its own prose numbers the same meanings 16/24; the server
  stores all eight of 0/4/8/12/16/24/64/72 unchanged and rejects none.
  Neither listing describes an enforced set, so the docstring no longer
  presents this as an open question, and `int` is now the measured
  answer rather than a hedge between two guesses.
- `force=True` really does hard-delete despite being sent in the request
  body where the document declares it a query parameter. Every teardown
  in the suite rests on this; a server that ignored the body would leave
  soft-deleted records piling up on a shared instance while still
  reporting success.
- Computer.entity carries completename and Contract.entity does not,
  confirming the distinction live.

Field coverage also checked by diffing each served payload against its
model: Computer, Contract, ContractCost, Contract_Item and ContractType
all match exactly, with nothing served that is unmodelled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.59%. Comparing base (0d43528) to head (763f709).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
+ Coverage   97.31%   97.59%   +0.28%     
==========================================
  Files          80       90      +10     
  Lines        2826     3163     +337     
==========================================
+ Hits         2750     3087     +337     
  Misses         76       76              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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