Skip to content

API: publish, unpublish and delete go through the table action service (#2569) - #2588

Merged
jh-RLI merged 1 commit into
developfrom
feature-2569-api-actions
Oct 2, 2026
Merged

jh-RLI merged 1 commit into
developfrom
feature-2569-api-actions

Conversation

@jh-RLI

@jh-RLI jh-RLI commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Closes #2569. Slice 17 of spec #2551.

The single-table API endpoints for publish (tables/<t>/move_publish/<topic>/), unpublish (tables/<t>/unpublish/) and delete (DELETE tables/<t>/) now call api/services/table_actions.py, the same path the tables tab's ⋯ menu uses. There is no second path. Request and response bodies stay as they were ({}, or {"reason": …} on a refusal). The committed openapi.yaml is unchanged (api.tests.test_openapi_schema green, no regeneration).

What changes for API clients

  • Publish is atomic. Before, an unknown topic set the embargo and moved the peer reviews, then failed with a generic 400 "Invalid request". Now it is a 400 naming the topic, and nothing is written.
  • draft is refused as a topic (400). Before, it published under the pseudo-topic.
  • An unknown embargo duration is refused (400). Before, it was ignored and the table was published anyway.
  • A missing open data license is still a 400. Its reason now uses the tables tab's wording ("Fails the Publish gate: License"). The old message's detail about which license problem is gone (see open point 3).
  • A failed OEDB drop on delete is a 500 naming the table. Before, the record was deleted and the client got 400 "Invalid request".
  • Every call logs one table_action … via=api line on oeplatform.table_actions.

All of this is in the changelog under "Changes".

What deliberately does not change

The issue's acceptance criterion "existing API tests stay green" decides these. TestMovePublish publishes the same table twice and expects 200 both times.

  • A published table can be published again (one more topic, embargo as given). The service leaves published tables out of a dashboard publish; the API passes the new publish parameter republish=True. The dashboard view never forwards it (its PARAMS whitelist), so the bulk rule is untouched.
  • An omitted embargo leaves the existing one as it is, as move_publish always did. The service's default is none, which lifts an embargo. Applied to the API, that would have silently ended an embargo on any re-publish that didn't repeat it. New value KEEP_EMBARGO = "keep", sent only by the API. _done_message tolerates it.
  • Unpublishing a draft answers 200 with nothing written (no log line). The API maps the service's "Not published" refusal to success, because the requested state already holds.
  • A published table is deleted without typed confirmation. The view passes the name from the address as confirm. That is exactly what the dialog asks for one published table. Whether the API should guard this is out of scope (spec, "Not changed").
  • The permission decorators still run first, so 401/403/404 and their bodies are unchanged.

How refusals map

table_action() in api/views.py:

service API
InvalidParameters 400, its message
ActionRefused with a role reason 403 "Permission denied"
ActionRefused "Not one of your tables", table gone 404 "Table does not exist"
ActionRefused "Not one of your tables", table there 403 "Permission denied"
any other ActionRefused 400, "Nothing was changed: …"

The decorators have already checked existence and role, so the first three only happen if something changed between that check and the service's row lock.

Delete and transactions

The service runs delete in a durable transaction, so it refuses to run nested. The view opens no atomic block, and ATOMIC_REQUESTS is set nowhere. test_the_view_does_not_run_the_service_inside_a_transaction checks this directly: Django's TestCase disables the durability check, so the test looks for any atomic block that isn't the test case's own. Table.delete() stays for its other callers.

Tests

api/tests/test_table_actions_api.py, 20 tests through the test client over the real endpoints:

  • publish: atomicity on an unknown topic, draft refused, bad embargo refused, gate refusal, embargo kept when omitted, re-publish adds a topic, via=api log line
  • unpublish: idempotent on a draft, log line
  • delete: published table, failed drop → 500, no surrounding transaction, log line
  • the refusal mapping

Existing TestMovePublish, TestDelete, test_publish_gate and test_modification_stamps are green. So are the dashboard's login.tests.test_table_actions, test_table_delete and test_table_dataset_actions (84). Full suite: 1,370 tests, the only failure the known LightImportTest under isolated settings, which passes on its own. It ran against a private OEDB built with alembic upgrade head, because two other sessions' runs were deadlocked in the shared sandbox.

No migration, no deploy step. No UI in this slice.

Open points for the maintainer

  1. republish and KEEP_EMBARGO are API-only behaviours inside the shared service. I chose explicit parameters over a check on via=, so the rule reads off the call. The alternative was to refuse re-publish via the API too. That breaks TestMovePublish and removes the API's only way to add a topic or change an embargo.
  2. A failed drop answers 500, which openapi.yaml doesn't declare. Declaring it would change the artifact, which this slice must not do. Before, the same case was an undeclared 400.
  3. The license refusal lost its detail ("which license problem"). The service groups left-out tables by reason text, so putting a per-table detail in that text would split the dashboard's groups. If API clients need it, it belongs in a field of its own.
  4. Tables tab: selection, bulk bar and bulk publish/unpublish #2564 (PR Tables tab: selection, bulk bar, bulk publish and unpublish #2586) and API: manage a Table's Holders through the REST API #2570 (PR Manage a Table's Holders through the REST API (#2570) #2587) both trial-merge cleanly with this branch. Tables tab: selection, bulk bar and bulk publish/unpublish #2564 edits the same _check; the merged result keeps both.
  5. Out of scope, as the issue says: whether the API should refuse deleting a published table, and the dataset assign/unassign API (the service leaves out tables the user holds no role on, unlike the API's curation model, "What Tables tab: add a Table to or remove it from one of my Datasets #2563 left").

🤖 Generated with Claude Code

#2569)

The single-table endpoints move_publish/<topic>/, unpublish/ and the
table DELETE call api.services.table_actions, the path the tables tab
takes, instead of a second one. Request and response bodies stay as they
were ({} / {"reason": ...}) and the committed openapi.yaml is unchanged.

- Publishing is atomic: an unknown topic no longer sets the embargo and
  moves peer reviews before failing. `draft` is refused as a topic, and
  an embargo duration other than none/6_months/1_year is a 400 instead of
  being ignored.
- What the API always did still holds: a published table is published
  again under another topic (`republish`), an omitted embargo leaves the
  existing one (`KEEP_EMBARGO`), unpublishing a draft is a 200, and a
  published table is deleted without the dashboard's typed confirmation
  (the address names it).
- Delete runs outside any transaction (the service's is durable). A
  failed OEDB drop after the record is gone answers 500 naming the table.
- Refusals map onto the API's existing answers: a role lost under the
  lock is 403, a table gone is 404, anything else 400 naming the reason.
- Every action logs `table_action ... via=api`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jh-RLI jh-RLI self-assigned this Oct 2, 2026
@jh-RLI
jh-RLI merged commit d4394c8 into develop Oct 2, 2026
4 of 5 checks passed
@jh-RLI
jh-RLI deleted the feature-2569-api-actions branch October 2, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API: single-table publish, unpublish and delete go through the table action service

1 participant