diff --git a/CHANGELOG.md b/CHANGELOG.md index b206869..9dfab7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,21 +17,54 @@ is the error. Tags carry no `v` prefix. ## [0.4.0] - 2026-10-02 -Adds Markdown <-> memo HTML conversion as an optional extra, and drops Python -3.10. A `0.4.0` section was first prepared on 2026-09-30 around a different +Adds Markdown <-> memo HTML conversion as an optional extra, drops Python +3.10, and **stops a ticket's workflow changing by accident -- which is a +breaking release.** `set_status` is gone (it was the vendor close request under +a name that hid it), `close_ticket` requires an explicit opt-in, and every +write the package can tell may change a ticket's workflow is refused before it +is sent unless the call says so. What it cannot tell is not covered (see +*Notes*). + +A `0.4.0` section was first prepared on 2026-09-30 around a different converter. It was never tagged or uploaded, and this section replaces it; `### Changed since the 2026-09-30 preparation` says what moved, for anyone who built against that branch. -**Upgrading.** One breaking change: **Python 3.10 is no longer supported** -(see `### Removed`). On 3.10, pip keeps resolving `0.3.0`, the last release -that installs there. On 3.11 and later, **nothing moves for a caller who does -not install the extra**: the core package imports none of its dependencies, no -existing call or model changes, and the one new name at the package root is an -exception class. +**Upgrading.** **Python 3.10 is no longer supported** (see `### Removed`). On +3.10, pip keeps resolving `0.3.0`, the last release that installs there. The +converter is purely additive: the core package imports none of the extra's +dependencies. Every other breaking change is in the workflow guard, and each is +marked `**BREAKING**` in `### Changed` or `### Removed`. Read `### Upgrading` +below. ### Added +- **The workflow guard.** `WorkflowEffect` (`INTERRUPTS`, `ADVANCES`, + `UNKNOWN`) and `EasyvistaWorkflowEffectRefused`, both exported at the package + root, and `easyvista_python_client.workflow` (`workflow_triggers`, + `classify_workflow_effects`), which names what a write may do to a ticket's + workflow. `EasyvistaWorkflowEffectRefused` is a `ValueError`, **not** an + `EasyvistaError`: the refused write is never sent, so there is no status code + and nothing transient, and it carries `effects` and `triggers` (what named + them). A ticket's status follows its workflow -- "Advancing through the steps + of a workflow changes the status of a ticket." (tier 1) -- so a write that + touches workflow state is not a bookkeeping write. +- An `allow_workflow_effect=` keyword, taking one `WorkflowEffect` or an + iterable of them, on `send`, `update_ticket`, `create_action`, `create_task`, + `update_action` and `end_action` (default: allows nothing), and **required** + on `close_ticket`. `RequestSpec.allow_workflow_effect` and + `RequestSpec.allowing()` carry the same opt-in on a request spec. +- `resources.actions.build_get_action(..., fields=...)` projects the item read, + as the list builders already did. +- `reassign_action(action_id, *, group_id=None, done_by_id=None)` on both + clients, with `resources.actions.build_reassign_action`: reassign an action + (for example, escalate the open workflow step to another group) without + ending it. It is not refused by the workflow guard. The vendor documents no + reassignment route (tier 1), so the effect was measured: 2026-10-02, one + instance, two tickets, so it may not generalise -- the group was stored, the + step stayed open, the ticket's status did not move and no new action rows + appeared; the ticket's owning group was read on one of the two tickets and + did not follow the action's. The person write (`done_by_id`) is unmeasured. - `easyvista_python_client.content.EasyvistaContentConverter`, behind the new optional extra `easyvista-python-client[content]`. It has two static methods. `from_transport(value, *, plain_text_is_markdown=False)` reads a @@ -121,6 +154,84 @@ exception class. imports it. `testing/test_public_api.py` fails if the copies drift, or if its list of the extra's import names stops matching `pyproject.toml`. +### Changed + +- **BREAKING** `close_ticket` requires `allow_workflow_effect=`, keyword-only + with no default: leaving it out is a `TypeError`, and a value that does not + include `WorkflowEffect.INTERRUPTS` is refused with + `EasyvistaWorkflowEffectRefused` before any request is made. The vendor + documents the close request as interrupting the workflow (tier 1, see + *Removed*), so the call site now says that it means to. +- **BREAKING** `end_action` is guarded. Unless `allow_workflow_effect` includes + `WorkflowEffect.ADVANCES` it first reads the action (one item read projecting + `ACTION_ID` and `WORKFLOW_ID`) and refuses, with no end request sent, when + the action is a workflow step (`WORKFLOW_ID` set), when the record comes back + without a `WORKFLOW_ID` column (which cannot be told from a step), or when + the read returns a different `ACTION_ID` than the one asked for. `end_all=True` + always needs `ADVANCES`. Ending an action you created yourself needs no + opt-in (see *Notes*). If the read fails, its error propagates and nothing is ended. + Ending a workflow step moves the workflow on -- vendor-documented only by the + UI's Finish wizard, and measured on one instance on 2026-09-01 (2 of 2), so + it may not generalise. +- **BREAKING** `end_action`'s explicit `action_id` must be a positive integer. + `0`, negatives, blanks, RFC numbers, floats and booleans now raise + `ValueError` before any request, and a numeric string is sent as an integer + (`"123"` goes out as `123`). `action_id=None` is still refused unless + `end_all=True`. +- **BREAKING** `update_ticket`, `update_action`, `create_action`, `create_task` + and `send` refuse a body or route that may change the workflow unless the call + passes `allow_workflow_effect=`. Named: the workflow-control bodies `closed`, + `end_action`, `suspended` and `restarted` as a top-level key in any casing on + any path; on a `requests/{rfc}` route, the status, catalog and parent-request + columns and a `DELETE`; on an existing `actions/{id}` route, the end date, + type, parent, ticket and workflow, stage and step columns (creating an action + or a task names a narrower set); a write to `actions/` where `` is not an + integer id, which is the vendor's end-action route `PUT + actions/{rfc_number}`, whatever the body says; every ticket sub-route that is + a command rather than a record (`close`, `suspend`, `restart`, + `workflowstart`, ...) and `requests/without-workflow`. The column rules apply + to the `requests/` and `actions/` routes only: a route of any other family is + not classified by column. What the typed models declare needs no opt-in, + and neither do text, owner, group, done-by, impact or urgency. The exact + lists are in `docs/vendor-api-reference.md`, "Ticket workflow". **This is a + deny-list, and a deny-list of columns cannot be complete**: what is not named + is unclassified, not proven neutral. +- **BREAKING** `update_action` refuses an action id that is not a positive + integer, `None` included: `PUT actions/{rfc_number}` is the end-action route + on the same path, so an RFC number would not edit an action. +- **BREAKING** The spec builders changed with the guard. A spec from + `resources.requests.build_close_ticket` or `resources.actions.build_end_action` + names a workflow effect, so this package's transport refuses it until it is + passed through `.allowing(...)`; code that builds one and sends it itself must + say so. And `resources.actions.build_end_action` and `build_update_action` + now raise `ValueError` at build time for an action id that is not a positive + integer, where they formerly passed the id through as given. +- **BREAKING** `send()` -- the path every typed method and the client's own + `send` go through -- refuses outright, with `ValueError` and whatever the + method or opt-in, a path containing a dot segment (`.` or `..`), a + percent-encoded slash or backslash, or a raw backslash: the HTTP client + collapses a dot segment, and a server may read the others as a separator, so + the request could reach a route other than the one that was checked. No API + route needs one. Whether this server reads them as separators is not + measured; the check fails closed. Document downloads (`get_bytes`, + `stream_bytes`) are reads and are never gated. +- **BREAKING** A request is treated as a read only when its method **and** the + value of every method-override header (`X-HTTP-Method-Override`, + `X-HTTP-Method`, `X-Method-Override`) on the request are reads, so such a + header cannot hide a write behind a `GET`: a request whose override header + names a write is classified as that write, and refused if it names a workflow + effect and the call did not allow it, whatever method it is sent as. The + headers read are the ones that go on the wire, `config.extra_headers` with + the request's own laid over them. Whether this API honours these headers is + not recorded. +- **BREAKING** A write that names a workflow effect and is allowed is sent + **once**, never retried: `close_ticket` and `end_action` formerly retried a + 429, a 5xx or a connection error when `max_retries` was above its default of + `0`, and now raise after the first attempt. Each close inserts another + anticipated closing action and each end ends whatever is open, so a resend + after a lost response is not a repeat of the same request. This includes + `end_action` on your own action. Every other request keeps its retries. + ### Removed - **BREAKING** — Python 3.10. `requires-python` is now `>=3.11`, the 3.10 @@ -131,6 +242,24 @@ exception class. dependency on 3.10 only) and `tomli` (in `dev` and `docs`). Nothing moves on 3.11 and later. The timestamp parser keeps the normalisation it carried for 3.10, so it accepts and refuses the same values as before. +- **BREAKING** `EasyvistaClient.set_status` / `AsyncEasyvistaClient.set_status` + and `resources.requests.build_set_status`. They sent the vendor CLOSE request, + which the vendor close page documents (tier 1, re-read 2026-10-02) as + interrupting the workflow, setting the final status, deleting the unfinished + actions when `delete_actions` is set (otherwise, by the package's reading of + the page, ending them) and inserting an anticipated closing action, none of it + conditional on the status sent. The page documents final statuses only, so + for a non-final one that is an extrapolation it neither exempts nor covers. + A synchroniser that used `set_status` to mirror an intermediate status closed + tickets early. The root cause was established on 2026-10-01/02 from the + synchroniser's code (it sent the close request right after every create and on + every status push) and from the vendor page; the drain of the open workflow + action across such a write was measured on one ticket (2026-09-01, one + instance, tier 4, so it may not generalise). The vendor documents no status + setter and this package has none: a flat status update is excluded from the + vendor's ticket update body (tier 1) and was seen to return 200 while dropping + the status (0.2.0 entry below), and a ticket's status follows its workflow. + `close_ticket` and `resources.requests.build_close_ticket` remain. ### Changed since the 2026-09-30 preparation @@ -177,8 +306,73 @@ None of this reached PyPI, so it is not a break for anyone upgrading from 4.15.0 (measured 2026-09-30), so the floor is now 4.15 and the workaround is removed. +### Upgrading + +- `client.set_status(rfc, status_guid=g)` -- delete it; nothing replaces it, + because the vendor documents no status setter (a flat status update is + excluded from its ticket update body, and was seen to drop the status -- see + 0.2.0) and a ticket's status follows its workflow. To move a ticket through + its workflow, end the step's open action with + `end_action(rfc, action_id=..., allow_workflow_effect=WorkflowEffect.ADVANCES)`. + The status that follows is the workflow's, not yours to choose. This is not + documented on the REST page, which is silent about the workflow; it was + measured on one instance on 2026-09-01 (2 of 2 tickets) and may not + generalise, so re-read the ticket afterwards. To close, call + `close_ticket(rfc, allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=g)`. +- `end_action` callers: pass the integer id of an action you read, never an RFC + number, `0` or a blank. Ending your own action still needs no opt-in (see + *Notes*), and now costs one extra item read. Ending a workflow step, or any action whose record + shows no `WORKFLOW_ID`, needs `WorkflowEffect.ADVANCES`, and so does + `end_all=True`. +- Code that puts a status, catalog or other named column into `extra_payload`, + or calls `send` with a workflow route or body, now raises until it passes + `allow_workflow_effect=`. `WorkflowEffect.UNKNOWN` means undocumented, not + harmless: read "Ticket workflow" in `docs/vendor-api-reference.md` first. +- Code that builds a close or end-action spec with `build_close_ticket` or + `build_end_action` and sends it through the transport itself: pass the spec + through `.allowing(...)` first, and pass `build_end_action` and + `build_update_action` an integer action id, never an RFC number. +- Catch `EasyvistaWorkflowEffectRefused` (or `ValueError`) where you record + per-record failures. It is a `ValueError`, **not** an `EasyvistaError`, and + carries no status code; it is never transient. Code that catches + `EasyvistaError` around a close for cleanup will NOT catch it. `ValueError` + also catches the other local refusals above. +- If you set `max_retries` above its default of `0`, a lost response to an + allowed workflow write now surfaces as an error instead of a silent resend. + Re-read the ticket before repeating it. +- The minor bump is deliberate: a dependant pinned `>=0.3.0,<0.4` does **not** + pick this up, and must widen its constraint on purpose. + ### Documentation +- `docs/vendor-api-reference.md` gains "Ticket workflow": what each documented + write does to a workflow, with the vendor page quoted (tier 1), the guard's + deny-list exactly as the code holds it, and the measurements labelled tier 4 + with their instance and date. O-CLOSE-DEFAULT is closed at tier 1: the vendor + close page documents an omitted `status_GUID` as defaulting to the Closed + meta-status. It was not measured here. +- The README, the user guide, the API reference and the `easyvista-client-setup`, + `easyvista-ticket-actions` and `easyvista-ticket-workflow` skills describe the + guard; the `easyvista-instance-discovery` skill now says `close_ticket` stops + the workflow and is not a way to pick an intermediate status. The + ticket-workflow skill's first gotcha is now that `close_ticket` is not a + status setter. +- The `PostRequest.workflow_start` docstring records that the flag is a no-op + (tier 4: two tickets identical but for it came back byte-identical, 2026-09-01, + one instance), so `workflow_start=False` does not create a ticket without its + workflow. The vendor create page documents no such parameter and states that + the workflow is started; the workflow-less create is the virtual-agent route + `requests/without-workflow`, which the guard refuses unless allowed. +- **Retracted:** that `set_status` reaches every status, as the 0.2.0 entry + below put it ("a fresh ticket landed on exactly the status requested every + time, non-terminal ones included") and the `RequestUpdate` docstring repeated + it ("That route reaches **every** status, not just terminal ones"). Six status + GUIDs were tried and each landed, but that was measured by re-reading the + status only: the measurement never looked at the workflow or the ticket's + open actions, and the vendor close page documents the close request as + interrupting the workflow (tier 1). A status that landed is not evidence that + nothing else moved, and that finding was read as a safe status setter, which + it was not. The 0.2.0 section is left as written. - `docs/content.rst` is a user-guide page for the converter. It covers what a memo holds, what each direction reads and writes, what text is escaped and why, and what survives a round trip and what is lost. It also covers what @@ -207,6 +401,17 @@ None of this reached PyPI, so it is not a break for anyone upgrading from ### Notes +- What the workflow guard does not establish. It is a deny-list: the vendor's + update pages accept every column of the ticket and action tables except an + excluded list (tier 1), and a per-instance business rule can fire on + any write, so a write the guard does not name is unclassified, not proven + neutral. `end_action` tells a workflow step from your own action by + `WORKFLOW_ID` (1500 of 1500 rows, 2026-09-02, one instance); whether an + action created under a step by `create_action` carries one is unmeasured, and + if it does, ending it is refused too -- the safe direction. The live checks + in `integration_tests/test_live_workflow_guard.py` are gated: its first two + tests read only, and the rest write and run only when + `EASYVISTA_TEST_RUN_WORKFLOW_CENSUS=1` is set. - **The 15 fixes on top of `glpi_python_client` `917f030`**, measured together on this package's synthetic corpus and on the 367-memo sample, and still to be proposed to `glpi_python_client`: diff --git a/README.md b/README.md index 3fe224e..91bfb9c 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ from easyvista_python_client import ( EasyvistaClient, EasyvistaConfig, PostRequest, + WorkflowEffect, ev_equals_filter, ) @@ -80,12 +81,10 @@ with EasyvistaClient(config) as client: for t in client.iter_tickets(search=open_status, page_size=100, max_records=1000): ... # async: `async for t in client.iter_tickets(...)` - # close it with your instance's "closed" status GUID. Every argument is - # optional -- `client.close_ticket(ticket.rfc_number)` sends the close with - # no status of its own, but where that lands the ticket is not established - # by this package; see the user guide before relying on it. + # close only when closing is the intent: it interrupts the ticket's workflow. client.close_ticket( ticket.rfc_number, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid="{00000000-0000-0000-0000-000000000000}", delete_actions=1, comment="Resolved", @@ -143,7 +142,15 @@ client.end_action( > action only ends it; ending the ticket's open workflow step advances the > workflow and moves the ticket's status. Naming `action_id` is therefore > required — the vendor's id-less "end every open action" form is behind an -> explicit `end_all=True`. +> explicit `end_all=True`. Ending a workflow step, or ending every open action +> with `end_all=True`, is refused unless the call passes +> `allow_workflow_effect=WorkflowEffect.ADVANCES`. Ending an action you +> created yourself needs no opt-in, with one unmeasured exception: whether an +> action `create_action` creates under the workflow step carries a +> `WORKFLOW_ID` has not been measured, and if it does, `end_action` refuses to +> end it without `ADVANCES` (the safe direction). +> The vendor documents no status setter, and this package has none: a ticket's +> status follows its workflow (user guide, "Changing a ticket's status"). ## Assets and documents diff --git a/docs/api_reference.rst b/docs/api_reference.rst index 12d6183..2308b35 100644 --- a/docs/api_reference.rst +++ b/docs/api_reference.rst @@ -146,6 +146,25 @@ direction does, what survives a round trip, and why it is not a sanitiser. .. autoclass:: easyvista_python_client.content.EasyvistaContentConverter +Workflow guard +-------------- + +A write that may change a ticket's workflow is refused before it is sent unless +the call allows the effect explicitly. See the module docstring for what is +named and why, and ``docs/vendor-api-reference.md``, "Ticket workflow". + +.. automodule:: easyvista_python_client.workflow + :no-members: + :no-special-members: + +.. autoclass:: easyvista_python_client.workflow.WorkflowEffect + +.. autofunction:: easyvista_python_client.workflow.as_effects + +.. autofunction:: easyvista_python_client.workflow.workflow_triggers + +.. autofunction:: easyvista_python_client.workflow.classify_workflow_effects + Exceptions ---------- @@ -165,6 +184,8 @@ Exceptions .. autoexception:: easyvista_python_client.exceptions.EasyvistaContentError +.. autoexception:: easyvista_python_client.exceptions.EasyvistaWorkflowEffectRefused + Resource engine --------------- diff --git a/docs/user_guide.rst b/docs/user_guide.rst index a8481a1..3dfa03c 100644 --- a/docs/user_guide.rst +++ b/docs/user_guide.rst @@ -105,7 +105,7 @@ The short version is one call: print("gap:", gap, reason) for status in profile.references["STATUS"]: - # .guid is what close_ticket and set_status address a status by. + # .guid is what close_ticket addresses a status by. print(status.id, status.label, status.guid) That is :meth:`~easyvista_python_client.EasyvistaClient.describe_instance`; see @@ -169,7 +169,8 @@ returns ``TITLE`` empty, for instance, so a listing wants **Never infer "closed" from a status id.** They are per-instance: on the verified instance ``8`` is *Clôturé* and ``12`` is *En cours* — adjacent numbers, opposite meanings. ``end_date_ut`` is the portable signal: empty on - an open ticket, stamped on a closed one. + an open ticket, stamped once it is resolved or closed -- at resolution, not + closure (measured 2026-09-02 on one instance; it may not generalise). Step 6 — **pin what you found in your own configuration.** This package holds no registry of instance values and never will: they belong to your deployment, @@ -207,9 +208,11 @@ asynchronous client inside an event loop (FastAPI, aiohttp) or for concurrent fa a ticket, 7 branches for a department. Two practical consequences. ``max_retries`` defaults to ``0``, so raise it if you fan out — a - 429 from a rate-limited instance is not retried otherwise. And share one open client across your - tasks rather than opening one per task: ``aclose()`` is terminal and is not reference-counted, so - the first ``async with`` block to exit closes the client for everyone still using it. + 429 from a rate-limited instance is not retried otherwise (a write that names a workflow effect + and was allowed with ``allow_workflow_effect=`` is sent once whatever ``max_retries`` says). And + share one open client across your tasks rather than opening one per task: ``aclose()`` is + terminal and is not reference-counted, so the first ``async with`` block to exit closes the + client for everyone still using it. ``create_tickets`` is deliberately **not** concurrent. Those are writes, EasyVista assigns the RFC number server-side, and a failure part-way through a concurrent batch would leave you unable @@ -344,41 +347,43 @@ Create several tickets in one call with :meth:`~easyvista_python_client.Easyvist PostRequest(catalog_code="INC_STANDARD", title="Printer B down"), ]) -Fetch, update, and close a ticket by its RFC number: +Fetch, update, and close a ticket by its RFC number. Closing is not a status +change -- it interrupts the ticket's workflow, which is why the call must say so +with ``allow_workflow_effect`` (see :ref:`changing-a-tickets-status`): .. code-block:: python - from easyvista_python_client import RequestUpdate + from easyvista_python_client import RequestUpdate, WorkflowEffect fetched = client.get_ticket(ticket.rfc_number) client.update_ticket(ticket.rfc_number, RequestUpdate(description="Updated details")) - # Close with your instance's "closed" status GUID. + # Close with your instance's "closed" status GUID. Close only when closing + # is the intent: it interrupts the ticket's workflow. client.close_ticket( ticket.rfc_number, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid="{00000000-0000-0000-0000-000000000000}", delete_actions=1, comment="Resolved", ) - # Every argument is optional -- this sends the close with no status of its - # own, letting the instance decide where the ticket lands. - client.close_ticket(ticket.rfc_number) - # Verify by re-reading, not by the return value: end_date_ut is empty on an - # open ticket and stamped on a closed one, and is more portable than any - # status id (on the verified instance 8 is "Clôturé" and 12 is "En cours"). + # open ticket and stamped once it is resolved or closed, and is more portable + # than any status id (on the verified instance 8 is "Clôturé" and 12 is + # "En cours"). assert client.get_ticket(ticket.rfc_number).end_date_ut is not None .. warning:: - Where a ticket lands when ``status_guid`` is omitted is **not established by - this package**. The client simply omits the key; what the server does with a - status-less ``closed`` body has never been measured against a live instance - here, and the behaviour is not recorded in ``docs/vendor-api-reference.md``. - Try it on a throwaway ticket and re-read before you build on it. Passing - your instance's closed ``status_guid`` explicitly is the form this package's - live suite actually exercises. + Where a ticket lands when ``status_guid`` is omitted is **not measured by + this package**. The client simply omits the key. The vendor's close page + documents an omitted ``status_GUID`` as defaulting to the Closed meta-status + (tier 1, recorded in ``docs/vendor-api-reference.md``, "Ticket workflow"), + but no live instance has been asked here. Try it on a throwaway ticket and + re-read before you build on it. Passing your instance's closed + ``status_guid`` explicitly is the form this package's live suite actually + exercises. .. note:: @@ -401,7 +406,9 @@ any write model; keys are serialized to their ``e_*`` API names automatically. There are **two** escape hatches, and they are not interchangeable. ``custom_fields`` only ever emits ``e_``-prefixed keys, so it cannot reach an *official* column this package declines to declare. ``extra_payload`` — also on every write model — is the un-prefixed one: whatever you put -in it reaches the wire exactly as written. +in it reaches the wire exactly as written -- unless it may change a ticket's workflow, in which +case the transport refuses the request before sending it (see +:ref:`changing-a-tickets-status`). .. code-block:: python @@ -425,6 +432,121 @@ Three properties are worth knowing before you reach for it: deliberately omits. Re-read the record afterwards: on this API a write can return HTTP 200, apply one field and drop another in silence. +.. _changing-a-tickets-status: + +Changing a ticket's status +-------------------------- + +The vendor documents no status setter, and this package has none. A ticket's +status follows its workflow -- "Advancing through the steps of a workflow +changes the status of a ticket." (vendor reference-tables page, Statuses +section, tier 1: https://docs.easyvista.com/docs/references-tables.md) -- and +the API offers three things that touch it: + +* :meth:`~easyvista_python_client.EasyvistaClient.end_action` on the workflow + step's open action moves the workflow on. The vendor's REST page for the call + never mentions the workflow; the support is the UI's Finish wizard ("The + workflow will proceed to the next step.", tier 1, + https://docs.easyvista.com/docs/action.md) and one measurement (2026-09-01, + one instance, 2 of 2, so it may not generalise: the ticket reached its + resolved status). It needs ``allow_workflow_effect=WorkflowEffect.ADVANCES``. +* :meth:`~easyvista_python_client.EasyvistaClient.close_ticket` is the vendor's + close request. The vendor documents it as interrupting the workflow ("The + workflow of the ticket is interrupted.", tier 1, + https://docs.easyvista.com/docs/rest-api-close-an-incident-request.md) and + inserting an anticipated closing action. The ticket's unfinished actions are + deleted, or ended: that they are *ended* is this package's reading of the + page's ``end_date`` row rather than an explicit sentence, and the page + documents final statuses only, so do not read ``status_guid`` as a way to pick + an in-progress status. It needs ``allow_workflow_effect=WorkflowEffect.INTERRUPTS``. +* Suspend and reopen are documented by the vendor but not wrapped here; their + pages say only that a suspend, or a reopening, action is created, and their + effect on the open actions is unmeasured. + :meth:`~easyvista_python_client.EasyvistaClient.send` reaches them with + ``allow_workflow_effect=WorkflowEffect.UNKNOWN``. + +The vendor documents no REST write that sets a ticket's status outside those +(tier 1, ``docs/vendor-api-reference.md``, "Ticket workflow"), so this package +has no setter to offer. Not documented is not the same as impossible: a business +rule on your instance can run on any write. + +To escalate the open workflow step to another group without ending it, use +:meth:`~easyvista_python_client.EasyvistaClient.reassign_action` (measured +2026-10-02, one instance, so it may not generalise: the step stays open and the +status does not move; the ticket's owning group, read on one ticket, did not +follow). + +Creating a ticket starts its workflow -- the vendor's create page lists "The +workflow associated with the ticket is started." among what a create does (tier +1). So read the status a new ticket landed on with +:meth:`~easyvista_python_client.EasyvistaClient.get_ticket`; do not follow the +create with ``close_ticket`` to land an initial status, because that interrupts +the workflow you just started. ``PostRequest.workflow_start=False`` is not a way +to avoid it: measured a no-op (2026-09-01, one instance: two tickets identical +but for this flag came back byte-identical), and the vendor create page +documents no such parameter. A workflow-less create is the separate +virtual-agent route, ``POST requests/without-workflow``, which the guard below +refuses unless allowed. + +A write that may change the workflow and does not say so is refused before it is +sent, with :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` -- a +``ValueError``, deliberately not an ``EasyvistaError``, because retrying it can +never succeed. ``allow_workflow_effect`` is **required** on ``close_ticket`` +and optional on the other writers -- ``update_ticket``, ``create_action``, +``create_task``, ``update_action``, ``end_action`` and ``send`` -- where it +defaults to allowing nothing. It takes one +:class:`~easyvista_python_client.WorkflowEffect` or an iterable of them, and +nothing else (a string is a ``TypeError``). A write that names a workflow effect +and was allowed is sent once, never retried, whatever ``max_retries`` says. + +.. code-block:: python + + from easyvista_python_client import ( + EasyvistaWorkflowEffectRefused, + RequestUpdate, + WorkflowEffect, + ) + + try: + # A status column on a ticket may change its workflow, so this is + # refused before anything is sent. + client.update_ticket(rfc, RequestUpdate(extra_payload={"STATUS_ID": 4})) + except EasyvistaWorkflowEffectRefused as exc: + print(exc.effects, exc.triggers) + + # Ending the workflow step's own open action moves the workflow on; say so. + client.end_action( + rfc, + action_id=step_action_id, + allow_workflow_effect=WorkflowEffect.ADVANCES, + ) + +``end_action`` is guarded differently, because what it does depends on which +action you name and the request body cannot say. Unless ``allow_workflow_effect`` +includes ``WorkflowEffect.ADVANCES`` it first reads the action (one read, +projecting ``ACTION_ID`` and ``WORKFLOW_ID``) and refuses, with no end request +sent, a workflow step (``WORKFLOW_ID`` set), a record that comes back without the +``WORKFLOW_ID`` column at all -- which cannot be told from a step -- and a read +that returns a different ``ACTION_ID`` from the one you asked for. +``end_all=True``, which ends every open action on the ticket, is always refused +without ``ADVANCES``, and an explicit ``action_id`` must be a positive integer. +Ending an action you created yourself needs no opt-in, with one unmeasured +exception: whether an action that ``create_action`` creates under the workflow +step carries a ``WORKFLOW_ID`` has not been measured. If it does, +``end_action`` refuses to end it unless the call passes +``allow_workflow_effect=WorkflowEffect.ADVANCES`` -- the safe direction. +``WORKFLOW_ID`` is what separates the engine's rows from a caller's (tier 4: +1500 of 1500 rows, 2026-09-02, one instance; it may not generalise). + +The guard is a deny-list of body keys, columns and routes (see +:mod:`easyvista_python_client.workflow`), and a deny-list of columns cannot be +complete: the vendor's update pages accept "all the fields from the SD_REQUEST +table except those mentioned below" (tier 1), so a write the guard does not name +is unclassified, not proven harmless. One refusal ignores +``allow_workflow_effect`` altogether: ``send`` refuses a path with a dot segment, +a percent-encoded slash or a backslash, because the request could reach a route +other than the one that was checked. + Actions (comments / followups) ------------------------------- @@ -627,8 +749,13 @@ comes back early by your instance's UTC offset. type-1 *Validation Self Service* action (2 tickets, 2/2); a control showed ending a type-94 action the caller had created changed neither the status nor the action count. Ending your own action is inert, ending a workflow - step is not. Omitting ``action_id`` ends **every** open action, which on a - ticket whose only open one is its workflow step means resolving it. + step is not -- which is why ``end_action`` refuses one unless the call + passes ``allow_workflow_effect=WorkflowEffect.ADVANCES`` (see + :ref:`changing-a-tickets-status`). The vendor documents the id-less form as + ending **every** open action, which on a ticket whose only open one is its + workflow step means resolving it; here that form is reachable only through + ``end_all=True`` (also needing ``ADVANCES``), and a bare ``action_id=None`` + raises ``ValueError``. .. warning:: @@ -1385,6 +1512,13 @@ The hierarchy is: :class:`~easyvista_python_client.EasyvistaAuthError` (401/403) :class:`~easyvista_python_client.EasyvistaServerError` (5xx), and :class:`~easyvista_python_client.EasyvistaConnectionError` (transport / timeout). +One error sits **outside** that hierarchy on purpose: +:class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused`, a plain +``ValueError``. It is raised before anything is sent, so it has no status code +and nothing transient about it: the same call can never succeed on a retry, and a +status-code-less ``EasyvistaError`` would invite exactly that retry. Catch it +separately; see :ref:`changing-a-tickets-status`. + End-to-end workflow ------------------- @@ -1392,7 +1526,12 @@ Create a ticket, add a comment, close it, and read it back: .. code-block:: python - from easyvista_python_client import EasyvistaClient, PostRequest, PostTask + from easyvista_python_client import ( + EasyvistaClient, + PostRequest, + PostTask, + WorkflowEffect, + ) with EasyvistaClient.from_env() as client: ticket = client.create_ticket( @@ -1418,8 +1557,10 @@ Create a ticket, add a comment, close it, and read it back: ticket.rfc_number, PostTask(action_type_id=94, group_id=3, description="Investigating"), ) + # Close only when closing is the intent: it interrupts the ticket's workflow. client.close_ticket( ticket.rfc_number, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid="{00000000-0000-0000-0000-000000000000}", comment="Replaced the VPN concentrator", ) diff --git a/docs/vendor-api-reference.md b/docs/vendor-api-reference.md index 1c56b54..5f95160 100644 --- a/docs/vendor-api-reference.md +++ b/docs/vendor-api-reference.md @@ -60,12 +60,15 @@ as the preferred subject identifier. Every other field is optional. | `e_*` | various | Custom fields, 2018.1.183.0+ | **Not in the table above, and not vendor-documented at all: `workflow_start`** -(tier 3, illustrative only). It appears only in the instance's own OpenAPI -schema for this route (`components.schemas`, read 2026-08-27): boolean, -"Optional. If true, starts the workflow for the created incident." Per the -tier table above, that schema is example-derived and not a normative -contract, so treat this field as unverified until tested against the -deployment you use it on. +(tier 3, illustrative only). The vendor create page documents no such parameter +and states that the workflow is started (tier 1). It appears only in the +instance's own OpenAPI schema for this route (`components.schemas`, read +2026-08-27): boolean, "Optional. If true, starts the workflow for the created +incident." Per the tier table above, that schema is example-derived and not a +normative contract. Measured a no-op (tier 4, 2026-09-01, one instance, so it +may not generalise): two tickets identical but for this flag came back +byte-identical, so `workflow_start=False` did not create a ticket without its +workflow there. Re-measure on the deployment you use it on. ## Create an action — `POST /requests/{rfc_number}/actions` (tier 1) @@ -209,7 +212,7 @@ per-step text for the workflow ones. `_PO`, `_SP`, `_L1`..`_L6`) on the item GET. There is no `action-types` route to ask, so on this deployment those ids **cannot be named through the API at all**. What is known about 28 is behavioural, not nominal: it is the row that -carries the text passed to `set_status(comment=...)`, so it must not be +carries the text passed to `close_ticket(comment=...)`, so it must not be filtered out of a timeline read. Also worth recording without acting on it: the instance's `POST /assets` schema @@ -241,6 +244,114 @@ tokens exist (`search=field:last_week`). Neither is exposed by this package. Envelope: `HREF`, `record_count`, `total_record_count`, `records`, `@next`. +## Ticket workflow — what each documented write does to it (tier 1, read 2026-10-02) + +"A workflow is a process that handles a type of tickets, arranged in a sequence of +actions performed in steps." — https://docs.easyvista.com/docs/workflow.md. +"Advancing through the steps of a workflow changes the status of a ticket." — +https://docs.easyvista.com/docs/references-tables.md, Statuses section (where also: +"The change of a meta-status performs actions in the workflow.") + +| Write | Effect on the workflow | Evidence | +| --- | --- | --- | +| `POST /requests` (create) | **starts** it | "3. The workflow associated with the ticket is started." — rest-api-create-an-incident-request.md | +| `POST /requests/without-workflow` (virtual agent) | does not start it | "3. The workflow associated with the ticket will not be started." — ev-service-manager-rest-api-create-ticket-via-virtual-agent.md | +| `PUT /requests/{rfc}/workflowstart?_flowcheck=…` (virtual agent) | **starts** it, for a ticket created without one | "This method allows the workflow from a specified ticket created via a virtual agent to be started." Body: "You must not supply any information (without curly brackets) in the body of the HTTP request." — ev-service-manager-rest-api-start-ticket-workflow-via-virtual-agent.md. The guard names this route (see the table below). | +| `PUT /requests/{rfc}` `{"closed": …}` | **interrupts** it, whatever status is sent | "1. The workflow of the ticket is interrupted." (sub-bullet: "workflowstop function, with **rfc_number** passed as a parameter"); then status = "the final status of the ticket"; "The unfinished actions associated with the ticket are deleted." (`end_date_ut = NULL` iff `delete_actions = True`; otherwise `end_date` is the "Closing date of open actions"); "An anticipated closing action associated with the ticket is inserted."; "Any further modification is impossible." — rest-api-close-an-incident-request.md. An omitted `status_GUID` defaults to the Closed meta-status (same page). | +| `PUT /actions/{rfc}` `{"end_action": …}` | **advances** it when the action is a workflow step | REST page: "If the action_id is not specified, all the ongoing actions associated with the rfc_number are ended." — rest-api-finish-an-action-attached-to-an-incident-request.md; it never mentions the workflow. UI Finish wizard: "The workflow will proceed to the next step." — action.md. Tier 4, one instance, may not generalise: 2026-09-01, 2/2 (step ended → ticket Résolu); a caller's own action, 3/3 across 2026-09-01 and 2026-09-02, no change. | +| `PUT /requests/{rfc}` `{"suspended"…}` / `{"restarted"…}` | not documented | "A suspend action for the ticket is created." / "Create a reopening action for the ticket." — nothing on open actions or status. rest-api-suspend-an-incident-request.md, rest-api-reopen-an-incident-request.md | +| Update a ticket, update an action (any body not named above) | not documented | Each page accepts "all the fields from the SD_REQUEST table except those mentioned below" / "…from the AM_ACTION table…", has no processing section and does not contain the word "workflow". rest-api-update-an-incident-request.md, rest-api-update-an-action.md | +| Create an action | not documented | Processing section: "An action is created for the ticket." then conditional logic on `parent_action_id` (which action the new one attaches to); the page does not contain the word "workflow". rest-api-create-an-action-for-an-incident-request.md | +| Create a task | not documented | Processing section: "A task associated with the ticket is created."; the page does not contain the word "workflow". rest-api-create-a-task-for-an-incident-request.md | +| Attach or delete a document | not documented | Attach: "They are attached to the specified ticket."; the delete page has no processing section; neither contains the word "workflow". rest-api-upload-and-attach-documents-to-an-incident-request.md, rest-api-delete-a-ticket-attachment.md | + +The vendor documents no REST write that sets a ticket's status outside the rows +above: the ticket update page excludes `status_id` from its body, with +`sd_catalog_id`, `initial_sd_catalog_id` and `parent_request_id` +(rest-api-update-an-incident-request.md). The vendor documents no status setter, and +this package has none: it removed `set_status`, which was the close request. Not +documented is not the same as impossible — see the business-rule caveat below. + +**The vendor documents no REST route that reassigns or transfers an action.** The +UI's "Assign action" button runs a wizard ("The action will automatically be +transferred." — action.md). `PUT /actions/{id}` with the group and/or the person is +allowed by the update page's "all the fields from the AM_ACTION table except those +mentioned below" rule, and its exclusion list does not name `GROUP_ID` or `DONE_BY_ID` +(rest-api-update-an-action.md, tier 1). This package does not refuse it, and wraps it +as `reassign_action(action_id, *, group_id=None, done_by_id=None)`. Allowed by the +documentation is not the same as shown harmless, so its effect was measured — +**tier 4, 2026-10-02, one instance (Service Manager 2025.3), two tickets, so it may +not generalise**: with the body `{"group_id": }`, on **both** tickets the group was +**stored** (`GROUP_ID` 57 → 50 on the open workflow step, read back immediately and +again five seconds later); the step **stayed open** (`END_DATE_UT` empty); the ticket's +status and `END_DATE_UT` **did not move**; the open actions were unchanged; and **no new +action rows** appeared. On the **first ticket only** the step was recorded as a type-20 +action with `WORKFLOW_ID` set, its `WORKFLOW_ID` was unchanged, `DONE_BY_ID` stayed +empty and no ticket field changed; and that is also the only ticket on which the +ticket's own `OWNING_GROUP_ID` was read — it **stayed at the old group**, so the +ticket's owning group did not follow the action's. Reassigning to a person +(`done_by_id`) was **not measured**. Whether the UI wizard's notifications fire is not +observable from the API. + +**Business rules can fire on any write** — "On Insert/On Update" of any record +(business-rule.md) — so a write this package does not gate is unclassified, not proven +neutral. + +**The guard.** `easyvista_python_client.workflow` names what a write may do, and the +transport refuses it, before any request is sent, unless the call passes +`allow_workflow_effect=`. What it names, for a request that is not a read, exactly as +`easyvista_python_client/workflow.py` holds it (keys are matched case-folded, as +top-level body keys): + +| Where | Named | Effect named | +| --- | --- | --- | +| any path | body keys `closed` | `INTERRUPTS` | +| any path | body keys `end_action` | `ADVANCES` | +| any path | body keys `suspended`, `restarted` | `UNKNOWN` | +| `requests/{rfc}` | body keys `status_id`, `status_guid`, `sd_catalog_id`, `initial_sd_catalog_id`, `catalog_guid`, `catalog_code`, `parent_request_id`; and a `DELETE` of the ticket | `UNKNOWN` | +| `requests/without-workflow` | the route itself | `UNKNOWN` | +| `requests/{rfc}/`, `` not `actions`, `tasks` or `documents` | the route itself: `close` is `INTERRUPTS`; any other (`suspend`, `restart`, `workflowstart`, or one a deployment adds) is `UNKNOWN` | as stated | +| `requests/{rfc}/actions` (create an action) | body keys `end_date_ut`, `end_date`, `status_id_on_terminate`, `workflow_id`, `stage_id`, `process_step_id` | `UNKNOWN` | +| `requests/{rfc}/tasks` (create a task) | body keys `status_id_on_terminate`, `workflow_id`, `stage_id`, `process_step_id`, `parent_action_id` | `UNKNOWN` | +| `actions/{id}` | body keys `end_date_ut`, `end_date`, `status_id_on_terminate`, `workflow_id`, `stage_id`, `process_step_id`, `parent_action_id`, `action_type_id`, `action_type_guid`, `action_type_name`, `request_id`, `rfc_number` | `UNKNOWN` | +| `actions/`, `` not an integer id (ASCII digits only) | the route itself, beside any key above: `PUT actions/{rfc_number}` is the vendor's end-action route (rest-api-finish-an-action-attached-to-an-incident-request.md), so a write to it is named whatever its body says | `ADVANCES` | + +So creating an action with a `parent_action_id`, a type or a ticket link needs no +opt-in (the create-action set is narrower than the update set), and a task's end date +is ordinary because a task is born ended. The column rules above apply to the +`requests/` and `actions/` routes only; a write to any other route family is not +classified by column. Not named anywhere: text, owner, group, done-by, impact, +urgency. A column deny-list cannot be complete, and this one says so. + +* **A request counts as a read only when its method and every method-override header + value are reads.** The method and the value of each of `X-HTTP-Method-Override`, + `X-HTTP-Method` and `X-Method-Override` (any casing) must all be `GET`, `HEAD` or + `OPTIONS`; otherwise it is classified as a write, so an override that says `GET` + cannot hide one. The headers read are the ones that go on the wire: + `config.extra_headers` with the request's own laid over them. Whether this API + honours any of those headers is not recorded here. +* **Refused outright, whatever the method and whatever `allow_workflow_effect` says** + (a `ValueError`, no request sent): a path with a dot segment (`.` or `..`, also + percent-encoded), a percent-encoded slash or backslash (`%2F`, `%5C`), or a raw + backslash. The HTTP client collapses a dot segment, so the request would reach a + route other than the one checked; a server may read an encoded slash or a backslash + as a path separator. Whether this server does is not measured — the check fails + closed, and no API route needs any of them. +* **`end_action` reads the action first.** Unless `allow_workflow_effect` includes + `WorkflowEffect.ADVANCES`, it makes one item read projecting `ACTION_ID` and + `WORKFLOW_ID`, and refuses — with no end request sent — a workflow step + (`WORKFLOW_ID` set), a record that comes back without the `WORKFLOW_ID` column at + all (which cannot be told from a step), or a record naming a different `ACTION_ID` + than the one asked for. `end_all=True` is refused outright without `ADVANCES`. + Ending a caller's own action needs no opt-in. What separates a workflow step from + a caller's action is `WORKFLOW_ID` (tier 4: 1500 of 1500 rows, 2026-09-02, one + instance). The live check that an item read of an action carries the column: + tier 4, 2026-10-02, one instance, may not generalise — the item read, with + `fields=ACTION_ID,WORKFLOW_ID` and without it, named `WORKFLOW_ID` on a workflow + step (set) and on a non-workflow action (present, empty). Whether an action + created under a step by `create_action` carries one is unmeasured; if it does, + ending it is refused too, the safe direction. + ## Route topology (tier 2) — `GET {api_root}/swagger`, read 2026-08-27 **A 403 does not discriminate.** This API answers 403 for a path that does not @@ -367,18 +478,9 @@ reading; see open item O-MEMOFORMAT for what is not yet known. that caused `RequestUpdate.urgency_id` to be removed. The exclusion may be a type mismatch we authored rather than an API limitation. Unresolved: settling it needs a live write. Same question for `severity_id`. -* **O-CLOSE** — should the close route move to `PUT /requests/{rfc}/close`? * **O-URGPATH** — the vendor documents `GET /urgencies`; the instance spec declares `GET /urgency`. Both return 200 live. Which is canonical is unknown. -* **O-CLOSE-DEFAULT** — `close_ticket` omits `status_GUID` from the body when - the caller omits it, and two docstrings previously stated that this closes - the ticket to the instance's default *Closed* meta-status, attributing it to - the vendor close page. **That sentence is not recorded anywhere in this file - and the behaviour is not exercised by the live suite** — every `close_ticket` - call in `integration_tests/` passes an explicit `status_guid`. Both - docstrings now hedge. Until someone either re-reads the vendor page and adds - the row here, or measures the omitted form live and dates it, the - documentation must not assert it. +* **O-CLOSE-DEFAULT** — **CLOSED at tier 1 (2026-10-02).** The vendor close page documents an omitted status_GUID as defaulting to the Closed meta-status. Not measured here. * **O-COSTGROUP** — `TIME_COST` / `CONTRACTUAL_COST` are parsed by `models/common._parse_ev_decimal`, which accepts either decimal separator and **refuses a grouping separator** rather than guessing (`'1.234,56'` and @@ -395,7 +497,7 @@ reading; see open item O-MEMOFORMAT for what is not yet known. * **O-ACTIONTYPE28** — types 14, 27 and 28 have an empty `ACTION_LABEL_*` in every language column at both list and item level, and there is no `action-types` route, so nothing in the API can name them. Type 28 is known - behaviourally (it carries `set_status(comment=...)` text) and 14 and 27 not + behaviourally (it carries `close_ticket(comment=...)` text) and 14 and 27 not at all. Settling this needs the EasyVista **admin console**, not the API: the administration screen listing action types, and specifically which type ids that deployment classes as *task* types. Nobody working on this package has diff --git a/easyvista_python_client/__init__.py b/easyvista_python_client/__init__.py index 0e4235f..9453b5a 100644 --- a/easyvista_python_client/__init__.py +++ b/easyvista_python_client/__init__.py @@ -22,6 +22,7 @@ EasyvistaRateLimitError, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) from .field_model import FieldClassification from .filters import ( @@ -45,6 +46,7 @@ from .references import DEFAULT_LANGUAGE_ORDER, Reference, localized_label from .reporting import TicketStatistics, aggregate_tickets from .timestamps import format_ev_datetime, parse_ev_datetime +from .workflow import WorkflowEffect __all__ = [ "DEFAULT_DISCOVERY_NAMES", @@ -70,6 +72,7 @@ "EasyvistaRateLimitError", "EasyvistaServerError", "EasyvistaValidationError", + "EasyvistaWorkflowEffectRefused", "Employee", "EmployeeUpdate", "FieldClassification", @@ -88,6 +91,7 @@ "SearchResult", "TicketContext", "TicketStatistics", + "WorkflowEffect", "__version__", "aggregate_tickets", "escape_ev_value", diff --git a/easyvista_python_client/_async/_transport.py b/easyvista_python_client/_async/_transport.py index 6dd7bbc..4885ab6 100644 --- a/easyvista_python_client/_async/_transport.py +++ b/easyvista_python_client/_async/_transport.py @@ -38,7 +38,9 @@ EasyvistaRateLimitError, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) +from easyvista_python_client.workflow import WorkflowEffect, workflow_triggers #: Default chunk size, in bytes, for :meth:`Transport.stream_bytes`. #: @@ -197,6 +199,49 @@ def merge_params( return None return {**self.config.default_params, **(call or {}), **(spec or {})} + def gate(self, spec: RequestSpec) -> frozenset[WorkflowEffect]: + """Refuse ``spec`` unless every workflow effect it names is allowed. + + Returns the effects it names -- all of them allowed by then -- so the + caller can tell a workflow write from an ordinary one. Empty for a read + and for an ordinary write. See :mod:`easyvista_python_client.workflow` + for what is named and why. Raises + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` (no + request is made), or ``ValueError`` for a path with a dot segment, a + percent-encoded slash or backslash, or a raw backslash. + + The headers read for a method override are the ones that go on the + wire: ``config.extra_headers`` with the spec's own laid over them, as + :meth:`headers` and the request merge them, so an override header set in + the configuration cannot hide a write behind a ``GET`` either. + """ + triggers = workflow_triggers( + spec.method, + spec.path, + spec.json, + {**self.config.extra_headers, **(spec.headers or {})}, + ) + refused = tuple( + (what, effect) + for what, effect in triggers + if effect not in spec.allow_workflow_effect + ) + if refused: + effects = frozenset(effect for _, effect in refused) + names = sorted(f"WorkflowEffect.{effect.name}" for effect in effects) + allow = names[0] if len(names) == 1 else "{" + ", ".join(names) + "}" + named = ", ".join(f"{what!r} ({effect.name})" for what, effect in refused) + raise EasyvistaWorkflowEffectRefused( + f"refused {spec.method} {spec.path} before sending it: {named} may " + f"change the ticket's workflow. If that is the intent, pass " + f"allow_workflow_effect={allow} to this call, or to send() when " + f"the method does not take it. See docs/vendor-api-reference.md, " + f"'Ticket workflow'.", + effects=effects, + triggers=refused, + ) + return frozenset(effect for _, effect in triggers) + def auth(self) -> httpx.Auth | None: if self.config.uses_basic_auth: return httpx.BasicAuth(self.config.login or "", self.config.password or "") @@ -362,9 +407,16 @@ async def send( ``config.default_params`` sits under both -- see :meth:`merge_params` for the full ordering. + + A request naming a workflow effect is refused before anything is sent + unless the spec allows it (:meth:`BaseTransport.gate`). One that is + allowed is attempted **once**: each close inserts another anticipated + closing action and each end ends whatever is open, so a resend after a + lost response is not a repeat of the same request. """ + effects = self.gate(spec) retryer = AsyncRetrying( - stop=stop_after_attempt(self.config.max_retries + 1), + stop=stop_after_attempt(1 if effects else self.config.max_retries + 1), wait=wait_exponential(multiplier=0.5, max=10), retry=retry_if_exception_type((_RetryableResponse, httpx.TransportError)), reraise=True, diff --git a/easyvista_python_client/_async/client.py b/easyvista_python_client/_async/client.py index 6d75598..ecde3df 100644 --- a/easyvista_python_client/_async/client.py +++ b/easyvista_python_client/_async/client.py @@ -49,6 +49,7 @@ EasyvistaAuthError, EasyvistaError, EasyvistaNotFound, + EasyvistaWorkflowEffectRefused, ) from easyvista_python_client.field_model import parse_memo from easyvista_python_client.filters import ev_equals_filter, is_safe_ev_value @@ -88,6 +89,7 @@ from easyvista_python_client.resources import employees as employees_res from easyvista_python_client.resources import requests as requests_res from easyvista_python_client.resources.discovery import SWAGGER_PATH +from easyvista_python_client.workflow import WorkflowEffect, as_effects # Width of the action-body fan-out: a ceiling on requests in flight at once on # the async surface, inert on the sync one. This is the one fan-out here whose @@ -98,6 +100,12 @@ # -- the server, not the client, is the bottleneck). _ACTION_FANOUT = 8 +# The projection end_action's guard reads: just enough to tell a workflow step +# (WORKFLOW_ID set) from a caller's own action (WORKFLOW_ID empty). Projected +# rather than left to the default item read so that the column is asked for +# by name -- see integration_tests/test_live_workflow_guard.py. +_WORKFLOW_PROBE_FIELDS = ("ACTION_ID", "WORKFLOW_ID") + def _unavailable_reason(exc: EasyvistaError) -> str: """One ``InstanceProfile.unavailable`` value, first token machine-readable. @@ -162,6 +170,7 @@ async def send( params: Mapping[str, Any] | None = None, json: Any = None, headers: Mapping[str, str] | None = None, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), ) -> Any: """Issue an arbitrary request against this instance's API root. @@ -182,7 +191,8 @@ async def send( :meth:`download_document` or :meth:`stream_document`. Everything else is shared with the typed methods: ``config.max_retries`` - attempts with the same backoff, and the same exception mapping -- 401 and + attempts with the same backoff (one attempt for an allowed workflow + write), and the same exception mapping -- 401 and 403 to :class:`~easyvista_python_client.EasyvistaAuthError`, 404 to :class:`~easyvista_python_client.EasyvistaNotFound`, 400 and 590 to :class:`~easyvista_python_client.EasyvistaValidationError`, with 590 never @@ -190,6 +200,18 @@ async def send( ``config.default_params`` is merged under ``params``; ``headers`` is merged over the client-level ones and may not carry ``Authorization``. + ``allow_workflow_effect`` is the way past the workflow guard. A write + whose content may change a ticket's workflow -- a ``closed``, + ``end_action``, ``suspended`` or ``restarted`` body on any path, a + status or catalog column on a ticket, an end date or type on an action, + any write to ``actions/{rfc_number}`` (the vendor's end-action route; an + integer id addresses an action instead), a ``requests/{rfc}/close``-style + route -- is refused with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` before + anything is sent, unless this argument names every + :class:`~easyvista_python_client.WorkflowEffect` it carries. See + ``docs/vendor-api-reference.md``, "Ticket workflow". + Returns the decoded JSON body, or ``{}`` when the response has none. Nothing is validated into a model and no envelope is unwrapped: the caller owns the shape, which is the point -- there is no model for a @@ -206,12 +228,29 @@ async def send( path, json=json, headers=dict(headers) if headers else None, + allow_workflow_effect=as_effects(allow_workflow_effect), ), params=params, ) # --- tickets ------------------------------------------------------------- async def create_ticket(self, ticket: PostRequest) -> Request: + """Create one ticket -- which starts its workflow. + + Per the vendor create page (tier 1, + https://docs.easyvista.com/docs/rest-api-create-an-incident-request.md): + a CALL action is inserted with its end date set to the ticket's + submission date, so it is born ended, then "3. The workflow + associated with the ticket is started." A fresh ticket carries one open + workflow-step action (tier 4, 2026-09-01, one instance). + + **Do not follow the create with** :meth:`close_ticket` **to land an + initial status.** That interrupts the workflow you just started; read + the status the ticket landed on with :meth:`get_ticket` instead. + + A 590 on create may still have created the row: reconcile by + ``EXTERNAL_REFERENCE`` rather than retrying. + """ spec, parse = requests_res.build_create_ticket( ticket, context=self._validation_context ) @@ -445,67 +484,88 @@ async def ticket_statistics( stats.population_total = population_total return stats - async def update_ticket(self, rfc_number: str, update: RequestUpdate) -> Request: + async def update_ticket( + self, + rfc_number: str, + update: RequestUpdate, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Request: """Update a ticket's writable fields. - Cannot set a status: there is no flat status update on this API. See - :meth:`set_status`, and :class:`RequestUpdate` for the measurements. + Cannot set a status: there is no flat status update on this API (see + :class:`RequestUpdate` for the measurements), the vendor documents no + status write that leaves the workflow alone, and this package has none + -- a ticket's status follows its workflow. See :meth:`close_ticket` for + the one request the vendor documents that does set a status. + + A body that may change the workflow -- a status or catalog column, or a + workflow-control body, typically put in ``extra_payload`` -- is refused + before it is sent unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. The fields :class:`RequestUpdate` declares need no opt-in. """ spec, parse = requests_res.build_update_ticket( rfc_number, update, context=self._validation_context ) - return parse(await self._transport.send(spec)) - - async def set_status( - self, rfc_number: str, *, status_guid: str, comment: str | None = None - ) -> Request: - """Set a ticket's status, addressed by ``STATUS_GUID``. - - This is the API's only working status write, and it reaches **every** - status rather than only terminal ones: given six different status GUIDs - in turn, a fresh ticket landed on exactly the status requested every - time, non-terminal ones included. - - It sends the documented ``{"closed": {"status_GUID": ...}}`` body -- the - same request :meth:`close_ticket` sends, under a name that matches what - it does, because "close" is what the wire calls it and not what it is - limited to. - - Note the addressing. A ``STATUS_GUID`` is not a ``STATUS_ID``; the two - are different columns, and only the GUID works here. Read a status's GUID - off any ticket in that status (the nested ``STATUS`` object carries - ``STATUS_GUID``) -- they are stable per instance but are **not** - portable between instances. - """ - spec, parse = requests_res.build_set_status( - rfc_number, - status_guid=status_guid, - comment=comment, - context=self._validation_context, - ) - return parse(await self._transport.send(spec)) + return parse(await self._transport.send(spec.allowing(allow_workflow_effect))) async def close_ticket( self, rfc_number: str, *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect], status_guid: str | None = None, delete_actions: int | bool | None = None, comment: str | None = None, end_date: str | None = None, catalog_guid: str | None = None, ) -> Request: - """Close a ticket, via the vendor's documented close route. + """Close a ticket early -- the vendor's close request, which stops its workflow. - Sends ``PUT requests/{rfc}`` with a ``closed`` wrapper -- + Sends ``PUT requests/{rfc}`` with a ``closed`` body, as the vendor + documents it: https://docs.easyvista.com/docs/rest-api-close-an-incident-request.md. - Every argument is optional. With no ``end_date`` the server stamps now. - With no ``status_guid`` this client sends no status of its own -- but - **where the ticket then lands is not established here**: the behaviour - is not recorded in ``docs/vendor-api-reference.md`` and no live test - exercises the omitted form, every one of them passing an explicit - ``status_guid``. Try it on a throwaway ticket and re-read before - relying on it (open item O-CLOSE-DEFAULT). + That page lists what the request does, and none of it depends on the + status passed (tier 1, re-read 2026-10-02): + + * "The workflow of the ticket is interrupted." + * The status is set to ``status_guid``, which the page calls "the final + status of the ticket". Omitted, the vendor documents the default as + the Closed meta-status. + * "The unfinished actions associated with the ticket are deleted" when + ``delete_actions`` is true. Otherwise ``end_date`` is the "Closing date + of open actions associated with the ticket" -- read here as: they are + ended, not left open. That reading rests on the parameter row and one + observation (2026-09-01, one instance), not on an explicit sentence. + * "An anticipated closing action associated with the ticket is + inserted." -- one per call, so every close adds a row. + + **So this is not a status setter. The vendor documents no status + setter, and this package has none.** A ticket's status follows its + workflow: "Advancing through the steps of a workflow + changes the status of a ticket." (tier 1, + https://docs.easyvista.com/docs/references-tables.md, Statuses section). + A non-final status sent + here still interrupts the workflow and closes the ticket's open + actions -- the page documents final statuses only, and nothing exempts + the others. To move a ticket through its workflow, end the workflow + step's open action instead; see :meth:`end_action`. + + That is why ``allow_workflow_effect`` is **required**: pass + ``WorkflowEffect.INTERRUPTS`` to say at the call site that interrupting + the workflow is the intent. Anything that does not include it is + refused with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` before + a request is made, and the allowed request is sent once, never + retried:: + + client.close_ticket( + rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, + status_guid=CLOSED_GUID, + ) + after = client.get_ticket(rfc) + assert after.end_date_ut is not None # the close actually landed **Verify the close by re-reading the status, not by the return value.** A status id is per-instance configuration and nothing about it is @@ -515,11 +575,7 @@ async def close_ticket( skip a ticket it believed was already closed. Read ``get_ticket(rfc).status_id`` (or ``.reference("STATUS")`` for the label) afterwards, and compare against a status you resolved from the - instance rather than a constant:: - - client.close_ticket(rfc, status_guid=CLOSED_GUID) - after = client.get_ticket(rfc) - assert after.end_date_ut is not None # the close actually landed + instance rather than a constant. ``end_date_ut`` is the more portable signal than any status id: it is empty while a ticket is being worked and stamped once it is finished. @@ -533,10 +589,9 @@ async def close_ticket( distinguishes the two without resolving the status against the instance. - ``status_guid`` reaches **any** status, not only terminal ones -- see - :meth:`set_status`, which is this same request under a name that says - so. ``catalog_guid`` requalifies the ticket as it closes. - ``delete_actions`` drops its actions. + ``catalog_guid`` requalifies the ticket as it closes -- the vendor notes + it is needed only for that. ``delete_actions`` deletes the unfinished + actions instead of ending them. ``end_date`` takes the instance's own date format, which is not ISO 8601 everywhere (``dd/mm/yyyy`` on the verified instance -- read @@ -553,10 +608,16 @@ async def close_ticket( catalog_guid=catalog_guid, context=self._validation_context, ) - return parse(await self._transport.send(spec)) + return parse(await self._transport.send(spec.allowing(allow_workflow_effect))) # --- actions ------------------------------------------------------------- - async def create_action(self, rfc_number: str, action: PostAction) -> Action: + async def create_action( + self, + rfc_number: str, + action: PostAction, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Create one action on a ticket. The returned :class:`Action` carries **no usable ``action_id``**: the @@ -599,13 +660,25 @@ async def create_action(self, rfc_number: str, action: PostAction) -> Action: refused on a ticket that accepted the same body earlier. The messages are literal, not a stage gate. :meth:`create_task` is not parent-resolved and is unaffected. + + A body that would create the record already tied into the workflow -- + ``WORKFLOW_ID``, ``STAGE_ID`` or ``STATUS_ID_ON_TERMINATE`` through + ``extra_payload``, an end date on an action, a parent on a task -- is + refused unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. """ spec, parse = actions_res.build_create_action( rfc_number, action, context=self._validation_context ) - return parse(await self._transport.send(spec)) + return parse(await self._transport.send(spec.allowing(allow_workflow_effect))) - async def create_task(self, rfc_number: str, task: PostTask) -> Action: + async def create_task( + self, + rfc_number: str, + task: PostTask, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Create a task on a ticket — an action that arrives already ENDED. **This is how you post a comment.** A task and an action are the same @@ -654,11 +727,17 @@ async def create_task(self, rfc_number: str, task: PostTask) -> Action: usable ``action_id``** — the create response is an HREF naming the parent request. Diff :meth:`list_actions` across the call to address what you just created. + + A body that would create the record already tied into the workflow -- + ``WORKFLOW_ID``, ``STAGE_ID`` or ``STATUS_ID_ON_TERMINATE`` through + ``extra_payload``, an end date on an action, a parent on a task -- is + refused unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. """ spec, parse = actions_res.build_create_task( rfc_number, task, context=self._validation_context ) - return parse(await self._transport.send(spec)) + return parse(await self._transport.send(spec.allowing(allow_workflow_effect))) async def list_actions( self, @@ -775,7 +854,13 @@ async def get_action( ) return parse(await self._transport.send(spec, params=params)) - async def update_action(self, action_id: str | int, update: ActionUpdate) -> Action: + async def update_action( + self, + action_id: str | int, + update: ActionUpdate, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Edit an existing action's note text. ``ActionUpdate`` carries two fields, ``description`` and ``comment``, @@ -800,6 +885,14 @@ async def update_action(self, action_id: str | int, update: ActionUpdate) -> Act recorded for that verb is what this API answers for an absent route as well as a denied one, so it did not distinguish them. + ``action_id`` must be a positive integer: ``PUT actions/{rfc_number}`` + is the vendor's end-action route on the same path, so an RFC number is + refused rather than sent. A body that may end, re-type, re-parent or + move the action (an end date, ``WORKFLOW_ID``, ``ACTION_TYPE_ID``, ..., + through ``extra_payload``) is refused unless ``allow_workflow_effect`` + names the effect; see :meth:`send`. ``GROUP_ID`` and ``DONE_BY_ID`` are + not refused -- see :meth:`reassign_action` for reassignment. + The returned :class:`Action` is the API's own echo and is **not verified**: the PUT's response body has never been captured, and if it answers empty or href-only the parser yields an ``Action`` whose fields @@ -809,6 +902,71 @@ async def update_action(self, action_id: str | int, update: ActionUpdate) -> Act spec, parse = actions_res.build_update_action( action_id, update, context=self._validation_context ) + return parse(await self._transport.send(spec.allowing(allow_workflow_effect))) + + async def reassign_action( + self, + action_id: str | int, + *, + group_id: int | None = None, + done_by_id: int | None = None, + ) -> Action: + """Reassign an action to another group and/or person. + + The closest API equivalent of the UI's reassignment of an action (its + "Assign action" button, which runs a wizard that may do more than this + one write does), and the way to escalate the open workflow step to + another group without ending it. Sends + ``PUT actions/{id}`` with the group and/or the person in charge; at + least one is required, and both are positive integers -- ids are + per-deployment, so look them up rather than hardcoding them. A group + id comes from :meth:`discover` or :meth:`list_reference_table` where + the groups table is readable to you; on the measured instance + (2026-10-02, one instance) ``GET groups`` answered 403, which this + API also answers for an absent route, so it settles nothing about why. + If yours does too, take the group id from your administrator, or from + a record that already carries one (an existing action's ``GROUP_ID``). + ``done_by_id`` is an employee id: find one with + :meth:`search_employees` or :meth:`get_employee`. + + **The vendor documents no reassignment route.** The UI's transfer is a + wizard, and ``PUT actions/{id}`` accepts "all the fields from the + AM_ACTION table except" a list that does not name these two + (tier 1, https://docs.easyvista.com/docs/rest-api-update-an-action.md). + So what this write does is measured, not specified: + + Measured 2026-10-02 on one instance (Service Manager 2025.3; two + tickets, so it may not generalise), with the body + ``{"group_id": }``. On **both** tickets -- a service request at + status id 6 and a fresh incident at status id 12 (ids are + per-instance) -- the group was stored: ``GROUP_ID`` went 57 -> 50 on + the open workflow step, and read 50 on a re-read immediately and again + five seconds later. On both, the step stayed open (``END_DATE_UT`` + empty), the ticket's ``STATUS_ID`` and ``END_DATE_UT`` did not move, + the open actions were unchanged, and no new action rows appeared. On + the **first ticket only** the step was recorded as a type-20 action + with ``WORKFLOW_ID`` set; its ``WORKFLOW_ID`` was unchanged, + ``DONE_BY_ID`` stayed empty, and no ticket field changed. + **The ticket's own ``OWNING_GROUP_ID`` does not follow the action's + group** -- it stayed 57 on the first ticket, the only one read for it, + so reassigning a step is not reassigning the ticket. The lower-case + key ``group_id`` was the one sent; the upper-case spelling was never + needed. The person (``done_by_id``) was **not measured**: the same + body shape is sent, but nothing here shows what the instance does + with it. Whether the UI wizard's notifications fire is not observable + from the API. + + Not refused by the workflow guard: the group and the person are data + the workflow reads, not workflow state. Re-read with :meth:`get_action` + before trusting the result -- this API answers 200 while dropping a + field it did not store. + """ + spec, parse = actions_res.build_reassign_action( + action_id, + group_id=group_id, + done_by_id=done_by_id, + context=self._validation_context, + ) return parse(await self._transport.send(spec)) async def end_action( @@ -821,6 +979,7 @@ async def end_action( start_date: str | None = None, elapsed_time: int | str | None = None, doneby_mail: str | None = None, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), ) -> Action: """Report an action as done — the step :meth:`create_action` leaves open. @@ -855,7 +1014,9 @@ async def end_action( the id-less form as ending *every open action on the ticket*, which on a ticket whose only open action is its workflow step means resolving it. That form is reachable only through ``end_all=True``; - a bare ``action_id=None`` raises ``ValueError`` before any request. + a bare ``action_id=None`` raises ``ValueError`` before any request, + as does an ``action_id`` that is not a positive integer -- a blank, or + an RFC number -- which is sent as an integer when it is one. The guard exists because ``Action.action_id`` is legitimately ``None`` all over this package — :meth:`create_action`'s response carries no id, and a ``fields=`` projection without ``ACTION_ID`` @@ -864,6 +1025,27 @@ async def end_action( with a single open action, so *how* it behaves against several open at once is vendor-documented, not measured here. + **And why ending a workflow step must be asked for.** Unless + ``allow_workflow_effect`` includes ``WorkflowEffect.ADVANCES``, this + method reads the action first -- one item read projecting + ``ACTION_ID`` and ``WORKFLOW_ID`` -- and refuses, with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` and + no end request sent, when the action is a workflow step + (``WORKFLOW_ID`` set), when the record comes back without the + column at all, which cannot be told apart from a step, or when the + record names a different ``ACTION_ID`` than the one asked for. + ``WORKFLOW_ID`` is what separates the engine's rows from a caller's + (tier 4, 1500 of 1500 rows, 2026-09-02, one instance -- see + :attr:`Action.is_workflow_generated`). Whether an action created under + the step by :meth:`create_action` carries one is unmeasured; if it + does, ending it is refused too -- the safe direction. ``end_all=True`` + is refused outright without ``ADVANCES``. Ending your own action + needs no opt-in. If the read fails, its error propagates and the end + request is not sent -- a 403 there says nothing about whether ending + is permitted. The end request, once sent, is never retried. Both kinds + of action were read with the column present (live, 2026-10-02, one + instance). + Both dates are passed through as **strings**, because the accepted format follows the instance rather than a standard: it is not ISO 8601 on every deployment, and accepting a ``datetime`` would mean this @@ -927,7 +1109,60 @@ async def end_action( doneby_mail=doneby_mail, context=self._validation_context, ) - return parse(await self._transport.send(spec)) + allowed = as_effects(allow_workflow_effect) + if WorkflowEffect.ADVANCES not in allowed: + await self._refuse_ending_a_workflow_step(action_id, end_all=end_all) + allowed = allowed | {WorkflowEffect.ADVANCES} + return parse(await self._transport.send(spec.allowing(allowed))) + + async def _refuse_ending_a_workflow_step( + self, action_id: str | int | None, *, end_all: bool + ) -> None: + """Raise unless one read shows ``action_id`` is not a workflow step.""" + advances = frozenset({WorkflowEffect.ADVANCES}) + if end_all or action_id is None: + raise EasyvistaWorkflowEffectRefused( + "end_all=True ends every open action on the ticket, its workflow " + "step included, which advances the ticket's workflow. Pass " + "allow_workflow_effect=WorkflowEffect.ADVANCES if that is the intent.", + effects=advances, + triggers=(("end_action", WorkflowEffect.ADVANCES),), + ) + # end_action built the end spec first, which refused anything but a + # positive integer, so this conversion cannot fail and the read + # addresses exactly the id the end request will name. + wanted = int(action_id) + spec, parse = actions_res.build_get_action( + wanted, fields=_WORKFLOW_PROBE_FIELDS, context=self._validation_context + ) + target = parse(await self._transport.send(spec)) + if target.action_id is not None and target.action_id != wanted: + raise EasyvistaWorkflowEffectRefused( + f"the read of action {wanted} returned a different action " + f"(ACTION_ID {target.action_id}), so ending it was refused rather " + "than risked. Pass allow_workflow_effect=" + "WorkflowEffect.ADVANCES to end it anyway.", + effects=advances, + triggers=(("ACTION_ID mismatch", WorkflowEffect.ADVANCES),), + ) + if "workflow_id" not in target.model_fields_set: + raise EasyvistaWorkflowEffectRefused( + f"could not tell whether action {wanted} is a workflow step: its " + "record came back without WORKFLOW_ID, so ending it was refused " + "rather than risked. Pass allow_workflow_effect=" + "WorkflowEffect.ADVANCES to end it anyway.", + effects=advances, + triggers=(("WORKFLOW_ID absent", WorkflowEffect.ADVANCES),), + ) + if target.workflow_id is not None: + raise EasyvistaWorkflowEffectRefused( + f"action {wanted} is a workflow step (WORKFLOW_ID " + f"{target.workflow_id}): ending it advances the ticket's workflow. " + "Pass allow_workflow_effect=WorkflowEffect.ADVANCES if that is the " + "intent.", + effects=advances, + triggers=(("WORKFLOW_ID", WorkflowEffect.ADVANCES),), + ) async def _resolve_action_body(self, action: Action) -> Action: """Return ``action`` with its note text resolved onto the memo that shows. @@ -1649,7 +1884,7 @@ async def discover( sweeps tickets, reads ``record["STATUS"]["STATUS_GUID"]``, and merges each guid onto the matching id. That costs one extra ticket sweep even under ``strategy="reference"``; pass ``with_guid=False`` to skip it. The - GUID is what :meth:`set_status` and :meth:`close_ticket` address a + GUID is what :meth:`close_ticket` addresses a status by -- a ``STATUS_ID`` will not work there -- so this is usually the value you came for. A status no sampled ticket currently holds keeps ``guid=None``: the sample cannot reach it. diff --git a/easyvista_python_client/_async/tests/test_client.py b/easyvista_python_client/_async/tests/test_client.py index 86fab01..c045ba9 100644 --- a/easyvista_python_client/_async/tests/test_client.py +++ b/easyvista_python_client/_async/tests/test_client.py @@ -29,8 +29,9 @@ EasyvistaNotFound, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) -from easyvista_python_client.models.action import ActionUpdate, PostAction +from easyvista_python_client.models.action import ActionUpdate, PostAction, PostTask from easyvista_python_client.models.asset import PostAsset from easyvista_python_client.models.department import ( Department, @@ -44,6 +45,7 @@ PostEmployee, ) from easyvista_python_client.models.request import PostRequest, RequestUpdate +from easyvista_python_client.workflow import WorkflowEffect ROOT = "https://ev.test/api/v1/acme" @@ -121,7 +123,31 @@ async def test_update_and_close_ticket(config): ) async with AsyncEasyvistaClient(config) as client: await client.update_ticket("I1", RequestUpdate(impact_id=4)) - await client.close_ticket("I1", comment="resolved") + await client.close_ticket( + "I1", allow_workflow_effect=WorkflowEffect.INTERRUPTS, comment="resolved" + ) + + +async def test_close_ticket_requires_the_opt_in_keyword(config): + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(TypeError): + await client.close_ticket("I1") # type: ignore[call-arg] + + +@respx.mock +async def test_close_ticket_refuses_an_opt_in_that_does_not_cover_interrupting(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.close_ticket( + "I1", allow_workflow_effect=WorkflowEffect.ADVANCES + ) + assert not route.called + + +def test_set_status_is_gone(): + """It was the vendor close request under a name that hid the close.""" + assert not hasattr(AsyncEasyvistaClient, "set_status") @respx.mock @@ -2354,7 +2380,7 @@ async def test_discover_status_populates_the_guid_from_a_ticket_sample(config): A STATUS_GUID is not searchable and no reference read returns one, but every ticket's nested STATUS object carries it. The GUID is what - ``set_status`` and ``close_ticket`` address a status by -- a STATUS_ID will + ``close_ticket`` addresses a status by -- a STATUS_ID will not work there -- so this is usually the value the caller came for. """ respx.get(f"{ROOT}/status").mock( @@ -2557,7 +2583,8 @@ async def test_end_action_forwards_every_keyword_to_the_body(config): A dropped ``start_date`` is invisible in the response -- the server simply derives one, and the derived one is early by the instance's UTC offset. So - this asserts the wire body, not the return value. + this asserts the wire body, not the return value. It tests the body, not the + workflow-step guard, so it opts in and makes no read. """ route = respx.put(f"{ROOT}/actions/I1").mock( return_value=httpx.Response(200, json={"HREF": f"{ROOT}/requests/I1"}) @@ -2570,6 +2597,7 @@ async def test_end_action_forwards_every_keyword_to_the_body(config): end_date="01/09/2026 17:15:00", elapsed_time=15, doneby_mail="tech@example.invalid", + allow_workflow_effect=WorkflowEffect.ADVANCES, ) assert json.loads(route.calls.last.request.content) == { "end_action": { @@ -2589,7 +2617,9 @@ async def test_end_action_addresses_the_ticket_not_the_action(config): return_value=httpx.Response(200, json={"HREF": f"{ROOT}/requests/I1"}) ) async with AsyncEasyvistaClient(config) as client: - await client.end_action("I1", action_id=42) + await client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.ADVANCES + ) assert route.calls.last.request.url.path.endswith("/actions/I1") @@ -2598,3 +2628,359 @@ async def test_end_action_refuses_a_missing_action_id_before_any_request(config) async with AsyncEasyvistaClient(config) as client: with pytest.raises(ValueError, match="end_all"): await client.end_action("I1", end_date="01/09/2026 17:00:00") + + +# --- the workflow guard on the client's writers ------------------------------ + + +@respx.mock +async def test_update_ticket_refuses_a_close_smuggled_through_extra_payload(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = RequestUpdate(extra_payload={"closed": {"status_GUID": "{G}"}}) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.update_ticket("I1", update) + assert not route.called + + +@respx.mock +async def test_update_ticket_sends_it_when_the_call_allows_it(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={"RFC_NUMBER": "I1"}) + ) + update = RequestUpdate(extra_payload={"closed": {"status_GUID": "{G}"}}) + async with AsyncEasyvistaClient(config) as client: + await client.update_ticket( + "I1", update, allow_workflow_effect=WorkflowEffect.INTERRUPTS + ) + assert route.call_count == 1 + assert "closed" in json.loads(route.calls.last.request.content) + + +@respx.mock +async def test_update_ticket_sends_what_the_sync_sends_without_an_opt_in(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={"RFC_NUMBER": "I1"}) + ) + update = RequestUpdate(title="t", description="d", impact_id=3, owner_id=7) + async with AsyncEasyvistaClient(config) as client: + await client.update_ticket("I1", update) + assert route.call_count == 1 + + +@respx.mock +async def test_update_action_refuses_an_end_date_without_an_opt_in(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = ActionUpdate(extra_payload={"END_DATE_UT": "01/10/2026 10:00:00"}) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.update_action(60350, update) + assert not route.called + + +@respx.mock +async def test_update_action_refuses_an_rfc_where_the_action_id_belongs(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = ActionUpdate(extra_payload={"end_action": {}}) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(ValueError, match="action id"): + await client.update_action("I250101_00001", update) + assert not route.called + + +@respx.mock +async def test_update_action_lets_a_reassignment_through(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + update = ActionUpdate(extra_payload={"GROUP_ID": 57}) + async with AsyncEasyvistaClient(config) as client: + await client.update_action(60350, update) + assert route.call_count == 1 + + +@respx.mock +async def test_reassign_action_sends_one_put_without_an_opt_in(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + await client.reassign_action(60350, group_id=57) + assert route.call_count == 1 + assert json.loads(route.calls.last.request.content) == {"group_id": 57} + + +@respx.mock +async def test_reassign_action_refuses_before_any_request(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(ValueError, match="group_id, done_by_id"): + await client.reassign_action(60350) + with pytest.raises(ValueError, match="action id"): + await client.reassign_action("I250101_00001", group_id=57) + assert not route.called + + +@respx.mock +async def test_send_refuses_a_close_unless_allowed(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.send("PUT", "requests/I1", json={"closed": {}}) + assert not route.called + await client.send( + "PUT", + "requests/I1", + json={"closed": {}}, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, + ) + assert route.call_count == 1 + + +@respx.mock +async def test_create_action_and_create_task_refuse_step_columns(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.create_action( + "I1", + PostAction( + action_type_id=94, group_id=3, extra_payload={"workflow_id": 37} + ), + ) + with pytest.raises(EasyvistaWorkflowEffectRefused): + await client.create_task( + "I1", + PostTask( + action_type_id=94, group_id=3, extra_payload={"parent_action_id": 1} + ), + ) + assert not route.called + + +@respx.mock +async def test_update_action_sends_it_when_the_call_allows_it(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + update = ActionUpdate(extra_payload={"END_DATE_UT": "01/10/2026 10:00:00"}) + async with AsyncEasyvistaClient(config) as client: + await client.update_action( + 60350, update, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "END_DATE_UT" in json.loads(route.calls.last.request.content) + + +@respx.mock +async def test_create_action_sends_it_when_the_call_allows_it(config): + route = respx.post(f"{ROOT}/requests/I1/actions").mock( + return_value=httpx.Response(200, json={}) + ) + action = PostAction( + action_type_id=94, group_id=3, extra_payload={"workflow_id": 37} + ) + async with AsyncEasyvistaClient(config) as client: + await client.create_action( + "I1", action, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "workflow_id" in json.loads(route.calls.last.request.content) + + +@respx.mock +async def test_create_task_sends_it_when_the_call_allows_it(config): + route = respx.post(f"{ROOT}/requests/I1/tasks").mock( + return_value=httpx.Response(200, json={}) + ) + task = PostTask( + action_type_id=94, group_id=3, extra_payload={"parent_action_id": 1} + ) + async with AsyncEasyvistaClient(config) as client: + await client.create_task( + "I1", task, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "parent_action_id" in json.loads(route.calls.last.request.content) + + +# --- end_action: the workflow-step guard ------------------------------------ + + +def _probe_route(workflow_id, *, action_id=42): + record = {"ACTION_ID": action_id} + if workflow_id is not None: + record["WORKFLOW_ID"] = workflow_id + return respx.get(f"{ROOT}/actions/42").mock( + return_value=httpx.Response(200, json=record) + ) + + +@respx.mock +async def test_end_action_refuses_a_workflow_step_without_sending_the_end(config): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match=r"is a workflow step \(WORKFLOW_ID" + ) as refused: + await client.end_action("I1", action_id=42) + assert refused.value.triggers == (("WORKFLOW_ID", WorkflowEffect.ADVANCES),) + assert probe.call_count == 1 + assert probe.calls.last.request.url.params["fields"] == "ACTION_ID,WORKFLOW_ID" + assert not end.called + + +@respx.mock +async def test_end_action_ends_the_callers_own_action_once(config): + _probe_route("") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + await client.end_action("I1", action_id=42, end_date="01/10/2026 10:00:00") + assert end.call_count == 1 + + +@respx.mock +async def test_end_action_fails_closed_when_the_read_names_no_workflow_id(config): + _probe_route(None) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match="could not tell" + ) as refused: + await client.end_action("I1", action_id=42) + assert refused.value.triggers == (("WORKFLOW_ID absent", WorkflowEffect.ADVANCES),) + assert not end.called + + +@respx.mock +async def test_end_action_refuses_end_all_without_any_request(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused, match="end_all"): + await client.end_action("I1", end_all=True) + assert not route.called + + +@respx.mock +async def test_end_action_with_advances_allowed_skips_the_read(config): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + await client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.ADVANCES + ) + await client.end_action( + "I1", end_all=True, allow_workflow_effect=WorkflowEffect.ADVANCES + ) + assert not probe.called + assert end.call_count == 2 + + +@respx.mock +async def test_end_action_refuses_a_missing_id_before_the_read(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(ValueError, match="needs an action_id"): + await client.end_action("I1") + assert not route.called + + +@respx.mock +async def test_end_action_does_not_send_the_end_when_the_read_fails(config): + respx.get(f"{ROOT}/actions/42").mock(return_value=httpx.Response(403, json={})) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaAuthError): + await client.end_action("I1", action_id=42) + assert not end.called + + +@respx.mock +async def test_end_action_still_refuses_a_workflow_step_when_only_unknown_is_allowed( + config, +): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused, match="workflow step"): + await client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert probe.call_count == 1 + assert not end.called + + +@respx.mock +async def test_end_action_with_unknown_allowed_still_ends_the_callers_own_action( + config, +): + _probe_route("") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + await client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert end.call_count == 1 + + +@pytest.mark.parametrize("blank", ["", " "]) +@respx.mock +async def test_end_action_refuses_a_blank_action_id_with_no_request_at_all( + config, blank +): + # A blank id would read the collection (GET actions/) and take the first + # row's empty WORKFLOW_ID for the target's. + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises(ValueError, match="positive integer"): + await client.end_action("I1", action_id=blank) + assert not route.called + + +@respx.mock +async def test_end_action_refuses_when_the_read_returns_a_different_action(config): + probe = _probe_route("", action_id=7) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match="different action" + ) as refused: + await client.end_action("I1", action_id=42) + assert refused.value.triggers == (("ACTION_ID mismatch", WorkflowEffect.ADVANCES),) + assert probe.call_count == 1 + assert not end.called + + +@respx.mock +async def test_end_action_compares_the_read_id_with_the_requested_id_as_integers( + config, +): + probe = _probe_route("", action_id="42") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + async with AsyncEasyvistaClient(config) as client: + await client.end_action("I1", action_id=" 42 ") + assert probe.calls.last.request.url.path.endswith("/actions/42") + assert json.loads(end.calls.last.request.content) == { + "end_action": {"action_id": 42} + } diff --git a/easyvista_python_client/_async/tests/test_transport.py b/easyvista_python_client/_async/tests/test_transport.py index 64137fa..655bf91 100644 --- a/easyvista_python_client/_async/tests/test_transport.py +++ b/easyvista_python_client/_async/tests/test_transport.py @@ -24,7 +24,9 @@ EasyvistaRateLimitError, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) +from easyvista_python_client.workflow import WorkflowEffect ROOT = "https://ev.test/api/v1/acme" @@ -945,3 +947,123 @@ async def test_request_spec_headers_override_the_client_level_ones(): def test_request_spec_refuses_the_credential_in_its_headers(): with pytest.raises(ValueError, match="must not set"): RequestSpec("GET", "requests", headers={"authorization": "Bearer other"}) + + +# --- the workflow guard ------------------------------------------------------ + + +def test_request_spec_normalises_and_validates_the_allow_set(): + spec = RequestSpec("PUT", "x", allow_workflow_effect=WorkflowEffect.ADVANCES) + assert spec.allow_workflow_effect == {WorkflowEffect.ADVANCES} + with pytest.raises(TypeError): + RequestSpec("PUT", "x", allow_workflow_effect=WorkflowEffect) + widened = ( + RequestSpec("PUT", "x") + .allowing(WorkflowEffect.INTERRUPTS) + .allowing([WorkflowEffect.UNKNOWN]) + ) + assert widened.allow_workflow_effect == { + WorkflowEffect.INTERRUPTS, + WorkflowEffect.UNKNOWN, + } + assert RequestSpec("PUT", "x") == RequestSpec("PUT", "x", allow_workflow_effect=()) + + +@respx.mock +async def test_send_refuses_a_workflow_write_before_any_request(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with Transport(_cfg()) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + await transport.send(RequestSpec("PUT", "requests/I1", json={"closed": {}})) + assert not route.called + assert refused.value.effects == {WorkflowEffect.INTERRUPTS} + assert "allow_workflow_effect=WorkflowEffect.INTERRUPTS" in str(refused.value) + + +@respx.mock +async def test_send_refuses_an_effect_that_was_not_the_one_allowed(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + spec = RequestSpec("PUT", "requests/I1", json={"closed": {}, "status_id": 8}) + async with Transport(_cfg()) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + await transport.send(spec.allowing(WorkflowEffect.INTERRUPTS)) + assert not route.called + assert refused.value.effects == {WorkflowEffect.UNKNOWN} + + +@respx.mock +async def test_send_sends_an_allowed_workflow_write_exactly_once_on_a_5xx(): + route = respx.put(f"{ROOT}/requests/I1").mock(return_value=httpx.Response(503)) + spec = RequestSpec("PUT", "requests/I1", json={"closed": {}}).allowing( + WorkflowEffect.INTERRUPTS + ) + async with Transport(_cfg(max_retries=3)) as transport: + with pytest.raises(EasyvistaServerError): + await transport.send(spec) + assert route.call_count == 1 + + +@respx.mock +async def test_send_sends_an_allowed_workflow_write_exactly_once_on_a_transport_error(): + route = respx.put(f"{ROOT}/actions/I1").mock(side_effect=httpx.ConnectError("boom")) + spec = RequestSpec( + "PUT", "actions/I1", json={"end_action": {"action_id": 1}} + ).allowing(WorkflowEffect.ADVANCES) + async with Transport(_cfg(max_retries=3)) as transport: + with pytest.raises(EasyvistaConnectionError): + await transport.send(spec) + assert route.call_count == 1 + + +@respx.mock +async def test_send_still_retries_an_ordinary_write(): + route = respx.put(f"{ROOT}/requests/I1").mock( + side_effect=[httpx.Response(503), httpx.Response(200, json={})] + ) + async with Transport(_cfg(max_retries=2)) as transport: + await transport.send(RequestSpec("PUT", "requests/I1", json={"title": "t"})) + assert route.call_count == 2 + + +@respx.mock +async def test_send_refuses_a_dot_segment_path_before_any_request(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + async with Transport(_cfg()) as transport: + with pytest.raises(ValueError, match="dot segment"): + await transport.send( + RequestSpec("PUT", "x/../requests/I1", json={"closed": {}}) + ) + assert not route.called + + +@respx.mock +async def test_send_refuses_a_write_hidden_behind_a_configured_override_header(): + # config.extra_headers is merged onto every request on the wire, so a + # method-override header set there can turn a GET into a write exactly as + # one on the spec can. + route = respx.route().mock(return_value=httpx.Response(200, json={})) + config = _cfg(extra_headers={"X-HTTP-Method-Override": "PUT"}) + async with Transport(config) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + await transport.send(RequestSpec("GET", "requests/I1", json={"closed": {}})) + assert not route.called + assert refused.value.effects == {WorkflowEffect.INTERRUPTS} + + +def test_gate_reads_the_headers_as_they_go_on_the_wire_the_spec_winning(): + configured = BaseTransport(_cfg(extra_headers={"X-HTTP-Method-Override": "PUT"})) + spec = RequestSpec("GET", "requests/I1", json={"closed": {}}) + assert configured.gate(spec.allowing(WorkflowEffect.INTERRUPTS)) == { + WorkflowEffect.INTERRUPTS + } + # The same header on the spec replaces the configured one, so a read stays one. + spec_reads = RequestSpec( + "GET", + "requests/I1", + json={"closed": {}}, + headers={"X-HTTP-Method-Override": "GET"}, + ) + assert configured.gate(spec_reads) == frozenset() + # And a configured header that is not an override changes nothing. + other = BaseTransport(_cfg(extra_headers={"X-Api-Key": "PUT"})) + assert other.gate(spec) == frozenset() diff --git a/easyvista_python_client/_sync/_transport.py b/easyvista_python_client/_sync/_transport.py index 52d0bfe..85fd1c2 100644 --- a/easyvista_python_client/_sync/_transport.py +++ b/easyvista_python_client/_sync/_transport.py @@ -38,7 +38,9 @@ EasyvistaRateLimitError, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) +from easyvista_python_client.workflow import WorkflowEffect, workflow_triggers #: Default chunk size, in bytes, for :meth:`Transport.stream_bytes`. #: @@ -197,6 +199,49 @@ def merge_params( return None return {**self.config.default_params, **(call or {}), **(spec or {})} + def gate(self, spec: RequestSpec) -> frozenset[WorkflowEffect]: + """Refuse ``spec`` unless every workflow effect it names is allowed. + + Returns the effects it names -- all of them allowed by then -- so the + caller can tell a workflow write from an ordinary one. Empty for a read + and for an ordinary write. See :mod:`easyvista_python_client.workflow` + for what is named and why. Raises + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` (no + request is made), or ``ValueError`` for a path with a dot segment, a + percent-encoded slash or backslash, or a raw backslash. + + The headers read for a method override are the ones that go on the + wire: ``config.extra_headers`` with the spec's own laid over them, as + :meth:`headers` and the request merge them, so an override header set in + the configuration cannot hide a write behind a ``GET`` either. + """ + triggers = workflow_triggers( + spec.method, + spec.path, + spec.json, + {**self.config.extra_headers, **(spec.headers or {})}, + ) + refused = tuple( + (what, effect) + for what, effect in triggers + if effect not in spec.allow_workflow_effect + ) + if refused: + effects = frozenset(effect for _, effect in refused) + names = sorted(f"WorkflowEffect.{effect.name}" for effect in effects) + allow = names[0] if len(names) == 1 else "{" + ", ".join(names) + "}" + named = ", ".join(f"{what!r} ({effect.name})" for what, effect in refused) + raise EasyvistaWorkflowEffectRefused( + f"refused {spec.method} {spec.path} before sending it: {named} may " + f"change the ticket's workflow. If that is the intent, pass " + f"allow_workflow_effect={allow} to this call, or to send() when " + f"the method does not take it. See docs/vendor-api-reference.md, " + f"'Ticket workflow'.", + effects=effects, + triggers=refused, + ) + return frozenset(effect for _, effect in triggers) + def auth(self) -> httpx.Auth | None: if self.config.uses_basic_auth: return httpx.BasicAuth(self.config.login or "", self.config.password or "") @@ -362,9 +407,16 @@ def send( ``config.default_params`` sits under both -- see :meth:`merge_params` for the full ordering. + + A request naming a workflow effect is refused before anything is sent + unless the spec allows it (:meth:`BaseTransport.gate`). One that is + allowed is attempted **once**: each close inserts another anticipated + closing action and each end ends whatever is open, so a resend after a + lost response is not a repeat of the same request. """ + effects = self.gate(spec) retryer = Retrying( - stop=stop_after_attempt(self.config.max_retries + 1), + stop=stop_after_attempt(1 if effects else self.config.max_retries + 1), wait=wait_exponential(multiplier=0.5, max=10), retry=retry_if_exception_type((_RetryableResponse, httpx.TransportError)), reraise=True, diff --git a/easyvista_python_client/_sync/client.py b/easyvista_python_client/_sync/client.py index 225290d..7ae7792 100644 --- a/easyvista_python_client/_sync/client.py +++ b/easyvista_python_client/_sync/client.py @@ -49,6 +49,7 @@ EasyvistaAuthError, EasyvistaError, EasyvistaNotFound, + EasyvistaWorkflowEffectRefused, ) from easyvista_python_client.field_model import parse_memo from easyvista_python_client.filters import ev_equals_filter, is_safe_ev_value @@ -88,6 +89,7 @@ from easyvista_python_client.resources import employees as employees_res from easyvista_python_client.resources import requests as requests_res from easyvista_python_client.resources.discovery import SWAGGER_PATH +from easyvista_python_client.workflow import WorkflowEffect, as_effects # Width of the action-body fan-out: a ceiling on requests in flight at once on # the async surface, inert on the sync one. This is the one fan-out here whose @@ -98,6 +100,12 @@ # -- the server, not the client, is the bottleneck). _ACTION_FANOUT = 8 +# The projection end_action's guard reads: just enough to tell a workflow step +# (WORKFLOW_ID set) from a caller's own action (WORKFLOW_ID empty). Projected +# rather than left to the default item read so that the column is asked for +# by name -- see integration_tests/test_live_workflow_guard.py. +_WORKFLOW_PROBE_FIELDS = ("ACTION_ID", "WORKFLOW_ID") + def _unavailable_reason(exc: EasyvistaError) -> str: """One ``InstanceProfile.unavailable`` value, first token machine-readable. @@ -162,6 +170,7 @@ def send( params: Mapping[str, Any] | None = None, json: Any = None, headers: Mapping[str, str] | None = None, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), ) -> Any: """Issue an arbitrary request against this instance's API root. @@ -182,7 +191,8 @@ def send( :meth:`download_document` or :meth:`stream_document`. Everything else is shared with the typed methods: ``config.max_retries`` - attempts with the same backoff, and the same exception mapping -- 401 and + attempts with the same backoff (one attempt for an allowed workflow + write), and the same exception mapping -- 401 and 403 to :class:`~easyvista_python_client.EasyvistaAuthError`, 404 to :class:`~easyvista_python_client.EasyvistaNotFound`, 400 and 590 to :class:`~easyvista_python_client.EasyvistaValidationError`, with 590 never @@ -190,6 +200,18 @@ def send( ``config.default_params`` is merged under ``params``; ``headers`` is merged over the client-level ones and may not carry ``Authorization``. + ``allow_workflow_effect`` is the way past the workflow guard. A write + whose content may change a ticket's workflow -- a ``closed``, + ``end_action``, ``suspended`` or ``restarted`` body on any path, a + status or catalog column on a ticket, an end date or type on an action, + any write to ``actions/{rfc_number}`` (the vendor's end-action route; an + integer id addresses an action instead), a ``requests/{rfc}/close``-style + route -- is refused with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` before + anything is sent, unless this argument names every + :class:`~easyvista_python_client.WorkflowEffect` it carries. See + ``docs/vendor-api-reference.md``, "Ticket workflow". + Returns the decoded JSON body, or ``{}`` when the response has none. Nothing is validated into a model and no envelope is unwrapped: the caller owns the shape, which is the point -- there is no model for a @@ -206,12 +228,29 @@ def send( path, json=json, headers=dict(headers) if headers else None, + allow_workflow_effect=as_effects(allow_workflow_effect), ), params=params, ) # --- tickets ------------------------------------------------------------- def create_ticket(self, ticket: PostRequest) -> Request: + """Create one ticket -- which starts its workflow. + + Per the vendor create page (tier 1, + https://docs.easyvista.com/docs/rest-api-create-an-incident-request.md): + a CALL action is inserted with its end date set to the ticket's + submission date, so it is born ended, then "3. The workflow + associated with the ticket is started." A fresh ticket carries one open + workflow-step action (tier 4, 2026-09-01, one instance). + + **Do not follow the create with** :meth:`close_ticket` **to land an + initial status.** That interrupts the workflow you just started; read + the status the ticket landed on with :meth:`get_ticket` instead. + + A 590 on create may still have created the row: reconcile by + ``EXTERNAL_REFERENCE`` rather than retrying. + """ spec, parse = requests_res.build_create_ticket( ticket, context=self._validation_context ) @@ -445,67 +484,88 @@ def ticket_statistics( stats.population_total = population_total return stats - def update_ticket(self, rfc_number: str, update: RequestUpdate) -> Request: + def update_ticket( + self, + rfc_number: str, + update: RequestUpdate, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Request: """Update a ticket's writable fields. - Cannot set a status: there is no flat status update on this API. See - :meth:`set_status`, and :class:`RequestUpdate` for the measurements. + Cannot set a status: there is no flat status update on this API (see + :class:`RequestUpdate` for the measurements), the vendor documents no + status write that leaves the workflow alone, and this package has none + -- a ticket's status follows its workflow. See :meth:`close_ticket` for + the one request the vendor documents that does set a status. + + A body that may change the workflow -- a status or catalog column, or a + workflow-control body, typically put in ``extra_payload`` -- is refused + before it is sent unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. The fields :class:`RequestUpdate` declares need no opt-in. """ spec, parse = requests_res.build_update_ticket( rfc_number, update, context=self._validation_context ) - return parse(self._transport.send(spec)) - - def set_status( - self, rfc_number: str, *, status_guid: str, comment: str | None = None - ) -> Request: - """Set a ticket's status, addressed by ``STATUS_GUID``. - - This is the API's only working status write, and it reaches **every** - status rather than only terminal ones: given six different status GUIDs - in turn, a fresh ticket landed on exactly the status requested every - time, non-terminal ones included. - - It sends the documented ``{"closed": {"status_GUID": ...}}`` body -- the - same request :meth:`close_ticket` sends, under a name that matches what - it does, because "close" is what the wire calls it and not what it is - limited to. - - Note the addressing. A ``STATUS_GUID`` is not a ``STATUS_ID``; the two - are different columns, and only the GUID works here. Read a status's GUID - off any ticket in that status (the nested ``STATUS`` object carries - ``STATUS_GUID``) -- they are stable per instance but are **not** - portable between instances. - """ - spec, parse = requests_res.build_set_status( - rfc_number, - status_guid=status_guid, - comment=comment, - context=self._validation_context, - ) - return parse(self._transport.send(spec)) + return parse(self._transport.send(spec.allowing(allow_workflow_effect))) def close_ticket( self, rfc_number: str, *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect], status_guid: str | None = None, delete_actions: int | bool | None = None, comment: str | None = None, end_date: str | None = None, catalog_guid: str | None = None, ) -> Request: - """Close a ticket, via the vendor's documented close route. + """Close a ticket early -- the vendor's close request, which stops its workflow. - Sends ``PUT requests/{rfc}`` with a ``closed`` wrapper -- + Sends ``PUT requests/{rfc}`` with a ``closed`` body, as the vendor + documents it: https://docs.easyvista.com/docs/rest-api-close-an-incident-request.md. - Every argument is optional. With no ``end_date`` the server stamps now. - With no ``status_guid`` this client sends no status of its own -- but - **where the ticket then lands is not established here**: the behaviour - is not recorded in ``docs/vendor-api-reference.md`` and no live test - exercises the omitted form, every one of them passing an explicit - ``status_guid``. Try it on a throwaway ticket and re-read before - relying on it (open item O-CLOSE-DEFAULT). + That page lists what the request does, and none of it depends on the + status passed (tier 1, re-read 2026-10-02): + + * "The workflow of the ticket is interrupted." + * The status is set to ``status_guid``, which the page calls "the final + status of the ticket". Omitted, the vendor documents the default as + the Closed meta-status. + * "The unfinished actions associated with the ticket are deleted" when + ``delete_actions`` is true. Otherwise ``end_date`` is the "Closing date + of open actions associated with the ticket" -- read here as: they are + ended, not left open. That reading rests on the parameter row and one + observation (2026-09-01, one instance), not on an explicit sentence. + * "An anticipated closing action associated with the ticket is + inserted." -- one per call, so every close adds a row. + + **So this is not a status setter. The vendor documents no status + setter, and this package has none.** A ticket's status follows its + workflow: "Advancing through the steps of a workflow + changes the status of a ticket." (tier 1, + https://docs.easyvista.com/docs/references-tables.md, Statuses section). + A non-final status sent + here still interrupts the workflow and closes the ticket's open + actions -- the page documents final statuses only, and nothing exempts + the others. To move a ticket through its workflow, end the workflow + step's open action instead; see :meth:`end_action`. + + That is why ``allow_workflow_effect`` is **required**: pass + ``WorkflowEffect.INTERRUPTS`` to say at the call site that interrupting + the workflow is the intent. Anything that does not include it is + refused with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` before + a request is made, and the allowed request is sent once, never + retried:: + + client.close_ticket( + rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, + status_guid=CLOSED_GUID, + ) + after = client.get_ticket(rfc) + assert after.end_date_ut is not None # the close actually landed **Verify the close by re-reading the status, not by the return value.** A status id is per-instance configuration and nothing about it is @@ -515,11 +575,7 @@ def close_ticket( skip a ticket it believed was already closed. Read ``get_ticket(rfc).status_id`` (or ``.reference("STATUS")`` for the label) afterwards, and compare against a status you resolved from the - instance rather than a constant:: - - client.close_ticket(rfc, status_guid=CLOSED_GUID) - after = client.get_ticket(rfc) - assert after.end_date_ut is not None # the close actually landed + instance rather than a constant. ``end_date_ut`` is the more portable signal than any status id: it is empty while a ticket is being worked and stamped once it is finished. @@ -533,10 +589,9 @@ def close_ticket( distinguishes the two without resolving the status against the instance. - ``status_guid`` reaches **any** status, not only terminal ones -- see - :meth:`set_status`, which is this same request under a name that says - so. ``catalog_guid`` requalifies the ticket as it closes. - ``delete_actions`` drops its actions. + ``catalog_guid`` requalifies the ticket as it closes -- the vendor notes + it is needed only for that. ``delete_actions`` deletes the unfinished + actions instead of ending them. ``end_date`` takes the instance's own date format, which is not ISO 8601 everywhere (``dd/mm/yyyy`` on the verified instance -- read @@ -553,10 +608,16 @@ def close_ticket( catalog_guid=catalog_guid, context=self._validation_context, ) - return parse(self._transport.send(spec)) + return parse(self._transport.send(spec.allowing(allow_workflow_effect))) # --- actions ------------------------------------------------------------- - def create_action(self, rfc_number: str, action: PostAction) -> Action: + def create_action( + self, + rfc_number: str, + action: PostAction, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Create one action on a ticket. The returned :class:`Action` carries **no usable ``action_id``**: the @@ -599,13 +660,25 @@ def create_action(self, rfc_number: str, action: PostAction) -> Action: refused on a ticket that accepted the same body earlier. The messages are literal, not a stage gate. :meth:`create_task` is not parent-resolved and is unaffected. + + A body that would create the record already tied into the workflow -- + ``WORKFLOW_ID``, ``STAGE_ID`` or ``STATUS_ID_ON_TERMINATE`` through + ``extra_payload``, an end date on an action, a parent on a task -- is + refused unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. """ spec, parse = actions_res.build_create_action( rfc_number, action, context=self._validation_context ) - return parse(self._transport.send(spec)) + return parse(self._transport.send(spec.allowing(allow_workflow_effect))) - def create_task(self, rfc_number: str, task: PostTask) -> Action: + def create_task( + self, + rfc_number: str, + task: PostTask, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Create a task on a ticket — an action that arrives already ENDED. **This is how you post a comment.** A task and an action are the same @@ -654,11 +727,17 @@ def create_task(self, rfc_number: str, task: PostTask) -> Action: usable ``action_id``** — the create response is an HREF naming the parent request. Diff :meth:`list_actions` across the call to address what you just created. + + A body that would create the record already tied into the workflow -- + ``WORKFLOW_ID``, ``STAGE_ID`` or ``STATUS_ID_ON_TERMINATE`` through + ``extra_payload``, an end date on an action, a parent on a task -- is + refused unless ``allow_workflow_effect`` names the effect; see + :meth:`send`. """ spec, parse = actions_res.build_create_task( rfc_number, task, context=self._validation_context ) - return parse(self._transport.send(spec)) + return parse(self._transport.send(spec.allowing(allow_workflow_effect))) def list_actions( self, @@ -775,7 +854,13 @@ def get_action( ) return parse(self._transport.send(spec, params=params)) - def update_action(self, action_id: str | int, update: ActionUpdate) -> Action: + def update_action( + self, + action_id: str | int, + update: ActionUpdate, + *, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), + ) -> Action: """Edit an existing action's note text. ``ActionUpdate`` carries two fields, ``description`` and ``comment``, @@ -800,6 +885,14 @@ def update_action(self, action_id: str | int, update: ActionUpdate) -> Action: recorded for that verb is what this API answers for an absent route as well as a denied one, so it did not distinguish them. + ``action_id`` must be a positive integer: ``PUT actions/{rfc_number}`` + is the vendor's end-action route on the same path, so an RFC number is + refused rather than sent. A body that may end, re-type, re-parent or + move the action (an end date, ``WORKFLOW_ID``, ``ACTION_TYPE_ID``, ..., + through ``extra_payload``) is refused unless ``allow_workflow_effect`` + names the effect; see :meth:`send`. ``GROUP_ID`` and ``DONE_BY_ID`` are + not refused -- see :meth:`reassign_action` for reassignment. + The returned :class:`Action` is the API's own echo and is **not verified**: the PUT's response body has never been captured, and if it answers empty or href-only the parser yields an ``Action`` whose fields @@ -809,6 +902,71 @@ def update_action(self, action_id: str | int, update: ActionUpdate) -> Action: spec, parse = actions_res.build_update_action( action_id, update, context=self._validation_context ) + return parse(self._transport.send(spec.allowing(allow_workflow_effect))) + + def reassign_action( + self, + action_id: str | int, + *, + group_id: int | None = None, + done_by_id: int | None = None, + ) -> Action: + """Reassign an action to another group and/or person. + + The closest API equivalent of the UI's reassignment of an action (its + "Assign action" button, which runs a wizard that may do more than this + one write does), and the way to escalate the open workflow step to + another group without ending it. Sends + ``PUT actions/{id}`` with the group and/or the person in charge; at + least one is required, and both are positive integers -- ids are + per-deployment, so look them up rather than hardcoding them. A group + id comes from :meth:`discover` or :meth:`list_reference_table` where + the groups table is readable to you; on the measured instance + (2026-10-02, one instance) ``GET groups`` answered 403, which this + API also answers for an absent route, so it settles nothing about why. + If yours does too, take the group id from your administrator, or from + a record that already carries one (an existing action's ``GROUP_ID``). + ``done_by_id`` is an employee id: find one with + :meth:`search_employees` or :meth:`get_employee`. + + **The vendor documents no reassignment route.** The UI's transfer is a + wizard, and ``PUT actions/{id}`` accepts "all the fields from the + AM_ACTION table except" a list that does not name these two + (tier 1, https://docs.easyvista.com/docs/rest-api-update-an-action.md). + So what this write does is measured, not specified: + + Measured 2026-10-02 on one instance (Service Manager 2025.3; two + tickets, so it may not generalise), with the body + ``{"group_id": }``. On **both** tickets -- a service request at + status id 6 and a fresh incident at status id 12 (ids are + per-instance) -- the group was stored: ``GROUP_ID`` went 57 -> 50 on + the open workflow step, and read 50 on a re-read immediately and again + five seconds later. On both, the step stayed open (``END_DATE_UT`` + empty), the ticket's ``STATUS_ID`` and ``END_DATE_UT`` did not move, + the open actions were unchanged, and no new action rows appeared. On + the **first ticket only** the step was recorded as a type-20 action + with ``WORKFLOW_ID`` set; its ``WORKFLOW_ID`` was unchanged, + ``DONE_BY_ID`` stayed empty, and no ticket field changed. + **The ticket's own ``OWNING_GROUP_ID`` does not follow the action's + group** -- it stayed 57 on the first ticket, the only one read for it, + so reassigning a step is not reassigning the ticket. The lower-case + key ``group_id`` was the one sent; the upper-case spelling was never + needed. The person (``done_by_id``) was **not measured**: the same + body shape is sent, but nothing here shows what the instance does + with it. Whether the UI wizard's notifications fire is not observable + from the API. + + Not refused by the workflow guard: the group and the person are data + the workflow reads, not workflow state. Re-read with :meth:`get_action` + before trusting the result -- this API answers 200 while dropping a + field it did not store. + """ + spec, parse = actions_res.build_reassign_action( + action_id, + group_id=group_id, + done_by_id=done_by_id, + context=self._validation_context, + ) return parse(self._transport.send(spec)) def end_action( @@ -821,6 +979,7 @@ def end_action( start_date: str | None = None, elapsed_time: int | str | None = None, doneby_mail: str | None = None, + allow_workflow_effect: WorkflowEffect | Iterable[WorkflowEffect] = (), ) -> Action: """Report an action as done — the step :meth:`create_action` leaves open. @@ -855,7 +1014,9 @@ def end_action( the id-less form as ending *every open action on the ticket*, which on a ticket whose only open action is its workflow step means resolving it. That form is reachable only through ``end_all=True``; - a bare ``action_id=None`` raises ``ValueError`` before any request. + a bare ``action_id=None`` raises ``ValueError`` before any request, + as does an ``action_id`` that is not a positive integer -- a blank, or + an RFC number -- which is sent as an integer when it is one. The guard exists because ``Action.action_id`` is legitimately ``None`` all over this package — :meth:`create_action`'s response carries no id, and a ``fields=`` projection without ``ACTION_ID`` @@ -864,6 +1025,27 @@ def end_action( with a single open action, so *how* it behaves against several open at once is vendor-documented, not measured here. + **And why ending a workflow step must be asked for.** Unless + ``allow_workflow_effect`` includes ``WorkflowEffect.ADVANCES``, this + method reads the action first -- one item read projecting + ``ACTION_ID`` and ``WORKFLOW_ID`` -- and refuses, with + :class:`~easyvista_python_client.EasyvistaWorkflowEffectRefused` and + no end request sent, when the action is a workflow step + (``WORKFLOW_ID`` set), when the record comes back without the + column at all, which cannot be told apart from a step, or when the + record names a different ``ACTION_ID`` than the one asked for. + ``WORKFLOW_ID`` is what separates the engine's rows from a caller's + (tier 4, 1500 of 1500 rows, 2026-09-02, one instance -- see + :attr:`Action.is_workflow_generated`). Whether an action created under + the step by :meth:`create_action` carries one is unmeasured; if it + does, ending it is refused too -- the safe direction. ``end_all=True`` + is refused outright without ``ADVANCES``. Ending your own action + needs no opt-in. If the read fails, its error propagates and the end + request is not sent -- a 403 there says nothing about whether ending + is permitted. The end request, once sent, is never retried. Both kinds + of action were read with the column present (live, 2026-10-02, one + instance). + Both dates are passed through as **strings**, because the accepted format follows the instance rather than a standard: it is not ISO 8601 on every deployment, and accepting a ``datetime`` would mean this @@ -927,7 +1109,60 @@ def end_action( doneby_mail=doneby_mail, context=self._validation_context, ) - return parse(self._transport.send(spec)) + allowed = as_effects(allow_workflow_effect) + if WorkflowEffect.ADVANCES not in allowed: + self._refuse_ending_a_workflow_step(action_id, end_all=end_all) + allowed = allowed | {WorkflowEffect.ADVANCES} + return parse(self._transport.send(spec.allowing(allowed))) + + def _refuse_ending_a_workflow_step( + self, action_id: str | int | None, *, end_all: bool + ) -> None: + """Raise unless one read shows ``action_id`` is not a workflow step.""" + advances = frozenset({WorkflowEffect.ADVANCES}) + if end_all or action_id is None: + raise EasyvistaWorkflowEffectRefused( + "end_all=True ends every open action on the ticket, its workflow " + "step included, which advances the ticket's workflow. Pass " + "allow_workflow_effect=WorkflowEffect.ADVANCES if that is the intent.", + effects=advances, + triggers=(("end_action", WorkflowEffect.ADVANCES),), + ) + # end_action built the end spec first, which refused anything but a + # positive integer, so this conversion cannot fail and the read + # addresses exactly the id the end request will name. + wanted = int(action_id) + spec, parse = actions_res.build_get_action( + wanted, fields=_WORKFLOW_PROBE_FIELDS, context=self._validation_context + ) + target = parse(self._transport.send(spec)) + if target.action_id is not None and target.action_id != wanted: + raise EasyvistaWorkflowEffectRefused( + f"the read of action {wanted} returned a different action " + f"(ACTION_ID {target.action_id}), so ending it was refused rather " + "than risked. Pass allow_workflow_effect=" + "WorkflowEffect.ADVANCES to end it anyway.", + effects=advances, + triggers=(("ACTION_ID mismatch", WorkflowEffect.ADVANCES),), + ) + if "workflow_id" not in target.model_fields_set: + raise EasyvistaWorkflowEffectRefused( + f"could not tell whether action {wanted} is a workflow step: its " + "record came back without WORKFLOW_ID, so ending it was refused " + "rather than risked. Pass allow_workflow_effect=" + "WorkflowEffect.ADVANCES to end it anyway.", + effects=advances, + triggers=(("WORKFLOW_ID absent", WorkflowEffect.ADVANCES),), + ) + if target.workflow_id is not None: + raise EasyvistaWorkflowEffectRefused( + f"action {wanted} is a workflow step (WORKFLOW_ID " + f"{target.workflow_id}): ending it advances the ticket's workflow. " + "Pass allow_workflow_effect=WorkflowEffect.ADVANCES if that is the " + "intent.", + effects=advances, + triggers=(("WORKFLOW_ID", WorkflowEffect.ADVANCES),), + ) def _resolve_action_body(self, action: Action) -> Action: """Return ``action`` with its note text resolved onto the memo that shows. @@ -1649,7 +1884,7 @@ def discover( sweeps tickets, reads ``record["STATUS"]["STATUS_GUID"]``, and merges each guid onto the matching id. That costs one extra ticket sweep even under ``strategy="reference"``; pass ``with_guid=False`` to skip it. The - GUID is what :meth:`set_status` and :meth:`close_ticket` address a + GUID is what :meth:`close_ticket` addresses a status by -- a ``STATUS_ID`` will not work there -- so this is usually the value you came for. A status no sampled ticket currently holds keeps ``guid=None``: the sample cannot reach it. diff --git a/easyvista_python_client/_sync/tests/test_client.py b/easyvista_python_client/_sync/tests/test_client.py index e492cb9..3010a8e 100644 --- a/easyvista_python_client/_sync/tests/test_client.py +++ b/easyvista_python_client/_sync/tests/test_client.py @@ -29,8 +29,9 @@ EasyvistaNotFound, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) -from easyvista_python_client.models.action import ActionUpdate, PostAction +from easyvista_python_client.models.action import ActionUpdate, PostAction, PostTask from easyvista_python_client.models.asset import PostAsset from easyvista_python_client.models.department import ( Department, @@ -44,6 +45,7 @@ PostEmployee, ) from easyvista_python_client.models.request import PostRequest, RequestUpdate +from easyvista_python_client.workflow import WorkflowEffect ROOT = "https://ev.test/api/v1/acme" @@ -121,7 +123,31 @@ def test_update_and_close_ticket(config): ) with EasyvistaClient(config) as client: client.update_ticket("I1", RequestUpdate(impact_id=4)) - client.close_ticket("I1", comment="resolved") + client.close_ticket( + "I1", allow_workflow_effect=WorkflowEffect.INTERRUPTS, comment="resolved" + ) + + +def test_close_ticket_requires_the_opt_in_keyword(config): + with EasyvistaClient(config) as client: + with pytest.raises(TypeError): + client.close_ticket("I1") # type: ignore[call-arg] + + +@respx.mock +def test_close_ticket_refuses_an_opt_in_that_does_not_cover_interrupting(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.close_ticket( + "I1", allow_workflow_effect=WorkflowEffect.ADVANCES + ) + assert not route.called + + +def test_set_status_is_gone(): + """It was the vendor close request under a name that hid the close.""" + assert not hasattr(EasyvistaClient, "set_status") @respx.mock @@ -2354,7 +2380,7 @@ def test_discover_status_populates_the_guid_from_a_ticket_sample(config): A STATUS_GUID is not searchable and no reference read returns one, but every ticket's nested STATUS object carries it. The GUID is what - ``set_status`` and ``close_ticket`` address a status by -- a STATUS_ID will + ``close_ticket`` addresses a status by -- a STATUS_ID will not work there -- so this is usually the value the caller came for. """ respx.get(f"{ROOT}/status").mock( @@ -2557,7 +2583,8 @@ def test_end_action_forwards_every_keyword_to_the_body(config): A dropped ``start_date`` is invisible in the response -- the server simply derives one, and the derived one is early by the instance's UTC offset. So - this asserts the wire body, not the return value. + this asserts the wire body, not the return value. It tests the body, not the + workflow-step guard, so it opts in and makes no read. """ route = respx.put(f"{ROOT}/actions/I1").mock( return_value=httpx.Response(200, json={"HREF": f"{ROOT}/requests/I1"}) @@ -2570,6 +2597,7 @@ def test_end_action_forwards_every_keyword_to_the_body(config): end_date="01/09/2026 17:15:00", elapsed_time=15, doneby_mail="tech@example.invalid", + allow_workflow_effect=WorkflowEffect.ADVANCES, ) assert json.loads(route.calls.last.request.content) == { "end_action": { @@ -2589,7 +2617,9 @@ def test_end_action_addresses_the_ticket_not_the_action(config): return_value=httpx.Response(200, json={"HREF": f"{ROOT}/requests/I1"}) ) with EasyvistaClient(config) as client: - client.end_action("I1", action_id=42) + client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.ADVANCES + ) assert route.calls.last.request.url.path.endswith("/actions/I1") @@ -2598,3 +2628,359 @@ def test_end_action_refuses_a_missing_action_id_before_any_request(config): with EasyvistaClient(config) as client: with pytest.raises(ValueError, match="end_all"): client.end_action("I1", end_date="01/09/2026 17:00:00") + + +# --- the workflow guard on the client's writers ------------------------------ + + +@respx.mock +def test_update_ticket_refuses_a_close_smuggled_through_extra_payload(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = RequestUpdate(extra_payload={"closed": {"status_GUID": "{G}"}}) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.update_ticket("I1", update) + assert not route.called + + +@respx.mock +def test_update_ticket_sends_it_when_the_call_allows_it(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={"RFC_NUMBER": "I1"}) + ) + update = RequestUpdate(extra_payload={"closed": {"status_GUID": "{G}"}}) + with EasyvistaClient(config) as client: + client.update_ticket( + "I1", update, allow_workflow_effect=WorkflowEffect.INTERRUPTS + ) + assert route.call_count == 1 + assert "closed" in json.loads(route.calls.last.request.content) + + +@respx.mock +def test_update_ticket_sends_what_the_sync_sends_without_an_opt_in(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={"RFC_NUMBER": "I1"}) + ) + update = RequestUpdate(title="t", description="d", impact_id=3, owner_id=7) + with EasyvistaClient(config) as client: + client.update_ticket("I1", update) + assert route.call_count == 1 + + +@respx.mock +def test_update_action_refuses_an_end_date_without_an_opt_in(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = ActionUpdate(extra_payload={"END_DATE_UT": "01/10/2026 10:00:00"}) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.update_action(60350, update) + assert not route.called + + +@respx.mock +def test_update_action_refuses_an_rfc_where_the_action_id_belongs(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + update = ActionUpdate(extra_payload={"end_action": {}}) + with EasyvistaClient(config) as client: + with pytest.raises(ValueError, match="action id"): + client.update_action("I250101_00001", update) + assert not route.called + + +@respx.mock +def test_update_action_lets_a_reassignment_through(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + update = ActionUpdate(extra_payload={"GROUP_ID": 57}) + with EasyvistaClient(config) as client: + client.update_action(60350, update) + assert route.call_count == 1 + + +@respx.mock +def test_reassign_action_sends_one_put_without_an_opt_in(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + client.reassign_action(60350, group_id=57) + assert route.call_count == 1 + assert json.loads(route.calls.last.request.content) == {"group_id": 57} + + +@respx.mock +def test_reassign_action_refuses_before_any_request(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(ValueError, match="group_id, done_by_id"): + client.reassign_action(60350) + with pytest.raises(ValueError, match="action id"): + client.reassign_action("I250101_00001", group_id=57) + assert not route.called + + +@respx.mock +def test_send_refuses_a_close_unless_allowed(config): + route = respx.put(f"{ROOT}/requests/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.send("PUT", "requests/I1", json={"closed": {}}) + assert not route.called + client.send( + "PUT", + "requests/I1", + json={"closed": {}}, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, + ) + assert route.call_count == 1 + + +@respx.mock +def test_create_action_and_create_task_refuse_step_columns(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.create_action( + "I1", + PostAction( + action_type_id=94, group_id=3, extra_payload={"workflow_id": 37} + ), + ) + with pytest.raises(EasyvistaWorkflowEffectRefused): + client.create_task( + "I1", + PostTask( + action_type_id=94, group_id=3, extra_payload={"parent_action_id": 1} + ), + ) + assert not route.called + + +@respx.mock +def test_update_action_sends_it_when_the_call_allows_it(config): + route = respx.put(f"{ROOT}/actions/60350").mock( + return_value=httpx.Response(200, json={}) + ) + update = ActionUpdate(extra_payload={"END_DATE_UT": "01/10/2026 10:00:00"}) + with EasyvistaClient(config) as client: + client.update_action( + 60350, update, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "END_DATE_UT" in json.loads(route.calls.last.request.content) + + +@respx.mock +def test_create_action_sends_it_when_the_call_allows_it(config): + route = respx.post(f"{ROOT}/requests/I1/actions").mock( + return_value=httpx.Response(200, json={}) + ) + action = PostAction( + action_type_id=94, group_id=3, extra_payload={"workflow_id": 37} + ) + with EasyvistaClient(config) as client: + client.create_action( + "I1", action, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "workflow_id" in json.loads(route.calls.last.request.content) + + +@respx.mock +def test_create_task_sends_it_when_the_call_allows_it(config): + route = respx.post(f"{ROOT}/requests/I1/tasks").mock( + return_value=httpx.Response(200, json={}) + ) + task = PostTask( + action_type_id=94, group_id=3, extra_payload={"parent_action_id": 1} + ) + with EasyvistaClient(config) as client: + client.create_task( + "I1", task, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert route.call_count == 1 + assert "parent_action_id" in json.loads(route.calls.last.request.content) + + +# --- end_action: the workflow-step guard ------------------------------------ + + +def _probe_route(workflow_id, *, action_id=42): + record = {"ACTION_ID": action_id} + if workflow_id is not None: + record["WORKFLOW_ID"] = workflow_id + return respx.get(f"{ROOT}/actions/42").mock( + return_value=httpx.Response(200, json=record) + ) + + +@respx.mock +def test_end_action_refuses_a_workflow_step_without_sending_the_end(config): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match=r"is a workflow step \(WORKFLOW_ID" + ) as refused: + client.end_action("I1", action_id=42) + assert refused.value.triggers == (("WORKFLOW_ID", WorkflowEffect.ADVANCES),) + assert probe.call_count == 1 + assert probe.calls.last.request.url.params["fields"] == "ACTION_ID,WORKFLOW_ID" + assert not end.called + + +@respx.mock +def test_end_action_ends_the_callers_own_action_once(config): + _probe_route("") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + client.end_action("I1", action_id=42, end_date="01/10/2026 10:00:00") + assert end.call_count == 1 + + +@respx.mock +def test_end_action_fails_closed_when_the_read_names_no_workflow_id(config): + _probe_route(None) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match="could not tell" + ) as refused: + client.end_action("I1", action_id=42) + assert refused.value.triggers == (("WORKFLOW_ID absent", WorkflowEffect.ADVANCES),) + assert not end.called + + +@respx.mock +def test_end_action_refuses_end_all_without_any_request(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused, match="end_all"): + client.end_action("I1", end_all=True) + assert not route.called + + +@respx.mock +def test_end_action_with_advances_allowed_skips_the_read(config): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.ADVANCES + ) + client.end_action( + "I1", end_all=True, allow_workflow_effect=WorkflowEffect.ADVANCES + ) + assert not probe.called + assert end.call_count == 2 + + +@respx.mock +def test_end_action_refuses_a_missing_id_before_the_read(config): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(ValueError, match="needs an action_id"): + client.end_action("I1") + assert not route.called + + +@respx.mock +def test_end_action_does_not_send_the_end_when_the_read_fails(config): + respx.get(f"{ROOT}/actions/42").mock(return_value=httpx.Response(403, json={})) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaAuthError): + client.end_action("I1", action_id=42) + assert not end.called + + +@respx.mock +def test_end_action_still_refuses_a_workflow_step_when_only_unknown_is_allowed( + config, +): + probe = _probe_route(37) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises(EasyvistaWorkflowEffectRefused, match="workflow step"): + client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert probe.call_count == 1 + assert not end.called + + +@respx.mock +def test_end_action_with_unknown_allowed_still_ends_the_callers_own_action( + config, +): + _probe_route("") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + client.end_action( + "I1", action_id=42, allow_workflow_effect=WorkflowEffect.UNKNOWN + ) + assert end.call_count == 1 + + +@pytest.mark.parametrize("blank", ["", " "]) +@respx.mock +def test_end_action_refuses_a_blank_action_id_with_no_request_at_all( + config, blank +): + # A blank id would read the collection (GET actions/) and take the first + # row's empty WORKFLOW_ID for the target's. + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with EasyvistaClient(config) as client: + with pytest.raises(ValueError, match="positive integer"): + client.end_action("I1", action_id=blank) + assert not route.called + + +@respx.mock +def test_end_action_refuses_when_the_read_returns_a_different_action(config): + probe = _probe_route("", action_id=7) + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + with pytest.raises( + EasyvistaWorkflowEffectRefused, match="different action" + ) as refused: + client.end_action("I1", action_id=42) + assert refused.value.triggers == (("ACTION_ID mismatch", WorkflowEffect.ADVANCES),) + assert probe.call_count == 1 + assert not end.called + + +@respx.mock +def test_end_action_compares_the_read_id_with_the_requested_id_as_integers( + config, +): + probe = _probe_route("", action_id="42") + end = respx.put(f"{ROOT}/actions/I1").mock( + return_value=httpx.Response(200, json={}) + ) + with EasyvistaClient(config) as client: + client.end_action("I1", action_id=" 42 ") + assert probe.calls.last.request.url.path.endswith("/actions/42") + assert json.loads(end.calls.last.request.content) == { + "end_action": {"action_id": 42} + } diff --git a/easyvista_python_client/_sync/tests/test_transport.py b/easyvista_python_client/_sync/tests/test_transport.py index ed5de63..f2e7024 100644 --- a/easyvista_python_client/_sync/tests/test_transport.py +++ b/easyvista_python_client/_sync/tests/test_transport.py @@ -24,7 +24,9 @@ EasyvistaRateLimitError, EasyvistaServerError, EasyvistaValidationError, + EasyvistaWorkflowEffectRefused, ) +from easyvista_python_client.workflow import WorkflowEffect ROOT = "https://ev.test/api/v1/acme" @@ -945,3 +947,123 @@ def test_request_spec_headers_override_the_client_level_ones(): def test_request_spec_refuses_the_credential_in_its_headers(): with pytest.raises(ValueError, match="must not set"): RequestSpec("GET", "requests", headers={"authorization": "Bearer other"}) + + +# --- the workflow guard ------------------------------------------------------ + + +def test_request_spec_normalises_and_validates_the_allow_set(): + spec = RequestSpec("PUT", "x", allow_workflow_effect=WorkflowEffect.ADVANCES) + assert spec.allow_workflow_effect == {WorkflowEffect.ADVANCES} + with pytest.raises(TypeError): + RequestSpec("PUT", "x", allow_workflow_effect=WorkflowEffect) + widened = ( + RequestSpec("PUT", "x") + .allowing(WorkflowEffect.INTERRUPTS) + .allowing([WorkflowEffect.UNKNOWN]) + ) + assert widened.allow_workflow_effect == { + WorkflowEffect.INTERRUPTS, + WorkflowEffect.UNKNOWN, + } + assert RequestSpec("PUT", "x") == RequestSpec("PUT", "x", allow_workflow_effect=()) + + +@respx.mock +def test_send_refuses_a_workflow_write_before_any_request(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with Transport(_cfg()) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + transport.send(RequestSpec("PUT", "requests/I1", json={"closed": {}})) + assert not route.called + assert refused.value.effects == {WorkflowEffect.INTERRUPTS} + assert "allow_workflow_effect=WorkflowEffect.INTERRUPTS" in str(refused.value) + + +@respx.mock +def test_send_refuses_an_effect_that_was_not_the_one_allowed(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + spec = RequestSpec("PUT", "requests/I1", json={"closed": {}, "status_id": 8}) + with Transport(_cfg()) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + transport.send(spec.allowing(WorkflowEffect.INTERRUPTS)) + assert not route.called + assert refused.value.effects == {WorkflowEffect.UNKNOWN} + + +@respx.mock +def test_send_sends_an_allowed_workflow_write_exactly_once_on_a_5xx(): + route = respx.put(f"{ROOT}/requests/I1").mock(return_value=httpx.Response(503)) + spec = RequestSpec("PUT", "requests/I1", json={"closed": {}}).allowing( + WorkflowEffect.INTERRUPTS + ) + with Transport(_cfg(max_retries=3)) as transport: + with pytest.raises(EasyvistaServerError): + transport.send(spec) + assert route.call_count == 1 + + +@respx.mock +def test_send_sends_an_allowed_workflow_write_exactly_once_on_a_transport_error(): + route = respx.put(f"{ROOT}/actions/I1").mock(side_effect=httpx.ConnectError("boom")) + spec = RequestSpec( + "PUT", "actions/I1", json={"end_action": {"action_id": 1}} + ).allowing(WorkflowEffect.ADVANCES) + with Transport(_cfg(max_retries=3)) as transport: + with pytest.raises(EasyvistaConnectionError): + transport.send(spec) + assert route.call_count == 1 + + +@respx.mock +def test_send_still_retries_an_ordinary_write(): + route = respx.put(f"{ROOT}/requests/I1").mock( + side_effect=[httpx.Response(503), httpx.Response(200, json={})] + ) + with Transport(_cfg(max_retries=2)) as transport: + transport.send(RequestSpec("PUT", "requests/I1", json={"title": "t"})) + assert route.call_count == 2 + + +@respx.mock +def test_send_refuses_a_dot_segment_path_before_any_request(): + route = respx.route().mock(return_value=httpx.Response(200, json={})) + with Transport(_cfg()) as transport: + with pytest.raises(ValueError, match="dot segment"): + transport.send( + RequestSpec("PUT", "x/../requests/I1", json={"closed": {}}) + ) + assert not route.called + + +@respx.mock +def test_send_refuses_a_write_hidden_behind_a_configured_override_header(): + # config.extra_headers is merged onto every request on the wire, so a + # method-override header set there can turn a GET into a write exactly as + # one on the spec can. + route = respx.route().mock(return_value=httpx.Response(200, json={})) + config = _cfg(extra_headers={"X-HTTP-Method-Override": "PUT"}) + with Transport(config) as transport: + with pytest.raises(EasyvistaWorkflowEffectRefused) as refused: + transport.send(RequestSpec("GET", "requests/I1", json={"closed": {}})) + assert not route.called + assert refused.value.effects == {WorkflowEffect.INTERRUPTS} + + +def test_gate_reads_the_headers_as_they_go_on_the_wire_the_spec_winning(): + configured = BaseTransport(_cfg(extra_headers={"X-HTTP-Method-Override": "PUT"})) + spec = RequestSpec("GET", "requests/I1", json={"closed": {}}) + assert configured.gate(spec.allowing(WorkflowEffect.INTERRUPTS)) == { + WorkflowEffect.INTERRUPTS + } + # The same header on the spec replaces the configured one, so a read stays one. + spec_reads = RequestSpec( + "GET", + "requests/I1", + json={"closed": {}}, + headers={"X-HTTP-Method-Override": "GET"}, + ) + assert configured.gate(spec_reads) == frozenset() + # And a configured header that is not an override changes nothing. + other = BaseTransport(_cfg(extra_headers={"X-Api-Key": "PUT"})) + assert other.gate(spec) == frozenset() diff --git a/easyvista_python_client/_transport.py b/easyvista_python_client/_transport.py index 7dee8fa..262b382 100644 --- a/easyvista_python_client/_transport.py +++ b/easyvista_python_client/_transport.py @@ -15,11 +15,12 @@ from __future__ import annotations -from collections.abc import Mapping -from dataclasses import dataclass +from collections.abc import Iterable, Mapping +from dataclasses import dataclass, replace from typing import Any from .config import reject_authorization +from .workflow import WorkflowEffect, as_effects @dataclass(frozen=True) @@ -36,6 +37,12 @@ class RequestSpec: ``json`` is typed ``Any`` rather than ``dict``: some routes this package does not wrap take a bare list body, and ``httpx`` accepts anything JSON-serialisable. + + ``allow_workflow_effect`` is the set of workflow effects this request may + carry -- see :mod:`easyvista_python_client.workflow`. The transport refuses + a request that names any effect not in it, so a builder that sends a + workflow command on purpose, or a client method passing the caller's + opt-in through, widens it with :meth:`allowing`. """ method: str @@ -43,7 +50,20 @@ class RequestSpec: params: dict[str, Any] | None = None json: Any = None headers: Mapping[str, str] | None = None + allow_workflow_effect: frozenset[WorkflowEffect] = frozenset() def __post_init__(self) -> None: if self.headers is not None: reject_authorization(self.headers, "RequestSpec.headers") + # Normalised here, so a spec given one member or a list compares and + # checks like one given a frozenset, and a bad value fails at + # construction rather than at send time. + object.__setattr__( + self, "allow_workflow_effect", as_effects(self.allow_workflow_effect) + ) + + def allowing(self, allow: WorkflowEffect | Iterable[WorkflowEffect]) -> RequestSpec: + """This spec, additionally allowing ``allow`` (a member or an iterable).""" + return replace( + self, allow_workflow_effect=self.allow_workflow_effect | as_effects(allow) + ) diff --git a/easyvista_python_client/directory.py b/easyvista_python_client/directory.py index 73e8b16..bd0697f 100644 --- a/easyvista_python_client/directory.py +++ b/easyvista_python_client/directory.py @@ -60,7 +60,8 @@ #: #: ``END_DATE_UT`` is included on purpose: a status id is per-instance and says #: nothing portable about openness, while ``END_DATE_UT`` is empty on an open -#: ticket and stamped on a closed one. ``STATUS`` (the nested object) and +#: ticket and stamped once it is resolved or closed (at resolution, not +#: closure: measured 2026-09-02, one instance). ``STATUS`` (the nested object) and #: ``STATUS_ID`` are both requested so ``.reference("STATUS")`` resolves a label #: where the instance returns one and an id where it does not. #: diff --git a/easyvista_python_client/exceptions.py b/easyvista_python_client/exceptions.py index 48eebee..5a367d2 100644 --- a/easyvista_python_client/exceptions.py +++ b/easyvista_python_client/exceptions.py @@ -2,6 +2,11 @@ from __future__ import annotations +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from .workflow import WorkflowEffect + class EasyvistaError(Exception): """Base class for all EasyVista client errors.""" @@ -76,3 +81,37 @@ class is part of the core package, so ``except EasyvistaContentError`` and ``status_code``, ``ev_code``, ``ev_message`` and ``body`` stay ``None``. Mirrors ``glpi_python_client``'s ``GlpiContentError``. """ + + +class EasyvistaWorkflowEffectRefused(ValueError): + """A write that may change a ticket's workflow was refused before it was sent. + + The refused write is never sent. The transport raises this before it sends + a request that names a :class:`~easyvista_python_client.WorkflowEffect` the + call did not allow through ``allow_workflow_effect=``; and ``end_action`` + raises it before its end request, when the action it was asked to end is a + workflow step or cannot be shown not to be. On the ``end_action`` path one + read of that action may precede the refusal -- it is how the action was + judged -- but the end request is never made. + + **Deliberately not an** :class:`EasyvistaError`. The refused write was + never sent, so there is no status code from it and nothing transient: the + same call can never succeed on a retry, and a caller that treats a + status-code-less ``EasyvistaError`` as "try again later" would retry it for + ever. It subclasses ``ValueError`` because it refuses the arguments, as + this package's other local refusals do. + + ``effects`` holds the refused effects; ``triggers`` the ``(what, effect)`` + pairs that named them -- a body key, a column, or a route. + """ + + def __init__( + self, + message: str, + *, + effects: frozenset[WorkflowEffect] = frozenset(), + triggers: tuple[tuple[str, WorkflowEffect], ...] = (), + ) -> None: + super().__init__(message) + self.effects = effects + self.triggers = triggers diff --git a/easyvista_python_client/models/common.py b/easyvista_python_client/models/common.py index 78ecbb3..ae8aa8d 100644 --- a/easyvista_python_client/models/common.py +++ b/easyvista_python_client/models/common.py @@ -291,7 +291,9 @@ def _point_unknown_keys_at_extra_payload(cls, data: Any) -> Any: "as extra_payload={...}: it merges last and reaches the wire as " "written. Some fields are absent here because they were measured " "to misbehave, not merely because they are undocumented, so " - "re-read afterwards -- a 200 is not a receipt on this API." + "re-read afterwards -- a 200 is not a receipt on this API. A key " + "that may change a ticket's workflow is refused before sending " + "unless the call passes allow_workflow_effect=." ) custom_fields: dict[str, Any] = Field(default_factory=dict) @@ -310,7 +312,9 @@ def to_api(self) -> dict[str, Any]: measured against a single instance, and a deployment where that field behaves differently needs a way through that is not a fork. Because it bypasses the model it also bypasses the model's validation: whatever is - put here reaches the wire as written. + put here reaches the wire as written -- unless it may change a ticket's + workflow, in which case the transport refuses the request before + sending it; see :mod:`easyvista_python_client.workflow`. **The merge is case-insensitive.** The vendor documents the ticket create body's field names as case-insensitive (tier 1 -- diff --git a/easyvista_python_client/models/request.py b/easyvista_python_client/models/request.py index 05edf4b..9d20cfa 100644 --- a/easyvista_python_client/models/request.py +++ b/easyvista_python_client/models/request.py @@ -230,16 +230,21 @@ class PostRequest(EasyvistaWriteModel): you can read again, follow the create with ``update_ticket(rfc, RequestUpdate(description=...))``. - ``workflow_start`` is a boolean and is sent even when ``False``, so a - caller disabling the workflow is not silently overridden -- that part is - real and unchanged. Its provenance is not like the fields above, though: - it does **not** appear anywhere in the vendor's own create-body - documentation. It is declared only in the instance's own OpenAPI schema - for ``POST /requests`` -- "Optional. If true, starts the workflow for the - created incident." -- which makes it **tier 3, illustrative only**: that - schema is example-derived, not a normative contract (see - ``docs/vendor-api-reference.md``). Treat it as unverified until tested - against the deployment you use it on. + ``workflow_start`` is a boolean and is sent as given, ``False`` included. + Its provenance is not like the fields above: it does **not** appear + anywhere in the vendor's own create-body documentation. It is declared only + in the instance's own OpenAPI schema for ``POST /requests`` -- "Optional. + If true, starts the workflow for the created incident." -- which makes it + **tier 3, illustrative only**: that schema is example-derived, not a + normative contract (see ``docs/vendor-api-reference.md``). + + Measured a no-op (tier 4, 2026-09-01, one instance: two tickets identical + but for this flag came back byte-identical), so ``workflow_start=False`` is + not a way to create a ticket without its workflow. The vendor create page + documents no such parameter and states the workflow is started. A + workflow-less create is the separate virtual-agent route, + ``POST requests/without-workflow``, which the workflow guard refuses unless + allowed (see :mod:`easyvista_python_client.workflow`). """ catalog_guid: str | None = None @@ -356,11 +361,14 @@ class RequestUpdate(EasyvistaWriteModel): ``extra="forbid"`` now makes ``RequestUpdate(status_id=...)`` raise at construction instead. - Set a status with :meth:`~easyvista_python_client.EasyvistaClient.set_status`, - which sends the documented ``{"closed": {"status_GUID": ...}}`` body. That - route reaches **every** status, not just terminal ones -- all six statuses - tried landed on exactly the one requested. It is addressed by - ``STATUS_GUID``, not by ``STATUS_ID``. + The vendor documents no status write that leaves the workflow alone, and + this package has none: a ticket's status follows its workflow, and the one + request the vendor documents that sets a status, + :meth:`~easyvista_python_client.EasyvistaClient.close_ticket`, interrupts + it (tier 1: "Advancing through the steps of a workflow changes the + status of a ticket." on + https://docs.easyvista.com/docs/references-tables.md, Statuses section, + and the vendor close page). * ``severity_id`` -- rejected with HTTP 590 (code 2013). Tier 4: measured on one instance, 2026-08-17. * ``urgency_id`` -- ``URGENCY_ID`` raised HTTP 590 *and the value still @@ -383,7 +391,12 @@ class RequestUpdate(EasyvistaWriteModel): To send ``status_id``, ``severity_id`` or ``urgency_id`` anyway on a deployment where they work, use ``extra_payload`` -- and re-read the - ticket afterwards, because a 200 from this endpoint is not a receipt. + ticket afterwards, because a 200 from this endpoint is not a receipt. A + ``status_id`` sent that way is refused before sending unless the call + passes ``allow_workflow_effect=WorkflowEffect.UNKNOWN`` to + ``update_ticket``, because a status column holds workflow state (see + :mod:`easyvista_python_client.workflow`); ``severity_id`` and ``urgency_id`` + are not refused. ``extra_payload`` does **not** help with priority: there is no writable column for it to reach. diff --git a/easyvista_python_client/models/tests/test_common.py b/easyvista_python_client/models/tests/test_common.py index c3754a0..f22ade2 100644 --- a/easyvista_python_client/models/tests/test_common.py +++ b/easyvista_python_client/models/tests/test_common.py @@ -230,6 +230,7 @@ def test_an_unknown_field_names_itself_and_extra_payload(): assert "ctalog_guid" in message assert "extra_payload" in message assert "a 200 is not a receipt on this API." in message + assert "allow_workflow_effect=" in message def test_a_known_field_is_not_intercepted(): diff --git a/easyvista_python_client/models/tests/test_request.py b/easyvista_python_client/models/tests/test_request.py index ecebadc..e8f873a 100644 --- a/easyvista_python_client/models/tests/test_request.py +++ b/easyvista_python_client/models/tests/test_request.py @@ -327,8 +327,9 @@ def test_request_update_refuses_status_id(): invisible: measured live, a flat status write is rejected 590 when sent alone and -- far worse -- returns 200, applies its companion field and drops the status silently when sent beside one. Anything that reinstates this field - reinstates a write that reports success and stores nothing. The status route - is ``set_status`` / the ``{"closed": {"status_GUID": ...}}`` envelope. + reinstates a write that reports success and stores nothing. The only request + that sets a status is ``close_ticket`` (the ``{"closed": {"status_GUID": + ...}}`` envelope), and it interrupts the workflow. """ with pytest.raises(ValidationError) as excinfo: RequestUpdate(status_id=2) diff --git a/easyvista_python_client/resources/actions.py b/easyvista_python_client/resources/actions.py index 19a1901..16d2053 100644 --- a/easyvista_python_client/resources/actions.py +++ b/easyvista_python_client/resources/actions.py @@ -22,6 +22,30 @@ ) +def _require_action_id(action_id: object) -> str: + """Return ``action_id`` as the digit string ``actions/{id}`` addresses. + + ``PUT actions/{rfc_number}`` is the vendor's END-ACTION route, on the same + path template as ``PUT actions/{action_id}`` (tier 1, + https://docs.easyvista.com/docs/webservice-rest.md). An RFC number here + would therefore not edit one action: it addresses the end-action route + instead, where an ``end_action`` body naming no ``action_id`` ends every + open action on the ticket. ``Action.action_id`` is legitimately + ``None`` across this package (a create response carries none; a projection + without ``ACTION_ID`` drops it), so ``None`` is refused rather than + addressing ``actions/None``. + """ + if action_id is None or isinstance(action_id, (bool, float)): + raise ValueError(f"an action id must be a positive integer, got {action_id!r}") + text = str(action_id).strip() + if not (text.isascii() and text.isdigit()) or int(text) <= 0: + raise ValueError( + f"an action id must be a positive integer, got {action_id!r}; an RFC " + "number addresses the end-action route on the same path instead" + ) + return text + + def build_create_action( rfc_number: str, payload: PostAction, @@ -140,6 +164,7 @@ def parse(data: Any) -> list[Action]: def build_get_action( action_id: str | int, *, + fields: Iterable[str] | str | None = None, context: dict[str, Any] | None = None, ) -> tuple[RequestSpec, Callable[[Any], Action]]: """Fetch ONE action by id. @@ -152,8 +177,10 @@ def build_get_action( no ``requests/{rfc}/actions/{id}`` route at all. See :func:`build_search_actions` for why the HTTP 403 an earlier note recorded against that path was never evidence of a permission restriction. + + ``fields`` projects the item read, as on the list. """ - return build_get(ACTIONS, action_id, context=context) + return build_get(ACTIONS, action_id, fields=fields, context=context) def build_update_action( @@ -169,8 +196,63 @@ def build_update_action( ``requests/{rfc}/actions/{id}`` route to send them to. See :func:`build_search_actions` for why the HTTP 403 an earlier note recorded against that path did not distinguish a denied route from an absent one. + + The id must be a positive integer -- see ``_require_action_id`` for why an + RFC number is refused. + """ + record_id = _require_action_id(action_id) + return build_update(ACTIONS, record_id, payload, context=context) + + +#: The body keys a reassignment sends. ``group_id`` is the lower-case spelling +#: the 2026-10-02 census found stored (one instance, 2/2 tickets), so the +#: upper-case retry was never needed. ``done_by_id`` is the same lower-case +#: convention as ``PostAction``, but a write to a person was **not measured**. +_REASSIGN_GROUP_KEY = "group_id" +_REASSIGN_DONE_BY_KEY = "done_by_id" + + +def _positive_int(value: object, name: str) -> int: + if isinstance(value, bool) or not isinstance(value, int) or value <= 0: + raise ValueError(f"{name} must be a positive integer, got {value!r}") + return value + + +def build_reassign_action( + action_id: str | int, + *, + group_id: int | None = None, + done_by_id: int | None = None, + context: dict[str, Any] | None = None, +) -> tuple[RequestSpec, Callable[[Any], Action]]: + """Build ``PUT actions/{id}`` reassigning an action to a group and/or person. + + The vendor documents no reassignment route: the UI's transfer is a wizard, + and ``PUT actions/{id}`` accepts "all the fields from the AM_ACTION table + except" a list that does not name the group or done-by columns (tier 1, + https://docs.easyvista.com/docs/rest-api-update-an-action.md). What this + write does was measured, not documented -- see the client's + ``reassign_action``. + + The id must be a positive integer, as for :func:`build_update_action`; + ``group_id`` and ``done_by_id`` must be positive integers, and at least one + is required. """ - return build_update(ACTIONS, action_id, payload, context=context) + path_id = _require_action_id(action_id) + body: dict[str, int] = {} + if group_id is not None: + body[_REASSIGN_GROUP_KEY] = _positive_int(group_id, "group_id") + if done_by_id is not None: + body[_REASSIGN_DONE_BY_KEY] = _positive_int(done_by_id, "done_by_id") + if not body: + raise ValueError("reassign_action needs group_id, done_by_id, or both") + spec = RequestSpec("PUT", f"actions/{path_id}", json=body) + + def parse(data: Any) -> Action: + records = extract_records(data, ACTIONS.envelope_key) + return Action.model_validate(records[0] if records else data, context=context) + + return spec, parse def build_end_action( @@ -216,6 +298,10 @@ def build_end_action( End Date" (measured 2026-09-01 on one instance -- one instance, one date, so it may not generalise). ``elapsed_time`` is a number of **minutes**. + ``action_id`` must be a positive integer, as for ``update_action``: it is + sent as an integer, and a blank, an RFC number or any other value is + refused rather than named in the body. + A blank ``rfc_number`` is refused rather than allowed to build ``PUT actions/``, which addresses the collection instead of a ticket. """ @@ -244,7 +330,10 @@ def build_end_action( ) end: dict[str, Any] = {} if action_id is not None: - end["action_id"] = action_id + # An integer on the wire, as ``update_action`` addresses one: a blank + # or an RFC number is no action id, and would otherwise let the + # client's pre-flight read address the collection (``GET actions/``). + end["action_id"] = int(_require_action_id(action_id)) if start_date is not None: end["start_date"] = start_date if end_date is not None: diff --git a/easyvista_python_client/resources/requests.py b/easyvista_python_client/resources/requests.py index 992d708..7440c09 100644 --- a/easyvista_python_client/resources/requests.py +++ b/easyvista_python_client/resources/requests.py @@ -83,23 +83,22 @@ def build_close_ticket( catalog_guid: str | None = None, context: dict[str, Any] | None = None, ) -> tuple[RequestSpec, Callable[[Any], Request]]: - """Build the ``{"closed": {...}}`` PUT spec — the API's status-set route. - - Despite the wire name, this envelope is **not limited to closing**. It is the - only working way to set a ticket's status, and it reaches every status: - handed each of six different ``STATUS_GUID``s in turn, a fresh ticket landed - on exactly the status requested every time -- including non-terminal ones - like "A prendre en compte" and "En cours". Nothing was forced to the closed - status. - - Note the addressing: ``status_GUID``, not ``STATUS_ID``. There is no flat - status update on this API -- see :class:`RequestUpdate` for what happens if - you try one. :func:`build_set_status` is the same spec under a name that says - what it does. - - ``delete_actions`` drops the ticket's actions; the vendor types it a - **boolean** and this builder passes either spelling through unchanged, since - EasyVista accepts ``true``/``false``, ``0``/``1`` and the quoted strings. + """Build the ``{"closed": {...}}`` PUT spec — the vendor's close request. + + This is the vendor's CLOSE request, and it is not a status setter. Per the + close page (tier 1, re-read 2026-10-02) it interrupts the ticket's + workflow, sets ``status_GUID`` as "the final status of the ticket", ends + or (with ``delete_actions``) deletes the unfinished actions, and inserts + an anticipated closing action -- none of it conditional on the status + sent. The client's ``close_ticket`` requires the caller to allow + ``WorkflowEffect.INTERRUPTS``; this builder returns a spec the transport + refuses until that is done. Note the addressing: ``status_GUID``, not + ``STATUS_ID``. + + ``delete_actions`` deletes the ticket's unfinished actions rather than + ending them (tier 1, the same page); the vendor types it a **boolean** and + this builder passes either spelling through unchanged, since EasyVista + accepts ``true``/``false``, ``0``/``1`` and the quoted strings. **The route is the vendor's own.** ``PUT requests/{rfc_number}`` with a ``closed`` wrapper is what the documentation specifies @@ -107,10 +106,9 @@ def build_close_ticket( a workaround for the ``PUT|PATCH requests/{rfc_number}/close`` path that also appears in an instance's OpenAPI. Every field below is tier 1, and every one is **optional**: omitting ``end_date`` stamps now, and omitting - ``status_guid`` simply leaves the key out of the body. **Where the ticket - then lands is not established here** -- the behaviour is not recorded in - ``docs/vendor-api-reference.md`` and no live test exercises the omitted - form (open item O-CLOSE-DEFAULT). + ``status_guid`` leaves the key out of the body, and the vendor documents + the default as the Closed meta-status (tier 1, the same page; never + measured here). ``catalog_guid`` requalifies the ticket as it closes -- the vendor notes it is needed only for that. ``end_date`` takes the instance's own date format, @@ -142,22 +140,3 @@ def parse(data: Any) -> Request: ) return spec, parse - - -def build_set_status( - rfc_number: str, - *, - status_guid: str, - comment: str | None = None, - context: dict[str, Any] | None = None, -) -> tuple[RequestSpec, Callable[[Any], Request]]: - """Build a spec that sets ``rfc_number``'s status to ``status_guid``. - - The same request :func:`build_close_ticket` builds, named for what it - actually does. ``status_guid`` is required here rather than optional: the - envelope without one is a close request with nothing to close to, and making - that unexpressible is the point of having this function at all. - """ - return build_close_ticket( - rfc_number, status_guid=status_guid, comment=comment, context=context - ) diff --git a/easyvista_python_client/resources/tests/test_actions.py b/easyvista_python_client/resources/tests/test_actions.py index 942352d..fd5a755 100644 --- a/easyvista_python_client/resources/tests/test_actions.py +++ b/easyvista_python_client/resources/tests/test_actions.py @@ -261,10 +261,36 @@ def test_build_end_action_sends_a_falsy_elapsed_time(value): assert spec.json["end_action"]["elapsed_time"] == value -def test_build_end_action_sends_a_falsy_action_id(): - """``action_id=0`` must address action 0, not become the bulk form.""" - spec, _ = a.build_end_action("I1", action_id=0) - assert spec.json["end_action"]["action_id"] == 0 +def test_build_end_action_never_turns_a_falsy_action_id_into_the_bulk_form(): + """``action_id=0`` is refused, not read as "no id" and not sent. + + Truthiness must not decide between naming an action and the id-less bulk + form (``is not None`` does); and 0 is not a valid action id at all, so it + is refused with the rest of the non-positive ids. + """ + with pytest.raises(ValueError, match="action id"): + a.build_end_action("I1", action_id=0) + + +@pytest.mark.parametrize("good", [42, "42", " 42 "]) +def test_build_end_action_puts_the_action_id_in_the_body_as_an_integer(good): + spec, _ = a.build_end_action("I1", action_id=good) + assert spec.json == {"end_action": {"action_id": 42}} + assert type(spec.json["end_action"]["action_id"]) is int + + +@pytest.mark.parametrize( + "bad", ["I250101_00001", "", " ", 0, -1, "-1", True, "12a", "²", 1.5] +) +def test_build_end_action_refuses_anything_but_a_positive_action_id(bad): + """The same rule as ``update_action``: an RFC number or a blank is no action id. + + A blank would also make the client's pre-flight read address the + collection (``GET actions/``), whose first row says nothing about the + action being ended. + """ + with pytest.raises(ValueError, match="action id"): + a.build_end_action("I1", action_id=bad) def test_build_end_action_passes_start_date_through(): @@ -274,7 +300,7 @@ def test_build_end_action_passes_start_date_through(): end_date="01/09/2026 17:15:00", elapsed_time="15", doneby_mail="a@b.c", ) assert spec.json["end_action"] == { - "action_id": "7", + "action_id": 7, "start_date": "01/09/2026 17:00:00", "end_date": "01/09/2026 17:15:00", "elapsed_time": "15", @@ -298,3 +324,74 @@ def test_build_end_action_parses_the_href_only_response_without_raising(): _, parser = a.build_end_action("I1", action_id=1) parsed = parser({"HREF": "https://host/api/v1/50004/requests/I1"}) assert parsed.action_id is None + + +@pytest.mark.parametrize( + "bad", ["I250101_00001", "", " ", 0, -1, "-1", True, None, "12a", "²", 1.5] +) +def test_build_update_action_refuses_anything_but_a_positive_action_id(bad): + """``PUT actions/{rfc_number}`` is the end-action route on the same template. + + An RFC number where an action id belongs would not edit one action: it + would address the end-action route, where an ``end_action`` body naming no + ``action_id`` ends every open action on the ticket. + """ + with pytest.raises(ValueError, match="action id"): + build_update_action(bad, ActionUpdate(description="x")) + + +@pytest.mark.parametrize("good", [60350, "60350", " 60350 "]) +def test_build_update_action_addresses_the_digit_string(good): + spec, _ = build_update_action(good, ActionUpdate(description="x")) + assert spec.path == "actions/60350" + + +def test_build_get_action_can_project_fields(): + spec, _ = build_get_action(42, fields=["ACTION_ID", "WORKFLOW_ID"]) + assert spec.path == "actions/42" + assert spec.params == {"fields": "ACTION_ID,WORKFLOW_ID"} + + +def test_build_reassign_action_puts_the_group_and_person(): + spec, _ = a.build_reassign_action(60350, group_id=57, done_by_id=12) + assert (spec.method, spec.path) == ("PUT", "actions/60350") + assert spec.json == {a._REASSIGN_GROUP_KEY: 57, a._REASSIGN_DONE_BY_KEY: 12} + + +def test_build_reassign_action_sends_only_what_it_was_given(): + spec, _ = a.build_reassign_action(60350, group_id=57) + assert spec.json == {a._REASSIGN_GROUP_KEY: 57} + spec, _ = a.build_reassign_action(60350, done_by_id=12) + assert spec.json == {a._REASSIGN_DONE_BY_KEY: 12} + + +def test_build_reassign_action_needs_a_target(): + with pytest.raises(ValueError, match="group_id, done_by_id"): + a.build_reassign_action(60350) + + +@pytest.mark.parametrize("bad", [0, -3, True, "57", 1.0]) +def test_build_reassign_action_refuses_a_non_positive_or_non_int_id(bad): + with pytest.raises(ValueError): + a.build_reassign_action(60350, group_id=bad) + with pytest.raises(ValueError): + a.build_reassign_action(60350, done_by_id=bad) + + +def test_build_reassign_action_refuses_an_rfc_as_the_action(): + with pytest.raises(ValueError, match="action id"): + a.build_reassign_action("I250101_00001", group_id=57) + + +def test_build_reassign_action_parses_an_empty_echo_without_raising(): + _, parser = a.build_reassign_action(60350, group_id=57) + assert parser({}).action_id is None + assert parser({"records": [{"ACTION_ID": 60350, "GROUP_ID": 57}]}).group_id == 57 + + +def test_build_reassign_action_body_is_not_a_workflow_effect(): + """The group and person are data the workflow reads, not workflow state.""" + from easyvista_python_client.workflow import workflow_triggers + + spec, _ = a.build_reassign_action(60350, group_id=57, done_by_id=12) + assert not workflow_triggers(spec.method, spec.path, spec.json) diff --git a/easyvista_python_client/resources/tests/test_requests.py b/easyvista_python_client/resources/tests/test_requests.py index c0a5cb7..a38bd3d 100644 --- a/easyvista_python_client/resources/tests/test_requests.py +++ b/easyvista_python_client/resources/tests/test_requests.py @@ -110,25 +110,8 @@ def test_build_close_ticket_full_documented_shape(): } -def test_build_set_status_sends_the_closed_envelope(): - """``set_status`` is the ``closed`` envelope, addressed by GUID. - - Pins both halves of the call shape that took several wrong turns to find: the - body is wrapped in ``closed`` (not flat), and the key is ``status_GUID`` (not - ``STATUS_ID``). The envelope is not limited to closing -- six different status - GUIDs each landed on exactly the status requested. - """ - spec, _parser = r.build_set_status("I1", status_guid="{G}", comment="c") - assert spec.method == "PUT" - assert spec.path == "requests/I1" - assert spec.json == {"closed": {"status_GUID": "{G}", "comment": "c"}} - - -def test_build_set_status_matches_build_close_ticket(): - """The two builders are the same request; only the name differs.""" - a, _ = r.build_set_status("I1", status_guid="{G}") - b, _ = r.build_close_ticket("I1", status_guid="{G}") - assert (a.method, a.path, a.json) == (b.method, b.path, b.json) +def test_build_set_status_is_gone(): + assert not hasattr(r, "build_set_status") def test_close_ticket_carries_the_two_previously_undeclared_documented_fields(): diff --git a/easyvista_python_client/testing/test_method_invocation.py b/easyvista_python_client/testing/test_method_invocation.py index a68ac5b..00768c3 100644 --- a/easyvista_python_client/testing/test_method_invocation.py +++ b/easyvista_python_client/testing/test_method_invocation.py @@ -36,6 +36,7 @@ PostRequest, PostTask, RequestUpdate, + WorkflowEffect, ) #: One payload that satisfies every parser in the package. @@ -52,6 +53,10 @@ { "RFC_NUMBER": "I1", "ACTION_ID": 1, + # Empty, not absent: end_action's guard reads this column off the + # action it is asked to end, and an empty one is the caller's own + # action (the safe path the registry should model). + "WORKFLOW_ID": "", "ASSET_ID": 1, "DEPARTMENT_ID": 1, "EMPLOYEE_ID": 1, @@ -75,8 +80,7 @@ # The escape hatch: an arbitrary route, parsed by nobody. PAYLOAD satisfies # it because `send` returns the raw JSON body unchanged. "send": (("GET", "requests"), {}), - "close_ticket": (("I1",), {}), - "set_status": (("I1",), {"status_guid": "{0000-0000}"}), + "close_ticket": (("I1",), {"allow_workflow_effect": WorkflowEffect.INTERRUPTS}), "count_tickets": ((), {}), "create_action": (("I1", PostAction(action_type_id=94, group_id=3)), {}), # action_id named explicitly: omitted, the vendor form ends EVERY open @@ -114,6 +118,7 @@ "iter_tickets": ((), {"max_records": 1}), "list_actions": (("I1",), {}), "list_documents": (("I1",), {}), + "reassign_action": ((1,), {"group_id": 3}), "resolve_memo": (("requests/I1/description",), {}), "search_assets": ((), {}), "search_departments": ((), {}), diff --git a/easyvista_python_client/tests/test_workflow.py b/easyvista_python_client/tests/test_workflow.py new file mode 100644 index 0000000..2d00230 --- /dev/null +++ b/easyvista_python_client/tests/test_workflow.py @@ -0,0 +1,269 @@ +"""The workflow-effect classifier: what a request may do to a ticket's workflow.""" + +import pytest + +from easyvista_python_client import EasyvistaError, EasyvistaWorkflowEffectRefused +from easyvista_python_client.workflow import ( + WorkflowEffect, + as_effects, + classify_workflow_effects, + workflow_triggers, +) + +I, A, U = ( # noqa: E741 -- short aliases keep the parametrised table readable + WorkflowEffect.INTERRUPTS, + WorkflowEffect.ADVANCES, + WorkflowEffect.UNKNOWN, +) + + +@pytest.mark.parametrize( + ("method", "path", "body", "expected"), + [ + # The vendor's workflow-control bodies, on the routes they belong to. + ("PUT", "requests/I1", {"closed": {"status_GUID": "{G}"}}, {I}), + ("PUT", "actions/I1", {"end_action": {"action_id": 1}}, {A}), + ("PUT", "requests/I1", {"suspended": {}}, {U}), + ("PUT", "requests/I1", {"restarted": {}}, {U}), + # ... matched case-insensitively, on any path, and inside a list body. + ("PUT", "requests/I1", {"Closed": {}}, {I}), + ("PUT", "actions/60350", {"END_ACTION": {}}, {A}), + ("PUT", "departments/7", {"closed": {}}, {I}), + ("PUT", "requests/I1", [{"closed": {}}], {I}), + # httpx serialises a tuple as a JSON array, so it is scanned like a list. + ("PUT", "requests/I1", ({"closed": {}},), {I}), + # Ticket columns that hold or select workflow state. + ("PUT", "requests/I1", {"STATUS_ID": 12}, {U}), + ("PUT", "requests/I1", {"status_guid": "{G}"}, {U}), + ("PUT", "requests/I1", {"SD_CATALOG_ID": 3}, {U}), + ("PUT", "requests/I1", {"initial_sd_catalog_id": 3}, {U}), + ("PUT", "requests/I1", {"catalog_guid": "{C}"}, {U}), + ("PUT", "requests/I1", {"catalog_code": "X"}, {U}), + ("PUT", "requests/I1", {"parent_request_id": 9}, {U}), + # Action columns that end, re-type, re-parent or move an action. + ("PUT", "actions/60350", {"END_DATE_UT": "01/01/2026 10:00:00"}, {U}), + ("PUT", "actions/60350", {"workflow_id": 1}, {U}), + ("PUT", "actions/60350", {"ACTION_TYPE_ID": 20}, {U}), + ("PUT", "actions/60350", {"parent_action_id": 1}, {U}), + ("PUT", "actions/60350", {"request_id": 5}, {U}), + # Create routes: an action born ended, a task tied to a step. + ( + "POST", + "requests/I1/actions", + {"action_type_id": 94, "end_date_ut": "x"}, + {U}, + ), + ( + "POST", + "requests/I1/tasks", + {"action_type_id": 94, "parent_action_id": 1}, + {U}, + ), + # Workflow routes. + ("PUT", "requests/I1/close", {}, {I}), + ("PATCH", "requests/I1/suspend", {}, {U}), + ("PUT", "requests/I1/restart", {}, {U}), + ("PUT", "requests/I1/workflowstart", None, {U}), + ("DELETE", "requests/I1", None, {U}), + ("POST", "requests/without-workflow", {"requests": [{}]}, {U}), + # Two effects at once. + ("PUT", "requests/I1", {"closed": {}, "status_id": 8}, {I, U}), + # PUT actions/{rfc_number} is the vendor's end-action route, so a write + # to an action path that is not an integer id is named whatever it sends. + ("PUT", "actions/S1", {"description": "d"}, {A}), + ("PUT", "actions/S261002_00002", {"description": "d"}, {A}), + ("PUT", "actions/S1", {"workflow_id": 1}, {A, U}), + ("DELETE", "actions/S1", None, {A}), + # An ASCII-digit id is an action id; a non-ASCII digit is not one. + ("PUT", "actions/٣", {"description": "d"}, {A}), + ], +) +def test_names_what_a_write_may_do_to_the_workflow(method, path, body, expected): + assert classify_workflow_effects(method, path, body) == frozenset(expected) + + +@pytest.mark.parametrize( + ("method", "path", "body"), + [ + # What the package's typed writes send, and what itsm_synchronisation sends. + ("POST", "requests", {"requests": [{"catalog_code": "C", "title": "t"}]}), + ( + "PUT", + "requests/I1", + { + "title": "t", + "description": "d", + "impact_id": 3, + "owner_id": 7, + "external_reference": "m", + }, + ), + ("PUT", "actions/60350", {"description": "edited"}), + # An integer id is an action, not the end-action route's RFC number. + ("PUT", "actions/60350", {"description": "d"}), + # A read names nothing, whatever the path; so does the collection. + ("GET", "actions/S1", None), + ("POST", "actions", {"actions": [{}]}), + # Reassignment is supported, so it is not named (design decision e). + ("PUT", "actions/60350", {"GROUP_ID": 57, "DONE_BY_ID": 12}), + ("PUT", "actions/60350", {"group_id": 57}), + ( + "POST", + "requests/I1/actions", + {"action_type_id": 94, "group_id": 3, "parent_action_id": 1}, + ), + ( + "POST", + "requests/I1/tasks", + {"action_type_id": 94, "group_id": 3, "end_date_ut": "x"}, + ), + ("POST", "requests/I1/documents", None), + ("DELETE", "requests/I1/documents/5", None), + ("POST", "groups", {"groups": [{}]}), + # Reads name nothing, whatever the body. + ("GET", "requests/I1", None), + ("HEAD", "requests/I1", None), + ("GET", "requests/I1", {"closed": {}}), + ], +) +def test_names_nothing_for_ordinary_writes_and_reads(method, path, body): + assert classify_workflow_effects(method, path, body) == frozenset() + + +def test_a_method_override_header_is_read_as_the_method(): + effects = classify_workflow_effects( + "GET", "requests/I1", {"closed": {}}, {"X-HTTP-Method-Override": "put"} + ) + assert effects == {I} + + +@pytest.mark.parametrize( + ("method", "path", "body", "headers", "expected"), + [ + # An override that says "read" must not turn a real write into a read. + ("POST", "requests/I1/close", {}, {"X-HTTP-Method-Override": "GET"}, {I}), + ("PUT", "requests/I1", {"closed": {}}, {"X-HTTP-Method": "GET"}, {I}), + ("PUT", "requests/I1", {"closed": {}}, {"x-method-override": " head "}, {I}), + # Two override headers: a read one must not hide a write one, in either order. + ( + "GET", + "requests/I1", + {"closed": {}}, + {"X-HTTP-Method": "GET", "X-HTTP-Method-Override": "PUT"}, + {I}, + ), + ( + "GET", + "requests/I1", + {"closed": {}}, + {"X-HTTP-Method-Override": "PUT", "X-HTTP-Method": "GET"}, + {I}, + ), + # An empty or unrecognised override is not a read. + ("GET", "requests/I1", {"closed": {}}, {"X-HTTP-Method-Override": ""}, {I}), + # DELETE semantics apply if any of the methods is a DELETE. + ("POST", "requests/I1", {}, {"X-HTTP-Method-Override": "DELETE"}, {U}), + ("DELETE", "requests/I1", None, {"X-HTTP-Method-Override": "GET"}, {U}), + # Only when every method is a read is the request a read. + ( + "GET", + "requests/I1", + {"closed": {}}, + {"X-HTTP-Method-Override": "HEAD"}, + set(), + ), + ("GET", "requests/I1", {"closed": {}}, {"Accept": "PUT"}, set()), + ], +) +def test_a_request_is_a_read_only_when_every_method_it_names_is_a_read( + method, path, body, headers, expected +): + effects = classify_workflow_effects(method, path, body, headers) + assert effects == frozenset(expected) + + +def test_query_string_case_and_doubled_slashes_do_not_hide_a_route(): + assert classify_workflow_effects("PUT", "/requests//I1/?x=1", {"status_id": 1}) == { + U + } + assert classify_workflow_effects("PUT", "REQUESTS/I1/CLOSE", {}) == {I} + + +@pytest.mark.parametrize( + "path", + [ + "x/../requests/I1", + "./requests/I1", + "requests/I1/.", + "requests/%2e%2e/I1", + "../50005/requests/I1", + ], +) +def test_a_dot_segment_is_refused_outright(path): + with pytest.raises(ValueError, match="dot segment"): + workflow_triggers("PUT", path, {"title": "t"}) + + +@pytest.mark.parametrize( + "path", + [ + "requests%2FI1%2Fclose", + "requests%2fI1/close", + "requests\\I1\\close", + "requests/I1%5Cclose", + ], +) +def test_an_encoded_slash_or_a_backslash_is_refused_outright(path): + with pytest.raises(ValueError, match="encoded slash or a backslash"): + workflow_triggers("PUT", path, {}) + + +def test_the_end_action_route_is_named_beside_the_body_that_selects_it(): + assert workflow_triggers("PUT", "actions/I1", {"end_action": {"action_id": 1}}) == ( + ("end_action", A), + ("actions/{rfc}", A), + ) + + +def test_triggers_name_the_key_that_matched_envelopes_first(): + assert workflow_triggers("PUT", "requests/I1", {"STATUS_ID": 8, "closed": {}}) == ( + ("closed", I), + ("status_id", U), + ) + + +@pytest.mark.parametrize( + ("allow", "expected"), + [(I, {I}), ((), set()), ([I, A], {I, A}), (frozenset({U}), {U})], +) +def test_as_effects_accepts_a_member_or_an_iterable_of_members(allow, expected): + assert as_effects(allow) == frozenset(expected) + + +@pytest.mark.parametrize( + "allow", + [ + WorkflowEffect, + "interrupts", + b"x", + ["interrupts"], + [I, "advances"], + 1, + None, + {"a": I}, + ], +) +def test_as_effects_refuses_anything_else(allow): + with pytest.raises(TypeError): + as_effects(allow) + + +def test_the_refusal_is_a_value_error_and_not_an_easyvista_error(): + exc = EasyvistaWorkflowEffectRefused( + "no", effects=frozenset({I}), triggers=(("closed", I),) + ) + assert isinstance(exc, ValueError) + assert not isinstance(exc, EasyvistaError) + assert exc.effects == {I} + assert exc.triggers == (("closed", I),) + assert str(exc) == "no" diff --git a/easyvista_python_client/workflow.py b/easyvista_python_client/workflow.py new file mode 100644 index 0000000..b15f398 --- /dev/null +++ b/easyvista_python_client/workflow.py @@ -0,0 +1,337 @@ +"""What a write may do to a ticket's workflow -- decided before it is sent. + +EasyVista drives a ticket's status from its workflow, not from a field: "A +workflow is a process that handles a type of tickets, arranged in a sequence of +actions performed in steps." (https://docs.easyvista.com/docs/workflow.md) and +"Advancing through the steps of a workflow changes the status of a ticket." +(https://docs.easyvista.com/docs/references-tables.md, Statuses section); both +tier 1, read 2026-10-02. Of the REST writes the vendor documents, four touch +the workflow: + +* creating a ticket starts it -- "3. The workflow associated with the ticket + is started." (https://docs.easyvista.com/docs/rest-api-create-an-incident-request.md); +* ``PUT requests/{rfc_number}/workflowstart`` starts the workflow of a ticket + created through the virtual-agent route, which does not start it + (https://docs.easyvista.com/docs/ev-service-manager-rest-api-start-ticket-workflow-via-virtual-agent.md); + this module names it by the sub-route rule below, as ``UNKNOWN``; +* the ``closed`` body on ``PUT requests/{rfc_number}`` interrupts it -- "1. The + workflow of the ticket is interrupted." + (https://docs.easyvista.com/docs/rest-api-close-an-incident-request.md), + whatever status it names; +* the ``end_action`` body on ``PUT actions/{rfc_number}`` ends actions, and + ending a workflow step's action moves the workflow on. The REST page is + silent about the workflow; the support is the UI's Finish wizard ("The + workflow will proceed to the next step." -- + https://docs.easyvista.com/docs/action.md) and one measurement (2026-09-01, + one instance, 2/2, so it may not generalise). + +Everything else is undocumented in workflow terms, which is not the same as +neutral: a per-instance business rule can run "On Insert/On Update" of any +record (https://docs.easyvista.com/docs/business-rule.md). So this module +names what it can, and the transport refuses anything named unless the call +site allowed it explicitly, with ``allow_workflow_effect=``. + +**What gets named.** (1) The vendor's workflow-control bodies -- ``closed``, +``end_action``, ``suspended``, ``restarted`` -- as a top-level body key, in any +casing, on any path. (2) On ticket and action routes, the columns that hold or +select workflow state: a status, a catalog (which selects the workflow), the +workflow, stage and step links, and on an existing action its end date, type, +parent or ticket (creating an action or a task names a narrower set, below). +(3) Ticket sub-routes that are workflow commands rather than records, and +``requests/without-workflow`` and the deletion of a ticket. (4) A write to +``actions/`` where ```` is not an integer id: ``PUT +actions/{rfc_number}`` is the end-action route, so the route is named +(``ADVANCES``) whatever the body says. The column rules in (2) apply to the +``requests/`` and ``actions/`` routes only; a write to any other route family +is not classified by column. Not named: data the workflow merely +reads -- text, owner, group, done-by, impact, urgency. Reassigning an action's +group or person is therefore not refused. + +**This is a deny-list, and a deny-list of columns cannot be complete**: the +vendor's update pages accept "all the fields from the SD_REQUEST table except +those mentioned below" for a ticket +(https://docs.easyvista.com/docs/rest-api-update-an-incident-request.md) and +"all the fields from the AM_ACTION table except those mentioned below" for an +action (https://docs.easyvista.com/docs/rest-api-update-an-action.md), each +followed by a list of exclusions (tier 1, read 2026-10-02). What is not +named here is unclassified, not proven neutral. +""" + +from __future__ import annotations + +import enum +from collections.abc import Iterable, Mapping +from typing import Any +from urllib.parse import unquote + + +class WorkflowEffect(enum.Enum): + """What a write may do to a ticket's workflow. + + ``INTERRUPTS``: the vendor documents the write as stopping the workflow + (the ``closed`` body). ``ADVANCES``: the write ends actions, and ending a + workflow step moves the workflow on (the ``end_action`` body). + ``UNKNOWN``: the write touches workflow state, or a route that does, and + nothing documents or measures what follows. + + Passed to ``allow_workflow_effect=`` as one member or an iterable of + members. Deliberately not a ``str`` enum, so ``"interrupts"`` is refused + rather than matched. + """ + + INTERRUPTS = "interrupts" + ADVANCES = "advances" + UNKNOWN = "unknown" + + +#: The vendor's workflow-control bodies, matched as a top-level key on ANY path. +_ENVELOPES: Mapping[str, WorkflowEffect] = { + "closed": WorkflowEffect.INTERRUPTS, + "end_action": WorkflowEffect.ADVANCES, + "suspended": WorkflowEffect.UNKNOWN, + "restarted": WorkflowEffect.UNKNOWN, +} + +#: Columns on ``requests/{rfc}`` that hold or select workflow state. The vendor +#: excludes ``status_id``, ``sd_catalog_id``, ``initial_sd_catalog_id`` and +#: ``parent_request_id`` from the update body outright +#: (https://docs.easyvista.com/docs/rest-api-update-an-incident-request.md, +#: tier 1, read 2026-10-02). The vendor documents requalifying the ticket's +#: category as starting a new workflow: "Requalify the category of the object. +#: A new workflow will then start." (https://docs.easyvista.com/docs/action.md, +#: tier 1, read 2026-10-02). Reading the catalog columns as that category is +#: this module's inference, not a measurement. +_REQUEST_COLUMNS = frozenset( + { + "status_id", + "status_guid", + "sd_catalog_id", + "initial_sd_catalog_id", + "catalog_guid", + "catalog_code", + "parent_request_id", + } +) + +#: Columns on ``actions/{id}`` that end, re-type, re-parent or move an action. +_ACTION_COLUMNS = frozenset( + { + "end_date_ut", + "end_date", + "status_id_on_terminate", + "workflow_id", + "stage_id", + "process_step_id", + "parent_action_id", + "action_type_id", + "action_type_guid", + "action_type_name", + "request_id", + "rfc_number", + } +) + +#: Columns that would create an action already ended or tied into a step. +_CREATE_ACTION_COLUMNS = frozenset( + { + "end_date_ut", + "end_date", + "status_id_on_terminate", + "workflow_id", + "stage_id", + "process_step_id", + } +) + +#: The same for a task, which is born ended: its end date is ordinary, a +#: parent is not. +_CREATE_TASK_COLUMNS = frozenset( + { + "status_id_on_terminate", + "workflow_id", + "stage_id", + "process_step_id", + "parent_action_id", + } +) + +#: Ticket sub-resources whose writes create or delete records. Any other +#: ``requests/{rfc}/`` write -- ``close``, ``suspend``, ``restart``, +#: ``workflowstart``, or whatever a deployment adds -- is a command and is named. +_RECORD_SUBRESOURCES = frozenset({"actions", "tasks", "documents"}) + +_READ_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) + +_METHOD_OVERRIDE_HEADERS = frozenset( + {"x-http-method-override", "x-http-method", "x-method-override"} +) + + +def as_effects( + allow: WorkflowEffect | Iterable[WorkflowEffect], +) -> frozenset[WorkflowEffect]: + """Normalise an ``allow_workflow_effect=`` argument to a frozenset of members. + + Accepts one :class:`WorkflowEffect` or an iterable of them; ``()`` allows + nothing. Anything else raises ``TypeError``: a string, because + ``"interrupts"`` is not a member and iterating it yields letters; and the + enum **class** itself, which is iterable and would allow every effect from a + one-token typo for ``WorkflowEffect.INTERRUPTS``. + """ + if isinstance(allow, WorkflowEffect): + return frozenset({allow}) + if isinstance(allow, (str, bytes, type, Mapping)) or not isinstance( + allow, Iterable + ): + raise TypeError( + "allow_workflow_effect takes a WorkflowEffect member or an iterable " + f"of members, not {allow!r}" + ) + effects = frozenset(allow) + strays = sorted( + repr(item) for item in effects if not isinstance(item, WorkflowEffect) + ) + if strays: + raise TypeError( + "allow_workflow_effect takes WorkflowEffect members only; got " + + ", ".join(strays) + ) + return effects + + +def _segments(path: str) -> list[str]: + """``path``'s non-empty segments, percent-decoded and case-folded. + + Refuses a ``.`` or ``..`` segment: httpx removes dot segments from the URL + it sends, so ``x/../requests/I1`` reaches ``requests/I1`` -- a route this + check would otherwise not have read. No API route needs one. + + Also refuses a backslash anywhere in the path, and a segment whose + percent-decoded form contains ``/`` or ``\\`` (``requests%2FI1%2Fclose``, + ``requests/I1%5Cclose``): splitting on ``/`` before decoding would read + either as one opaque segment and miss the route, yet a server may read it as + a separator. This fails closed -- whether the server decodes ``%2F`` or + treats ``\\`` as a separator is not measured, and no API route needs either. + """ + bare = path.split("?", 1)[0].split("#", 1)[0] + decoded = [unquote(part) for part in bare.split("/") if part] + if "\\" in bare or any("/" in part or "\\" in part for part in decoded): + raise ValueError( + f"refusing path {path!r}: it contains an encoded slash or a " + "backslash, which a server may read as a path separator, so the " + "request could reach a different route from the one this check read" + ) + segments = [part.casefold() for part in decoded] + if any(part in {".", ".."} for part in segments): + raise ValueError( + f"refusing path {path!r}: it contains a dot segment, which the HTTP " + "client collapses, so the request would reach a different route " + "from the one written" + ) + return segments + + +def _methods(method: str, headers: Mapping[str, str] | None) -> set[str]: + """The real method and every method-override header value, upper-cased. + + A server that honours an override header runs the override, not the real + method, and a request may carry several such headers, so any one of them + could be the one that is honoured. The request is therefore judged by all + of them: it is a read only if every one of them is a read. + """ + methods = {method.strip().upper()} + for name, value in (headers or {}).items(): + if name.casefold() in _METHOD_OVERRIDE_HEADERS: + methods.add(str(value).strip().upper()) + return methods + + +def _body_keys(body: Any) -> set[str]: + # httpx serialises a tuple as a JSON array, so it is scanned like a list. + records = ( + [body] + if isinstance(body, Mapping) + else [item for item in body if isinstance(item, Mapping)] + if isinstance(body, (list, tuple)) + else [] + ) + return {str(key).casefold() for record in records for key in record} + + +def workflow_triggers( + method: str, + path: str, + body: Any = None, + headers: Mapping[str, str] | None = None, +) -> tuple[tuple[str, WorkflowEffect], ...]: + """Every ``(what, effect)`` this request names, envelopes first. + + ``what`` is the case-folded body key, or the route, that matched. Empty for + a read and for an ordinary write. + + A request is a read only when the real ``method`` **and** the value of every + method-override header in ``headers`` (``X-HTTP-Method-Override``, + ``X-HTTP-Method``, ``X-Method-Override``, matched case-insensitively) are + all reads (``GET``, ``HEAD``, ``OPTIONS``). Otherwise it is classified as a + write, so an override that says ``GET`` cannot hide a write, and a ``DELETE`` + among them applies the ``DELETE`` rule. A body given as a mapping, or as a + list or tuple of mappings, is scanned for its top-level keys. + + Raises ``ValueError``, whatever the method, for a path that contains a dot + segment (``.`` or ``..``), a percent-encoded slash or backslash, or a raw + backslash. The first is collapsed by the HTTP client and the others may be + read by a server as a separator, so each could reach a route other than the + one this function read. + """ + segments = _segments(path) + methods = _methods(method, headers) + if methods <= _READ_METHODS: + return () + keys = _body_keys(body) + found: list[tuple[str, WorkflowEffect]] = [ + (key, _ENVELOPES[key]) for key in sorted(keys) if key in _ENVELOPES + ] + + def columns(names: frozenset[str]) -> None: + found.extend((key, WorkflowEffect.UNKNOWN) for key in sorted(keys & names)) + + head = segments[0] if segments else "" + if head == "requests" and len(segments) == 2: + if segments[1] == "without-workflow": + found.append(("requests/without-workflow", WorkflowEffect.UNKNOWN)) + else: + if "DELETE" in methods: + found.append(("DELETE requests/{rfc}", WorkflowEffect.UNKNOWN)) + columns(_REQUEST_COLUMNS) + elif head == "requests" and len(segments) >= 3: + sub = segments[2] + if sub == "actions": + columns(_CREATE_ACTION_COLUMNS) + elif sub == "tasks": + columns(_CREATE_TASK_COLUMNS) + elif sub not in _RECORD_SUBRESOURCES: + effect = ( + WorkflowEffect.INTERRUPTS if sub == "close" else WorkflowEffect.UNKNOWN + ) + found.append((f"requests/{{rfc}}/{sub}", effect)) + elif head == "actions" and len(segments) == 2: + columns(_ACTION_COLUMNS) + # ``PUT actions/{rfc_number}`` is the vendor's end-action route, and the + # same path shape as an action edit; only the segment tells them apart. + # An integer id addresses an action. Anything else is the ticket's RFC + # number, which selects the end-action route whatever the body names. + if not (segments[1].isascii() and segments[1].isdigit()): + found.append(("actions/{rfc}", WorkflowEffect.ADVANCES)) + return tuple(found) + + +def classify_workflow_effects( + method: str, + path: str, + body: Any = None, + headers: Mapping[str, str] | None = None, +) -> frozenset[WorkflowEffect]: + """The set of effects :func:`workflow_triggers` names for this request.""" + return frozenset( + effect for _, effect in workflow_triggers(method, path, body, headers) + ) diff --git a/integration_tests/conftest.py b/integration_tests/conftest.py index 3895a9c..e66297d 100644 --- a/integration_tests/conftest.py +++ b/integration_tests/conftest.py @@ -7,8 +7,8 @@ and no ``EASYVISTA_TEST_*`` environment simply skips the suite rather than failing it. -They are not read-only. A full run creates and closes **21 tickets** (one shared -``rich_ticket``, two ``probe_tickets``, and 18 from ``ticket_factory``), plus 8 +They are not read-only. A full run creates and closes **20 tickets** (one shared +``rich_ticket``, two ``probe_tickets``, and 17 from ``ticket_factory``), plus 8 actions, 5 document uploads and **6 to 14 ticket updates** (4 fixed PUTs -- title, rename, description, external reference -- plus the ``IMPACT_ID`` / ``OWNER_ID`` read-back in the ticket-identity test, which tries up to 5 @@ -16,7 +16,10 @@ of which it may reject outright); ``test_live_smoke`` additionally issues one create the server is *expected to reject*, so no ticket persists from it. Every created ticket is registered for cleanup before it is asserted on, and closed -in teardown. Point them at a preprod/test instance, never production. +in teardown. The opt-in workflow census in ``test_live_workflow_guard.py`` adds +up to 3 tickets and reassigns workflow steps, which may notify the target group +or person; it runs only when ``EASYVISTA_TEST_RUN_WORKFLOW_CENSUS=1``. Point +them at a preprod/test instance, never production. Credentials resolve from an uppercase env var first, then a lowercase file under ``secrets/``: @@ -87,6 +90,7 @@ EasyvistaRateLimitError, EasyvistaServerError, PostRequest, + WorkflowEffect, ev_equals_filter, is_safe_ev_value, ) @@ -304,8 +308,10 @@ def live_write_client(live_config: EasyvistaConfig) -> Iterator[EasyvistaClient] tell a safe GET from a ``create_action``. Rather than weaken the retry that makes reads trustworthy, the non-idempotent verbs get their own client with retries off: ``create_ticket``, ``create_action`` and ``add_document``. - ``update_ticket`` (fixed-value PUTs) and ``close_ticket`` are idempotent and - stay on ``live_client``. + ``update_ticket`` (fixed-value PUTs) stays on ``live_client``; + ``close_ticket`` is not idempotent in effect (each call inserts an + anticipated closing action), but the transport sends an allowed close once + on any client. ``replace`` on a frozen dataclass re-runs ``__post_init__``, which is required because ``_server_normalized`` is ``field(init=False)``. @@ -588,12 +594,15 @@ def _close_tracked( Error records carry the exception's TYPE and status code, never the exception object: ``str(exc)`` is the transport's message, which interpolates server prose this suite did not author (P2). + + Teardown interrupts each ticket's workflow on purpose -- that is what closing is. """ errors: list[tuple[str, str, int | None]] = [] for rfc in tracked: try: client.close_ticket( rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=cfg["status_guid"], delete_actions=1, comment=reason, diff --git a/integration_tests/test_fixture_helpers.py b/integration_tests/test_fixture_helpers.py index 8129657..4a27e58 100644 --- a/integration_tests/test_fixture_helpers.py +++ b/integration_tests/test_fixture_helpers.py @@ -508,7 +508,9 @@ def __init__(self, failing: set[str] | None = None) -> None: self.closed: list[str] = [] self._failing = failing or set() - def close_ticket(self, rfc, *, status_guid, delete_actions, comment): + def close_ticket( + self, rfc, *, allow_workflow_effect, status_guid, delete_actions, comment + ): if rfc in self._failing: raise EasyvistaConnectionError("connection failed") self.closed.append(rfc) diff --git a/integration_tests/test_live_instance_discovery.py b/integration_tests/test_live_instance_discovery.py index 99db717..b07e727 100644 --- a/integration_tests/test_live_instance_discovery.py +++ b/integration_tests/test_live_instance_discovery.py @@ -54,8 +54,8 @@ def test_describe_instance_profiles_the_live_deployment( "no statuses discovered; check profile.unavailable['STATUS'] -- a " "denial and an empty table are different things" ) - # The GUID is the value set_status and close_ticket actually address a - # status by, and it is only ever readable off a sampled ticket. + # The GUID is the value close_ticket actually addresses a status by, and + # it is only ever readable off a sampled ticket. assert any(s.guid for s in statuses), ( "no discovered status carried a STATUS_GUID; the sample reached no " "ticket, or the nested STATUS object stopped carrying one" diff --git a/integration_tests/test_live_smoke.py b/integration_tests/test_live_smoke.py index 09506b4..4970e7b 100644 --- a/integration_tests/test_live_smoke.py +++ b/integration_tests/test_live_smoke.py @@ -4,18 +4,19 @@ env vars or ``secrets/easyvista_test_*`` files. Never runs in CI (which runs ``pytest -m "not integration"``). NEVER point at production. -This module WRITES. It creates up to three tickets and closes every one: +This module WRITES. It creates up to two tickets and closes every one: * one under-specified create the server is expected to reject -- which still creates the row (measured: 9 of 9 rejected creates left one), so it is reconciled by its ``external_reference`` marker and closed. An earlier version of this file claimed "no ticket persists from this module ... read-only-safe by construction"; that was wrong and leaked one ticket per live run; -* one create with the full documented body, to prove the ids land; -* one from ``ticket_factory`` for the ``set_status`` check. +* one create with the full documented body, to prove the ids land. -The ticket-creating fixture lives in ``conftest.py`` and is also used by -``test_live_search_syntax``. +This module creates and closes its own tickets, by marker. The shared +ticket-creating fixtures (``rich_ticket``, ``probe_tickets``, +``ticket_factory``) live in ``conftest.py`` and serve the other live modules, +``test_live_search_syntax`` among them. Every assertion here is by shape, and every one routes through ``_assertions`` or a pre-bound local (design principle P2). pytest's assertion rewriter reports @@ -44,6 +45,7 @@ EasyvistaValidationError, PostRequest, Request, + WorkflowEffect, ev_equals_filter, ) from integration_tests._assertions import assert_shape @@ -206,42 +208,6 @@ def test_the_documented_create_body_lands_every_id( _close_by_marker(live_client, live_write_config, marker) -def test_set_status_reaches_a_non_terminal_status( - live_client: EasyvistaClient, - live_write_client: EasyvistaClient, - live_write_config: dict[str, str], - ticket_factory, -) -> None: - """``set_status`` sets an arbitrary status, not only a closing one. - - The API has no flat status update -- ``RequestUpdate`` carries no - ``status_id`` for that reason -- and the ``{"closed": {"status_GUID": ...}}`` - envelope is the only route. Its wire name suggests it only closes; measured, - it reaches every status tried. - - This pins the non-terminal case specifically, because that is the surprising - half and the half a future reader is most likely to "simplify" away. The GUID - is read off the instance rather than hardcoded: status GUIDs are per-instance - configuration, so a literal here would be a value this repo must not carry - and would be wrong on any other deployment anyway. - """ - rfc = ticket_factory() - before = live_client.get_ticket(rfc).status_id - target_guid, target_id = _a_different_status(live_client, exclude=before) - if target_guid is None: - pytest.skip("no second status with a readable GUID on this instance") - - live_write_client.set_status( - rfc, status_guid=target_guid, comment="capability-suite status probe" - ) - after = live_client.get_ticket(rfc).status_id - # Bound as bools: the ids are instance configuration, not suite-authored (P2). - moved = str(after) != str(before) - landed_on_target = str(after) == str(target_id) - assert moved, "set_status did not change the ticket's status" - assert landed_on_target, "set_status landed on a status other than the one asked" - - def _close_by_marker(client: EasyvistaClient, cfg: dict[str, str], marker: str) -> None: """Close every ticket carrying ``marker``, however it got there. @@ -268,35 +234,10 @@ def _close_by_marker(client: EasyvistaClient, cfg: dict[str, str], marker: str) continue try: client.close_ticket( - rfc, status_guid=cfg["status_guid"], comment="smoke cleanup" + rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, + status_guid=cfg["status_guid"], + comment="smoke cleanup", ) except EasyvistaError: continue - - -def _a_different_status( - client: EasyvistaClient, *, exclude: object -) -> tuple[str | None, str | None]: - """Return ``(status_guid, status_id)`` for some status that is not ``exclude``. - - Read off the instance because status GUIDs are per-instance configuration: a - literal would be a value this repo must not carry, and would be wrong on any - other deployment. Found by sampling tickets and taking the first whose status - differs -- the nested ``STATUS`` object carries both the id and the GUID, - but only on an UNPROJECTED read, so no ``fields`` is passed here. - """ - try: - sampled = client.search_tickets(sort="LAST_UPDATE DESC", max_rows=60) - except EasyvistaError: - return None, None - for record in sampled.records: - status = record.model_extra.get("STATUS") if record.model_extra else None - if not isinstance(status, dict): - continue - sid = status.get("STATUS_ID") - guid = status.get("STATUS_GUID") - if sid is None or not guid: - continue - if str(sid) != str(exclude): - return str(guid), str(sid) - return None, None diff --git a/integration_tests/test_live_workflow_guard.py b/integration_tests/test_live_workflow_guard.py new file mode 100644 index 0000000..a413642 --- /dev/null +++ b/integration_tests/test_live_workflow_guard.py @@ -0,0 +1,506 @@ +"""Live checks behind the workflow guard. + +``end_action`` refuses to end a workflow step unless the caller allows +``WorkflowEffect.ADVANCES``, and it tells a step from the caller's own action +with one projected item read: ``WORKFLOW_ID`` set means a step. That only works +if the read names the column on BOTH kinds of action -- if it omits the key on +a caller's action, the guard (which fails closed) would refuse every end. + +The first two tests here read only. Tests below the census marker WRITE: each +takes the ``census_opt_in`` fixture first, so they run only when +``EASYVISTA_TEST_RUN_WORKFLOW_CENSUS=1`` is set in the environment, which is +how the user's explicit approval is given. Without it they skip before any +ticket is created. +""" + +from __future__ import annotations + +import os +import time +from collections.abc import Callable +from pathlib import Path +from typing import NamedTuple + +import pytest + +from easyvista_python_client import Action, ActionUpdate, EasyvistaClient, RequestUpdate + +#: Recent tickets scanned for one action of each kind; bounded, so a quiet +#: instance skips instead of sweeping the whole table. +_TICKETS_TO_SCAN = 15 +_LIST_PROJECTION = ["ACTION_ID", "ACTION_TYPE_ID", "WORKFLOW_ID", "END_DATE_UT"] +_PROBE_FIELDS = ["ACTION_ID", "WORKFLOW_ID"] + + +def _one_of_each(client: EasyvistaClient) -> tuple[int, int]: + """Return (a workflow-step action id, a non-workflow action id).""" + step: int | None = None + other: int | None = None + # The default order is oldest-first, and on the measured instance + # (2026-10-02) the oldest tickets carry only an already-ended CALL action + # and no workflow step, so the scan reads the newest tickets instead. + for ticket in client.iter_tickets( + fields=["RFC_NUMBER"], sort="REQUEST_ID DESC", max_records=_TICKETS_TO_SCAN + ): + if not ticket.rfc_number: + continue + for action in client.iter_actions( + ticket.rfc_number, fields=_LIST_PROJECTION, max_records=200 + ): + if action.action_id is None: + continue + if action.workflow_id is not None: + step = step or action.action_id + else: + other = other or action.action_id + if step is not None and other is not None: + return step, other + pytest.skip( + f"no ticket among the {_TICKETS_TO_SCAN} scanned carries both a workflow " + "step and a non-workflow action" + ) + + +def _probe(client: EasyvistaClient, action_id: int): + # The same projection end_action's guard asks for, through the public read. + return client.get_action(action_id, params={"fields": ",".join(_PROBE_FIELDS)}) + + +def test_the_projected_item_read_names_workflow_id_on_both_kinds_of_action( + live_client: EasyvistaClient, +) -> None: + step, other = _one_of_each(live_client) + step_row = _probe(live_client, step) + other_row = _probe(live_client, other) + # Through _require, never a bare assert: on failure the rewriter would print + # the whole Action, href host included (P2). + _require( + "workflow_id" in step_row.model_fields_set, + "the projected item read omits WORKFLOW_ID on a workflow step", + ) + _require(step_row.workflow_id is not None, "a workflow step has no WORKFLOW_ID") + _require( + "workflow_id" in other_row.model_fields_set, + "the projected item read omits WORKFLOW_ID on a non-workflow action: " + "end_action's pre-flight would refuse every caller action", + ) + _require(other_row.workflow_id is None, "a non-workflow action has a WORKFLOW_ID") + + +def test_the_plain_item_read_names_workflow_id_on_both_kinds_of_action( + live_client: EasyvistaClient, +) -> None: + """Recorded for comparison; the guard uses the projected read.""" + step, other = _one_of_each(live_client) + step_row = live_client.get_action(step) + other_row = live_client.get_action(other) + _require( + "workflow_id" in step_row.model_fields_set, + "the plain item read omits WORKFLOW_ID on a workflow step", + ) + _require( + "workflow_id" in other_row.model_fields_set, + "the plain item read omits WORKFLOW_ID on a non-workflow action", + ) + + +# --- census: WRITES, run only with the user's explicit approval ------------ + +_OPT_IN_VARIABLE = "EASYVISTA_TEST_RUN_WORKFLOW_CENSUS" +_SECRETS_DIR = Path(__file__).resolve().parents[1] / "secrets" + +#: Seconds between the immediate after-read and the settled one. The assertions +#: run on the settled read; both are printed, so a write that lands late, or +#: reverts, shows up in the output instead of reading as a clean pass. +_SETTLE_SECONDS = 5 + + +@pytest.fixture(scope="session") +def census_opt_in() -> None: + """Skip the census unless the environment says the user approved the writes. + + Session-scoped and listed first by every census test, so it is evaluated + before any other fixture and before a ticket can be created. + """ + if os.environ.get(_OPT_IN_VARIABLE) != "1": + pytest.skip( + "the workflow census WRITES to the live instance (creates tickets, " + f"reassigns a workflow step); set {_OPT_IN_VARIABLE}=1 to run it" + ) + + +def _resolve_local(env_names: tuple[str, ...], filename: str) -> str | None: + """Env var first, then ``secrets/``; ``None`` when neither is set.""" + for name in env_names: + value = os.environ.get(name) + if value and value.strip(): + return value.strip() + path = _SECRETS_DIR / filename + if path.is_file(): + text = path.read_text(encoding="utf-8").strip() + if text: + return text + return None + + +# Module-local on purpose: this file must also run from a checkout whose +# conftest.py lacks this fixture. +@pytest.fixture(scope="session") +def live_reassign_config() -> dict[str, str]: + """The group (and optionally the person) a workflow step is reassigned to. + + Separate from every other write config so that an instance without one + skips only the reassignment census. The group must differ from the one a + fresh ticket's workflow step is assigned to, and reassigning to the group + or to the person may notify them. + """ + group = _resolve_local( + ("EASYVISTA_TEST_REASSIGN_GROUP_ID",), "easyvista_test_reassign_group_id" + ) + if not group: + pytest.skip( + "the reassignment census needs EASYVISTA_TEST_REASSIGN_GROUP_ID " + "(or secrets/easyvista_test_reassign_group_id)" + ) + resolved = {"group_id": group} + person = _resolve_local( + ("EASYVISTA_TEST_REASSIGN_DONE_BY_ID",), "easyvista_test_reassign_done_by_id" + ) + if person: + resolved["done_by_id"] = person + return resolved + + +_CENSUS_PROJECTION = [ + "ACTION_ID", + "ACTION_TYPE_ID", + "WORKFLOW_ID", + "END_DATE_UT", + "GROUP_ID", + "DONE_BY_ID", + "REQUEST_ID", +] + +#: The item read the before and after states both go through, so a before/after +#: difference can never come from comparing two different read paths. +_ITEM_FIELDS = "ACTION_ID,REQUEST_ID,GROUP_ID,DONE_BY_ID,END_DATE_UT,WORKFLOW_ID" + + +def _require(condition: object, label: str) -> None: + """Assert ``condition``; the failure text is ``label`` and nothing else. + + The caller evaluates the condition as an argument, and it is bound to a + plain local here before the assert, so pytest's assertion rewriter has no + operand to render. An ``assert action.group_id == target`` would print the + whole ``Action`` -- hrefs and labels -- on failure (P2; see ``_assertions.py``). + """ + __tracebackhide__ = True + ok = bool(condition) + assert ok, label + + +def _target(config: dict[str, str], key: str) -> int: + """The configured id as a positive int; fails with a label, never the value. + + Called before ``ticket_factory()`` so a misconfiguration creates no ticket. + """ + try: + value = int(config[key]) + except ValueError: + value = 0 + if value <= 0: + pytest.fail(f"the configured {key} must be a positive integer", pytrace=False) + return value + + +class _Snapshot(NamedTuple): + """Everything the census compares, read in one pass.""" + + request_id: int | None + status_id: int | None + ticket_end: object + ticket_end_named: bool + open_ids: frozenset[int] + row_ids: frozenset[int] + step: Action + + +def _actions(client: EasyvistaClient, rfc: str) -> list[Action]: + rows = list(client.iter_actions(rfc, fields=_CENSUS_PROJECTION, max_records=500)) + for row in rows: + _require( + "end_date_ut" in row.model_fields_set, + "END_DATE_UT is not named by the projected list read", + ) + _require( + isinstance(row.action_id, int), + "a row of the projected list read carries no ACTION_ID", + ) + return rows + + +def _the_open_step(client: EasyvistaClient, rfc: str) -> Action: + steps = [ + row + for row in _actions(client, rfc) + if row.end_date_ut is None and row.workflow_id is not None + ] + _require( + len(steps) == 1, "the ticket does not carry exactly one open workflow step" + ) + step = steps[0] + _require( + isinstance(step.action_id, int) and step.action_id > 0, + "the open workflow step carries no usable ACTION_ID", + ) + return step + + +def _read(client: EasyvistaClient, action_id: int) -> Action: + return client.get_action(action_id, params={"fields": _ITEM_FIELDS}) + + +def _snapshot(client: EasyvistaClient, rfc: str, step_id: int) -> _Snapshot: + ticket = client.get_ticket(rfc) + rows = _actions(client, rfc) + return _Snapshot( + request_id=ticket.request_id, + status_id=ticket.status_id, + ticket_end=ticket.end_date_ut, + ticket_end_named="end_date_ut" in ticket.model_fields_set, + open_ids=frozenset( + row.action_id + for row in rows + if row.end_date_ut is None and row.action_id is not None + ), + row_ids=frozenset(row.action_id for row in rows if row.action_id is not None), + step=_read(client, step_id), + ) + + +def _describe(before: _Snapshot, after: _Snapshot, column: str | None) -> str: + stored = f"stored={getattr(after.step, column)} " if column else "" + return ( + f"{stored}step_open={after.step.end_date_ut is None} " + f"status {before.status_id}->{after.status_id} " + f"ticket_end_date_ut {before.ticket_end}->{after.ticket_end} " + f"open {sorted(before.open_ids)}->{sorted(after.open_ids)} " + f"new_rows={sorted(after.row_ids - before.row_ids)}" + ) + + +def _check_before(step: Action, before: _Snapshot) -> None: + """Preconditions that make a later "unchanged" verdict mean something. + + Run before the write, so a vacuous read (a column that is not named, a step + that belongs to another ticket) stops the census with nothing sent. + """ + _require(before.request_id is not None, "the fresh ticket carries no REQUEST_ID") + _require(step.request_id is not None, "the listed step carries no REQUEST_ID") + _require( + step.request_id == before.request_id, + "the listed step does not belong to the fresh ticket", + ) + _require( + before.step.request_id == before.request_id, + "the step's item read does not belong to the fresh ticket", + ) + _require(before.status_id is not None, "the ticket's STATUS_ID reads empty") + _require( + before.ticket_end_named, + "END_DATE_UT is not named by the ticket read, so 'unchanged' is vacuous", + ) + _require( + "end_date_ut" in before.step.model_fields_set, + "END_DATE_UT is not named by the projected item read", + ) + _require( + before.step.end_date_ut is None, "the step is already ended before the write" + ) + _require(before.step.workflow_id is not None, "the step reads as no workflow step") + + +def _check_unchanged(before: _Snapshot, after: _Snapshot) -> None: + """The workflow-neutral half of the verdict, on the settled read.""" + _require( + "end_date_ut" in after.step.model_fields_set, + "END_DATE_UT is not named by the settled item read", + ) + _require(after.step.end_date_ut is None, "the write ENDED the workflow step") + _require( + after.open_ids == before.open_ids, + "the write changed the ticket's open actions", + ) + _require(after.status_id == before.status_id, "the write moved the ticket's status") + _require( + after.ticket_end == before.ticket_end, + "the write changed the ticket's END_DATE_UT", + ) + + +def _write_and_observe( + client: EasyvistaClient, + rfc: str, + step_id: int, + label: str, + before: _Snapshot, + send: Callable[[], object], + column: str | None = None, +) -> _Snapshot: + """Send one write, then read the state twice; a failed write still reads. + + A raised write is reduced to its type name and status code, which are + printed; the exception itself is dropped, because its message is server + prose this suite keeps out of test output (P2). The reads are still taken + and printed, and then the test fails with a label-only message. + Returns the settled snapshot. + """ + failure: str | None = None + try: + send() + except Exception as exc: + failure = ( + f"census write failed: {type(exc).__name__} " + f"status_code={getattr(exc, 'status_code', None)}" + ) + print(f"CENSUS {rfc} {label}: {failure}") + immediate = _snapshot(client, rfc, step_id) + print(f"CENSUS {rfc} {label} immediate: {_describe(before, immediate, column)}") + time.sleep(_SETTLE_SECONDS) + settled = _snapshot(client, rfc, step_id) + print( + f"CENSUS {rfc} {label} +{_SETTLE_SECONDS}s: " + f"{_describe(before, settled, column)}" + ) + if failure is not None: + pytest.fail(failure, pytrace=False) + return settled + + +def _census( + client: EasyvistaClient, + write_client: EasyvistaClient, + rfc: str, + step: Action, + column: str, + target: int, + *, + require_current: bool, +) -> None: + """Write ``target`` into ``column`` of the step and record what moved. + + The lower-case body key is sent first, as ``PostAction``'s verified create + body spells it. If the column did not store, the upper-case key goes to the + SAME step, so the key spelling is settled without a second ticket; both + outcomes are printed and the test passes if either stored. + + ``require_current`` is for a column a workflow step is born with (the + group). ``DONE_BY_ID`` is documented empty on a generated step, so there an + empty before-value is the expected shape; the column must still be NAMED by + the read, which is what tells empty from unprojected. + """ + step_id = step.action_id # a positive int: _the_open_step refuses anything else + before = _snapshot(client, rfc, step_id) + _check_before(step, before) + name = column.upper() + _require( + column in before.step.model_fields_set, + f"{name} is not named by the projected item read", + ) + current = getattr(before.step, column) + if require_current: + _require( + current is not None, f"{name} reads empty on the step before the write" + ) + _require(current != target, f"{name} already equals the target before the write") + print(f"CENSUS {rfc}: before {column}={current} target={target}") + + outcomes: dict[str, bool] = {} + for key in (column, name): + body = {key: target} + settled = _write_and_observe( + client, + rfc, + step_id, + f"key={key}", + before, + lambda body=body: write_client.update_action( + step_id, ActionUpdate(extra_payload=body) + ), + column, + ) + _check_unchanged(before, settled) + outcomes[key] = getattr(settled.step, column) == target + if outcomes[key]: + break + print(f"CENSUS {rfc}: stored by key spelling {outcomes}") + _require( + any(outcomes.values()), + f"neither {column!r} nor {name!r} was stored -- a 200 is not a receipt", + ) + + +def test_reassigning_the_workflow_step_to_a_group_keeps_it_open( + census_opt_in, + live_client, + live_write_client, + ticket_factory, + live_reassign_config, +) -> None: + target = _target(live_reassign_config, "group_id") + rfc = ticket_factory() + step = _the_open_step(live_client, rfc) + _census( + live_client, + live_write_client, + rfc, + step, + "group_id", + target, + require_current=True, + ) + + +def test_reassigning_the_workflow_step_to_a_person_keeps_it_open( + census_opt_in, + live_client, + live_write_client, + ticket_factory, + live_reassign_config, +) -> None: + if "done_by_id" not in live_reassign_config: + pytest.skip("EASYVISTA_TEST_REASSIGN_DONE_BY_ID not configured") + target = _target(live_reassign_config, "done_by_id") + rfc = ticket_factory() + step = _the_open_step(live_client, rfc) + _census( + live_client, + live_write_client, + rfc, + step, + "done_by_id", + target, + require_current=False, + ) + + +def test_the_ticket_writes_the_sync_makes_keep_the_workflow_step_open( + census_opt_in, live_client, live_write_client, ticket_factory +) -> None: + """Title is what the sync writes each sweep; only description was censused.""" + rfc = ticket_factory() + step = _the_open_step(live_client, rfc) + step_id = step.action_id # a positive int: _the_open_step refuses anything else + before = _snapshot(live_client, rfc, step_id) + _check_before(step, before) + settled = _write_and_observe( + live_client, + rfc, + step_id, + "title", + before, + lambda: live_write_client.update_ticket( + rfc, RequestUpdate(title=f"{rfc} census title") + ), + ) + _check_unchanged(before, settled) diff --git a/scripts/tests/test_skills_contract.py b/scripts/tests/test_skills_contract.py index 93bf957..cae3431 100644 --- a/scripts/tests/test_skills_contract.py +++ b/scripts/tests/test_skills_contract.py @@ -20,7 +20,12 @@ only names imported from the package root and keywords passed to a client method or a write model are looked up. - **No positional arguments and no arity.** Only ``keyword=`` arguments are - matched against the signature. + matched against the signature -- with one exception: a **required + keyword-only** parameter (``close_ticket``'s ``allow_workflow_effect``) must + be passed by every snippet that calls the method, since a snippet that omits + it raises ``TypeError`` when an agent runs it verbatim. A call that splats + ``**kwargs`` is exempt, because the splat may supply it. A positional + parameter's arity is still not checked. - **No required fields and no value types.** ``PostAsset(catalog_id="1")`` passes even though the field is an ``int``, and a write model missing a mandatory field passes too -- nothing is ever instantiated. @@ -298,6 +303,25 @@ def _write_model_name(func: ast.expr) -> str | None: return None +def _missing_required_keywords( + call: ast.Call, signature: inspect.Signature +) -> set[str]: + """Required keyword-only parameters of ``signature`` that ``call`` omits. + + A call that splats ``**kwargs`` may be supplying any of them, so its + required ones cannot be judged from the source text and none is reported. + """ + if any(keyword.arg is None for keyword in call.keywords): + return set() + required = { + name + for name, param in signature.parameters.items() + if param.kind is inspect.Parameter.KEYWORD_ONLY + and param.default is inspect.Parameter.empty + } + return required - {keyword.arg for keyword in call.keywords} + + def _snippet_trees(skill: Path) -> list[ast.Module]: text = (skill / "SKILL.md").read_text(encoding="utf-8") return [ast.parse(block) for block in _python_blocks(text)] @@ -371,6 +395,12 @@ def test_client_methods_and_keywords_exist(skill: Path) -> None: f"{skill.name} passes {keyword.arg}= to client.{method}(), " f"which accepts {sorted(accepted)}" ) + missing = _missing_required_keywords(call, signature) + assert not missing, ( + f"{skill.name} calls client.{method}() without its required " + f"keyword-only parameter(s) {sorted(missing)}; an agent runs a " + "skill's snippet verbatim, so the snippet would raise TypeError" + ) @pytest.mark.parametrize("skill", _skill_dirs(), ids=_skill_ids()) @@ -461,6 +491,47 @@ def test_snippet_hosts_are_synthetic(skill: Path) -> None: ) +@pytest.mark.parametrize( + ("source", "expected"), + [ + # The case the check exists for: a required keyword is left out. + ('client.close_ticket("R", status_guid="g")', {"allow_workflow_effect"}), + # A splat may supply it, so the call is exempt rather than reported. + ('client.close_ticket("R", **opts)', set()), + ('client.close_ticket("R", status_guid="g", **opts)', set()), + # A compliant call reports nothing, whatever else it passes. + ('client.close_ticket("R", allow_workflow_effect=effect)', set()), + ( + 'client.close_ticket("R", allow_workflow_effect=effect, status_guid="g")', + set(), + ), + ], +) +def test_required_keyword_check_sees_what_it_should( + source: str, expected: set[str] +) -> None: + """The required-keyword check is itself checked, so it cannot go inert. + + ``test_client_methods_and_keywords_exist`` only ever sees the skills as they + are. If the helper silently returned an empty set, every skill would pass + it. This feeds it synthetic snippets and a real signature that does carry a + required keyword-only parameter. + """ + signature = inspect.signature(ev.EasyvistaClient.close_ticket) + required = { + name + for name, param in signature.parameters.items() + if param.kind is inspect.Parameter.KEYWORD_ONLY + and param.default is inspect.Parameter.empty + } + assert "allow_workflow_effect" in required, ( + "close_ticket no longer has a required keyword-only parameter, so this " + "self-test no longer exercises the check; pick another method" + ) + (call,) = _client_calls(ast.parse(source)) + assert _missing_required_keywords(call, signature) == expected + + def test_write_models_map_is_complete() -> None: """Every exported EasyvistaWriteModel subclass maps in _WRITE_MODELS. diff --git a/scripts/validate_docs_examples.py b/scripts/validate_docs_examples.py index 004da90..f5d0948 100644 --- a/scripts/validate_docs_examples.py +++ b/scripts/validate_docs_examples.py @@ -332,7 +332,13 @@ def signatures() -> None: "create_tickets": {"tickets"}, "get_ticket": {"rfc_number"}, "update_ticket": {"rfc_number", "update"}, - "close_ticket": {"rfc_number", "status_guid", "delete_actions", "comment"}, + "close_ticket": { + "rfc_number", + "allow_workflow_effect", + "status_guid", + "delete_actions", + "comment", + }, "create_action": {"rfc_number", "action"}, "list_actions": {"rfc_number"}, "iter_actions": {"rfc_number", "fields", "page_size", "max_records"}, @@ -805,6 +811,7 @@ def run_live_writes( PostRequest, Request, RequestUpdate, + WorkflowEffect, ) created_rfcs: list[str] = [] @@ -935,11 +942,13 @@ def create_asset() -> None: if autoclose and status_guid: for target in list(created_rfcs): r.check_perm( - f"close_ticket('{target}', status_guid=..., delete_actions=1," - " comment=...)", + f"close_ticket('{target}'," + " allow_workflow_effect=WorkflowEffect.INTERRUPTS," + " status_guid=..., delete_actions=1, comment=...)", partial( client.close_ticket, target, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=status_guid, delete_actions=1, comment="Resolved by validation", diff --git a/scripts/validate_live_content_fidelity.py b/scripts/validate_live_content_fidelity.py index e88fa88..17f78fd 100644 --- a/scripts/validate_live_content_fidelity.py +++ b/scripts/validate_live_content_fidelity.py @@ -676,7 +676,12 @@ def run_tolerance( -> ``update_ticket(description=probe)`` -> read back at ``/comment`` -> classify. Each probe ticket is closed right after unless ``close_each`` is False. """ - from easyvista_python_client import EasyvistaError, PostRequest, RequestUpdate + from easyvista_python_client import ( + EasyvistaError, + PostRequest, + RequestUpdate, + WorkflowEffect, + ) results: list[Probe] = [] for index, (label, payload) in enumerate(COMMENT_PROBES, start=1): @@ -723,6 +728,7 @@ def run_tolerance( try: client.close_ticket( rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=status_guid, delete_actions=1, comment="tolerance probe cleanup", @@ -742,8 +748,11 @@ def run_tolerance( # cleanup # --------------------------------------------------------------------------- # def do_close(client: EasyvistaClient, rfc: str, status_guid: str) -> None: + from easyvista_python_client import WorkflowEffect + client.close_ticket( rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=status_guid, delete_actions=1, comment="Cloture apres validation de fidelite du contenu", diff --git a/skills/easyvista-client-setup/SKILL.md b/skills/easyvista-client-setup/SKILL.md index c66cea8..91a733e 100644 --- a/skills/easyvista-client-setup/SKILL.md +++ b/skills/easyvista-client-setup/SKILL.md @@ -34,8 +34,11 @@ with `async for`; `await client.stream_document(...)` raises `TypeError`. anything with credentials in the environment. 6. Keep `verify_ssl=True` unless the user confirms an internal endpoint that cannot present a valid chain. -7. Raise `max_retries` above its `0` default only for flaky networks; 429 and - 5xx are retried with exponential backoff, and 590 deliberately is not. +7. Raise `max_retries` above its `0` default only for flaky networks, and keep + it `0` for a client that writes: 429, 5xx and connection errors are retried + with exponential backoff **for every verb** (a resent create can duplicate + the ticket), and 590 deliberately is not. A write allowed to change the + workflow is sent once whatever `max_retries` says. 8. Use the client as a context manager so its HTTP session closes; call `client.close()` (`await client.aclose()`) when it outlives the block. @@ -51,7 +54,7 @@ Every `EasyvistaConfig` field, and its default: | `login` | `None` | HTTP Basic credential, paired with `password` | | `password` | `None` | HTTP Basic credential, paired with `login` | | `timeout` | `30.0` | Seconds | -| `max_retries` | `0` | Applies to 429 and 5xx only | +| `max_retries` | `0` | Applies to 429, 5xx and connection errors, for every verb — except that a write allowed to change the workflow is sent once. Keep it 0 for writers: a resent create can duplicate the ticket. | | `verify_ssl` | `True` | `True`/`False`, a CA-bundle path, or an `ssl.SSLContext` — a private CA does **not** require disabling verification | | `default_max_rows` | `100` | Page size when `max_rows` / `page_size` is omitted | | `api_version` | `"v1"` | Used to build `api_root` | @@ -92,7 +95,8 @@ config = EasyvistaConfig( ## Reaching a route this package does not wrap `client.send()` is the escape hatch. This package wraps roughly ten of the -paths an instance advertises; `send` reaches the rest with the same retries +paths an instance advertises; `send` reaches the rest with the same retries, +the same workflow guard — pass `allow_workflow_effect=` for a workflow write — and the same error mapping, returning the decoded JSON unchanged. ```python @@ -237,9 +241,14 @@ with EasyvistaClient.from_env() as client: | 429 | `EasyvistaRateLimitError` | Retried when `max_retries > 0`. | | 5xx | `EasyvistaServerError` | Retried when `max_retries > 0`. | | Transport failure (timeout, refused connection) | `EasyvistaConnectionError` | No response was obtained at all. | +| *(none: raised before sending)* | `EasyvistaWorkflowEffectRefused` — a `ValueError`, **not** an `EasyvistaError` | A write that may change the workflow, refused before sending; never retry it. | -Every one of these carries `status_code`, `ev_code` and `ev_message`, and all -derive from `EasyvistaError`. +Every HTTP-derived one of these carries `status_code`, `ev_code` and +`ev_message`, and all derive from `EasyvistaError`. The last row is the +exception on both counts: it has no response to carry a status from, so it is +not an `EasyvistaError`, and `except EasyvistaError` does not catch it. Never +retry it: pass the right `allow_workflow_effect=` when changing the workflow is +the intent, and otherwise drop the write. ## Gotchas @@ -261,6 +270,8 @@ derive from `EasyvistaError`. - `from_env()` accepts no overrides, unlike the sister GLPI client's. - `default_max_rows` (100) is the page size used when `max_rows` / `page_size` is omitted — it is not a total cap; the `iter_*` methods page past it. -- Retries are off by default (`max_retries=0`). +- Retries are off by default (`max_retries=0`). Leave them off for a client that + writes: the retry covers every verb, and a resent create can duplicate the + ticket. - Closing matters: the client owns an HTTP session. Prefer the context manager. diff --git a/skills/easyvista-instance-discovery/SKILL.md b/skills/easyvista-instance-discovery/SKILL.md index 19e7c55..9709d48 100644 --- a/skills/easyvista-instance-discovery/SKILL.md +++ b/skills/easyvista-instance-discovery/SKILL.md @@ -1,6 +1,6 @@ --- name: easyvista-instance-discovery -description: "Discover what one EasyVista deployment actually exposes with easyvista_python_client — get_api_spec reads the instance's own OpenAPI, list_reference_table reads any list route into column-free records, discover resolves one reference name to the ids/labels/codes/GUIDs in use, and describe_instance profiles the lot into an InstanceProfile. Use before hardcoding any id, when a ticket create is rejected for an unknown catalog, urgency, impact or group, when you need a STATUS_GUID for set_status or close_ticket, or when you need to know which routes a deployment declares at all." +description: "Discover what one EasyVista deployment actually exposes with easyvista_python_client — get_api_spec reads the instance's own OpenAPI, list_reference_table reads any list route into column-free records, discover resolves one reference name to the ids/labels/codes/GUIDs in use, and describe_instance profiles the lot into an InstanceProfile. Use before hardcoding any id, when a ticket create is rejected for an unknown catalog, urgency, impact or group, when you need a STATUS_GUID for close_ticket, or when you need to know which routes a deployment declares at all." license: MIT compatibility: "Requires Python 3.11+, easyvista-python-client, and network access to an EasyVista Service Manager REST API. Every call here is a GET; nothing is created, updated or deleted." metadata: @@ -39,7 +39,9 @@ start-up and fail loudly; never freeze one into code. named there. 2. For one reference, `discover(name)`. Use `.id` for a write model's `*_id` field, `.code` for `PostRequest(catalog_code=...)`, and `.guid` for - `set_status` / `close_ticket`. + `close_ticket` — the vendor close request, which stops the ticket's workflow + (documented for final statuses; nothing exempts a non-final one), so it is + not a way to pick an intermediate status (see `easyvista-ticket-workflow`). 3. For a route this package does not model at all, `list_reference_table(path)` — check `get_api_spec()["paths"]` to see which your deployment declares. 4. Never cache an id across deployments. Re-resolve, or fail loudly. @@ -56,7 +58,7 @@ with EasyvistaClient.from_env() as client: print("gap:", gap, reason) for status in client.discover("STATUS"): - # .guid is what set_status and close_ticket address a status by. + # .guid is what close_ticket addresses a status by. print(status.id, status.label, status.guid) for catalog in client.discover("CATALOG_REQUEST"): diff --git a/skills/easyvista-ticket-actions/SKILL.md b/skills/easyvista-ticket-actions/SKILL.md index 1820b65..112c67d 100644 --- a/skills/easyvista-ticket-actions/SKILL.md +++ b/skills/easyvista-ticket-actions/SKILL.md @@ -1,6 +1,6 @@ --- name: easyvista-ticket-actions -description: "Read and write the action log on an EasyVista ticket with easyvista_python_client — create_task and PostTask (the one call that posts a COMMENT: a task is an action born already ended, so its text shows in the history), plus create_action, end_action (an action is born OPEN and its text does not show until ended), list_actions, iter_actions, get_action and update_action with PostAction, Action and ActionUpdate. Covers why there is no private-comment flag and that visibility is the action TYPE instead, how to recover a created action's id, how to page a whole log past the one-page cap, and how to resolve an action's note text, which the list endpoint does not return. Use for ticket comments, followups, work notes, internal or private comments, progress entries or any per-ticket action history." +description: "Read and write the action log on an EasyVista ticket with easyvista_python_client — create_task and PostTask (the one call that posts a COMMENT: a task is an action born already ended, so its text shows in the history), plus create_action, end_action (an action is born OPEN and its text does not show until ended), list_actions, iter_actions, get_action, update_action and reassign_action (hand an action, such as the open workflow step, to another group or person without ending it) with PostAction, Action and ActionUpdate. Covers why there is no private-comment flag and that visibility is the action TYPE instead, how to recover a created action's id, how to page a whole log past the one-page cap, and how to resolve an action's note text, which the list endpoint does not return. Use for ticket comments, followups, work notes, internal or private comments, progress entries or any per-ticket action history, and to reassign, escalate or transfer an action." license: MIT compatibility: "Requires Python 3.11+, easyvista-python-client, network access to an EasyVista Service Manager REST API, and a profile authorized for the actions sub-resource." metadata: @@ -14,9 +14,10 @@ metadata: > `easyvista-client-setup`. Actions are EasyVista's per-ticket work log — the closest equivalent to a -followup. Six methods: `create_action(rfc, action)`, `list_actions(rfc)`, -`iter_actions(rfc)`, `get_action(action_id)`, `update_action(action_id, -update)` and `end_action(rfc, action_id=...)`. The list and item shapes differ +followup. Eight methods: `create_task(rfc, task)`, `create_action(rfc, action)`, +`list_actions(rfc)`, `iter_actions(rfc)`, `get_action(action_id)`, +`update_action(action_id, update)`, `reassign_action(action_id, group_id=...)` +and `end_action(rfc, action_id=...)`. The list and item shapes differ substantially, which is where most mistakes come from. ## Two shapes of the same record @@ -195,10 +196,43 @@ with EasyvistaClient.from_env() as client: > spawned a new open type-1 *Validation Self Service* action. A control the > same day showed ending a type-94 action the caller had created left both the > status and the action count untouched. So ending your own action is inert; -> ending a workflow step is a state change on the ticket. **Omitting -> `action_id` ends every open action**, which on a ticket whose only open one -> is its workflow step means resolving it — name the action unless you mean -> that. +> ending a workflow step is a state change on the ticket. +> +> **`end_action` therefore guards it.** It makes one read first (an item read +> projecting `ACTION_ID` and `WORKFLOW_ID`) and refuses a workflow step +> (`WORKFLOW_ID` set), a record that comes back without `WORKFLOW_ID`, or a +> record whose `ACTION_ID` is not the one you asked for, unless you pass +> `allow_workflow_effect=WorkflowEffect.ADVANCES`. If that read fails, its error +> propagates and the end request is not sent; a 403 there says nothing about +> whether ending is permitted. Ending your own action needs no opt-in; whether +> an action created under the step carries a `WORKFLOW_ID` is unmeasured — if it +> does, the end is refused, and you opt in. `WORKFLOW_ID` is what separates the +> engine's rows from a caller's (tier 4: 1500 of 1500 rows, 2026-09-02, one +> instance, so it may not generalise). The refusal is +> `EasyvistaWorkflowEffectRefused`, a `ValueError` (not an `EasyvistaError`), +> raised before the end request; an allowed end is sent once, never retried. +> `action_id` must be a positive integer. +> +> **`end_all=True` ends every open action, the workflow step included; it needs +> `WorkflowEffect.ADVANCES`.** Omitting `action_id` is not that form: a bare +> `action_id=None` is refused, because `Action.action_id` is legitimately `None` +> on a create response or a projection without `ACTION_ID`. + +To end a workflow step on purpose, say so at the call site: + +```python +from easyvista_python_client import EasyvistaClient, WorkflowEffect + +with EasyvistaClient.from_env() as client: + client.end_action( + "YOUR_RFC_NUMBER", + action_id=YOUR_WORKFLOW_STEP_ACTION_ID, + # The workflow moves on to its next step. Status ids are per instance, + # so read the ticket back rather than assuming where it landed. + allow_workflow_effect=WorkflowEffect.ADVANCES, + ) + print(client.get_ticket("YOUR_RFC_NUMBER").reference("STATUS").display) +``` > **Retraction (2026-09-01).** An earlier revision of this skill said every > documented form returned `590 Action not found` and called that an @@ -321,6 +355,47 @@ Two asymmetries worth knowing: and PATCH on `actions/{id}` — there is no DELETE verb — so there is deliberately no `delete_action`. +## Reassign an action + +`reassign_action(action_id, group_id=..., done_by_id=...)` hands an action to +another group and/or person without ending it — the way to escalate the open +workflow step. At least one id is required; both are positive integers, and both +are per-deployment, so look them up (next section) rather than hardcoding them. A +group id is the one that may not be readable: on the measured instance +(2026-10-02, one instance) `GET groups` answered 403, which this API also answers +for an absent route, so if yours does too, take the group id from your +administrator or from a record that already carries one. `done_by_id` is an +employee id: find one with `search_employees` or `get_employee`. + +```python +from easyvista_python_client import EasyvistaClient + +with EasyvistaClient.from_env() as client: + actions = client.iter_actions( + "YOUR_RFC_NUMBER", + fields=["ACTION_ID", "WORKFLOW_ID", "GROUP_ID", "END_DATE_UT"], + ) + # The open workflow step: WORKFLOW_ID set, no end date yet. + step = next(a for a in actions if a.is_workflow_generated and not a.end_date_ut) + client.reassign_action(step.action_id, group_id=YOUR_OTHER_GROUP_ID) + # Re-read: this API answers 200 while dropping a field it did not store. + print(client.get_action(step.action_id).group_id) +``` + +The vendor documents no REST reassignment route (tier 1, +`rest-api-update-an-action.md` lists no group column among its exclusions, and the +UI's transfer is a wizard), so the effect is measured, not specified. Measured +2026-10-02 on one instance (Service Manager 2025.3; two tickets, so it may not +generalise): the group was stored (`GROUP_ID` 57 to 50 on the open workflow step +of both tickets); the step stayed open and the ticket's status did not move; the +open actions were unchanged and no new action rows appeared. **The ticket's own +`OWNING_GROUP_ID` does not follow the action's group** (it stayed 57 on the first +ticket, the only one read for it), so reassigning a step is not reassigning the +ticket. Reassigning to a person (`done_by_id`) was **not measured**, and whether +the UI wizard's notifications fire is not observable from the API. +`reassign_action` is not refused by the workflow guard, because the group and the +person are data the workflow reads, not workflow state. + ## Discover the ids first `action_type_id` and `group_id` are instance-specific. One call finds both — @@ -375,9 +450,14 @@ with the EasyVista administrator, then pin the ids in your own configuration. and an open action renders in the UI as a pending row with its text NOT shown, which reads as though the note was lost. Finish it with `end_action(rfc, action_id=...)` (see the section above for the fields - and for what ending a *workflow* action does to the ticket). Note the - create route is parent-resolved: it needs exactly one open action on the - ticket, or an explicit `parent_action_id` naming an open one. + and for what ending a *workflow* action does to the ticket). + `end_action` reads the action first and refuses a workflow step + (`WORKFLOW_ID` set) or a record without `WORKFLOW_ID` unless you pass + `allow_workflow_effect=WorkflowEffect.ADVANCES`; ending your own action + needs no opt-in, but whether an action created under a step carries a + `WORKFLOW_ID` is unmeasured — if it does, the end is refused, and you opt + in. Note the create route is parent-resolved: it needs exactly one open + action on the ticket, or an explicit `parent_action_id` naming an open one. 4. To address the action or task you just created, diff `list_actions` across the call — the create response cannot give you the id (see Gotchas). 5. To read note text, either call `get_action` and resolve the memo href with @@ -397,7 +477,7 @@ with EasyvistaClient.from_env() as client: client.create_task( "YOUR_RFC_NUMBER", PostTask( - action_type_id=1, + action_type_id=94, group_id=1, description="Called the user back; printer power-cycled.", ), @@ -406,7 +486,8 @@ with EasyvistaClient.from_env() as client: Only when the work is genuinely still to be done, an **action** instead. It is born open, so its text does not render in the history until it is ended — -finish it with `end_action` (above) once the work is done: +finish it with `end_action` (above) once the work is done; that needs no opt-in +for an action you created, subject to the `WORKFLOW_ID` caveat above: ```python from easyvista_python_client import EasyvistaClient, PostAction @@ -415,7 +496,7 @@ with EasyvistaClient.from_env() as client: action = client.create_action( "YOUR_RFC_NUMBER", PostAction( - action_type_id=1, + action_type_id=94, group_id=1, description="Chase the supplier for a replacement drum.", ), @@ -423,9 +504,10 @@ with EasyvistaClient.from_env() as client: print(action.href) ``` -`action_type_id=1` and `group_id=1` above are placeholders — use the ids -`client.discover("ACTION_TYPE")` and `client.discover("GROUP")` printed for -your instance. +`action_type_id=94` and `group_id=1` above are placeholders — 94 is the +public-comment type on one measured instance, where type 1 is a workflow step +type (measured 2026-09-01, one instance; it may not generalise). Read your own +with `client.discover("ACTION_TYPE")` and `client.discover("GROUP")`. ```python from easyvista_python_client import EasyvistaClient, PostAction @@ -436,7 +518,7 @@ with EasyvistaClient.from_env() as client: # The create response carries no ACTION_ID, so diff the list around it. before = {a.action_id for a in client.list_actions(rfc)} client.create_action( - rfc, PostAction(action_type_id=1, group_id=1, description="Triaged.") + rfc, PostAction(action_type_id=94, group_id=1, description="Triaged.") ) after = client.list_actions(rfc) created = [a for a in after if a.action_id not in before] @@ -547,9 +629,25 @@ with EasyvistaClient.from_env() as client: two or more gives `590 "Ambiguous query : many parent actions found"`, and an explicit `parent_action_id` naming an **open** action succeeds either way (an ended one is refused). A fresh ticket carries exactly one open workflow action, - and every `set_status` drains the open set to zero — so in practice a bare - `create_action` works only on a ticket nobody has moved yet. `create_task` is - not parent-resolved and is unaffected. + and a close request drains the open set to zero (the vendor close page says + the unfinished actions are deleted, or by our reading ended — tier 1; one + ticket was censused on 2026-09-01 on one instance — tier 4, so it may not + generalise) — so in practice a bare `create_action` works only on a ticket + nobody has moved yet. `create_task` is not parent-resolved and is unaffected. +- **The workflow guard covers the other action writes too.** `create_action`, + `create_task` and `update_action` refuse a body whose `extra_payload` ties the + record into the workflow — `WORKFLOW_ID`, `STAGE_ID`, `PROCESS_STEP_ID` or + `STATUS_ID_ON_TERMINATE`; on an action also an end date (a task is born ended, + so its end date is ordinary); on an update also a type, a parent or a ticket + link; on a task also `PARENT_ACTION_ID` — with + `EasyvistaWorkflowEffectRefused`, before any request, unless the call passes + `allow_workflow_effect=`. The fields `PostAction`, `PostTask` and + `ActionUpdate` declare need no opt-in. That is a deny-list of columns, and one + cannot be complete: the vendor's + [update-an-action page](https://docs.easyvista.com/docs/rest-api-update-an-action.md) + accepts "all the fields from the AM_ACTION table except those mentioned below" + (tier 1, read 2026-10-02), so a column it does not name is unclassified, not + proven neutral. - `action.action_type` is a nested object on the live API, not a string. Use `action.reference("ACTION_TYPE").display` for the label. - Resolving every body costs two extra requests per action (item fetch, then diff --git a/skills/easyvista-ticket-workflow/SKILL.md b/skills/easyvista-ticket-workflow/SKILL.md index 98a7ebe..585a591 100644 --- a/skills/easyvista-ticket-workflow/SKILL.md +++ b/skills/easyvista-ticket-workflow/SKILL.md @@ -84,14 +84,25 @@ deployment needs before you build a payload for it. `update_ticket(rfc, RequestUpdate(description=...))`. `RequestUpdate` also accepts `title`, `impact_id`, `owner_id` and `external_reference` (capped at 50 characters) after create — see the Gotchas for what it deliberately - omits, and use `set_status(rfc, status_guid=...)` for a status. + omits. The vendor documents no status write, and this package has none: a + ticket's status follows its workflow. To + complete a workflow step, end its open action with `end_action(rfc, + action_id=..., allow_workflow_effect=WorkflowEffect.ADVANCES)` — without + `ADVANCES`, ending a workflow step is refused (see + `easyvista-ticket-actions`); `close_ticket` is the vendor CLOSE request and + needs `allow_workflow_effect=WorkflowEffect.INTERRUPTS`. 6. Read one ticket with `get_ticket(rfc)`; search a page with `search_tickets(...)`, which returns a `SearchResult` carrying `.records`, `.record_count` (this page) and `.total_record_count` (every match on the server); walk every match with `iter_tickets(...)`, which yields `Request` objects directly and pages for you. -7. Close with `close_ticket(rfc, status_guid=..., delete_actions=..., - comment=...)`. +7. Close only when closing is the intent, with `close_ticket(rfc, + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid=..., + delete_actions=..., comment=...)`. The close request interrupts the + workflow, ends (by our reading of the page) or, with `delete_actions`, + deletes the unfinished actions, and inserts an anticipated closing action + (vendor close page, tier 1) — documented for final statuses; nothing exempts + the others. Then re-read the ticket: a 200 is not a receipt. ## Examples @@ -165,18 +176,29 @@ with EasyvistaClient.from_env() as client: ``` ```python -from easyvista_python_client import EasyvistaClient +from easyvista_python_client import EasyvistaClient, WorkflowEffect with EasyvistaClient.from_env() as client: closed = client.close_ticket( "YOUR_RFC_NUMBER", + # Required: the close request interrupts the ticket's workflow. + allow_workflow_effect=WorkflowEffect.INTERRUPTS, status_guid="YOUR_CLOSED_STATUS_GUID", - delete_actions=1, + delete_actions=1, # DELETES the unfinished actions comment="Resolved: printer power-cycled.", ) print(closed.rfc_number) + + # A 200 is not a receipt on this API: re-read. end_date_ut is stamped at + # resolution or closure, so a value here means "resolved or closed". + print(client.get_ticket("YOUR_RFC_NUMBER").end_date_ut) ``` +`allow_workflow_effect` is a required keyword of `close_ticket` (leaving it out is +a `TypeError`), and a value that does not include `WorkflowEffect.INTERRUPTS` is +refused before any request is sent (see the first Gotcha). `WorkflowEffect` and +`EasyvistaWorkflowEffectRefused` are both importable from the package root. + ```python from easyvista_python_client import EasyvistaClient, PostRequest @@ -192,6 +214,35 @@ with EasyvistaClient.from_env() as client: ## Gotchas +- **`close_ticket` is not a status setter.** It stops the workflow. Using it to + land an intermediate status (the package's former status setter was this same + request) ended the ticket's initial workflow action — that is how a + synchroniser closed tickets early. The root cause was established on + 2026-10-01/02 from the synchroniser's code (it sent the close request right + after every create and on every status push) and from the vendor close page + (tier 1): that page lists four processing steps, none conditional on the + status sent — the workflow is interrupted, the status is set, the unfinished + actions are deleted (or, by our reading, ended) and an anticipated closing + action is inserted + ([vendor close page](https://docs.easyvista.com/docs/rest-api-close-an-incident-request.md)). + The drain of the open action across such a status write was measured on + 2026-09-01 on one instance (one ticket censused, target status id 24 there; + tier 4, so it may not generalise). The page documents *final* statuses only, + so for a non-final one it is an extrapolation the page neither exempts nor + covers. Writes that may change the + workflow are refused unless the call passes `allow_workflow_effect=`; the + refusal is `EasyvistaWorkflowEffectRefused`, a `ValueError` (not an + `EasyvistaError`), raised before any request, and an allowed workflow write is + sent once, never retried. +- **`update_ticket` cannot set a status either, and refuses the attempt.** A + `status_id`, `status_guid`, catalog or `parent_request_id` key smuggled in + through `extra_payload` raises `EasyvistaWorkflowEffectRefused` unless you + opt in with `allow_workflow_effect=`. The vendor's + [update-an-incident-request page](https://docs.easyvista.com/docs/rest-api-update-an-incident-request.md) + excludes `status_id`, `sd_catalog_id`, `initial_sd_catalog_id` and + `parent_request_id` from its body outright (tier 1, read 2026-10-02), so an + opt-in is permission to *send* it, not evidence the server will honour it; + re-read after any such write. - **Timestamp columns are aware `datetime`, so a record dump is not JSON-serialisable.** `submit_date_ut`, `creation_date_ut`, `max_resolution_date_ut`, `expected_date_ut`, `end_date_ut` and `last_update`