Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

[Changelog CI] Add Changelog for Version 0.1.4#3

Closed
github-actions[bot] wants to merge 1 commit into
mainfrom
changelog-ci-0.1.4-1725657462
Closed

[Changelog CI] Add Changelog for Version 0.1.4#3
github-actions[bot] wants to merge 1 commit into
mainfrom
changelog-ci-0.1.4-1725657462

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 6, 2024

Copy link
Copy Markdown
Contributor

Version: 0.1.4

  • #1: Create openregister.csr

rjzondervan added a commit that referenced this pull request May 21, 2026
…-side; surface root cause on rollback (#1593)

**Root cause of the operator-reported 500:**
The `oc_openregister_entities` table has TWO `NOT NULL`-with-no-default
columns (`category` and `updated_at`) that the existing detector flow
(`EntityRecognitionHandler::findOrCreateEntity`) always populates. The
new manual-entity flow only populated `detected_at`, so the insert
failed at the DB layer with two consecutive `SQLSTATE[HY000] 1364
Field 'X' doesn't have a default value` errors — one per column.

**Fix #1 — `category` is now server-derived, removed from the API:**
- `EntityRecognitionHandler::getCategoryForType()` is now
  `public static` (was `private`) so producers outside the detector
  pipeline can derive the same mapping without taking on the
  handler's DI graph.
- `ManualEntityService::lookupOrCreateEntity()` calls it on every new
  insert. Rationale: the operator UI rarely has the context to make a
  category decision, the detector flow already derives consistently,
  and exposing the field as operator-input was the original source of
  the bug (omitted = null = NOT NULL violation). Manual-entity rows
  and detector rows now share categories for identical types.
- `category` removed from the request body, the service signature, the
  `writeAuditTrails` signature, the controller body-parsing, and the
  unit tests. The audit trail still records `category` in
  `changed.fields` — read from the entity after insert, so the
  persisted value is captured verbatim.
- Operator-override on category is intentionally NOT exposed in v1; a
  follow-up if a concrete use case emerges.

**Fix #2 — `updated_at` populated alongside `detected_at`:**
- Same root cause class. Mirroring the detector flow's column
  population fixes it.

**Fix #3 — operator-debuggable rollback path:**
The service's `catch (Throwable $error)` block wrapped the original
exception as `ManualEntityException(REASON_INTERNAL_ERROR)` without
logging. Combined with the controller's `mapManualEntityException`
only logging the reason code (not the previous-exception message), a
500 left no debuggable trail in `nextcloud.log` — exactly what the
operator hit. Added a `LoggerInterface::error()` call inside the
service's rollback handler that captures `errorType`, `error`, and
`trace` of the underlying Throwable before re-wrapping. Future
operator-reported 500s will have a stack-trace-level log entry
keyed by reqId.

**Spec + docs updates:**
- `docs/Features/manual-entity-anonymisation.md` — request body shape
  loses the `category` line; added a note explaining the server-side
  derivation.
- `openspec/changes/manual-entity-anonymisation/proposal.md` — example
  body updated; the 400 error hint updated.
- `openspec/changes/manual-entity-anonymisation/design.md` — derivation
  rationale folded into the existing "lookup-or-create" section; the
  open-question on category resolved with a "not in the API at all"
  decision.
- `openspec/changes/manual-entity-anonymisation/specs/entity-relation-grondslagen/spec.md`
  — Requirement updated, "Category mismatch does NOT trigger an
  update" scenario replaced with "New catalogue row carries the
  server-derived category".
- `openspec validate manual-entity-anonymisation` clean.

**Verification:**
- PHPCS clean (4 touched files including the new logger call).
- PHPStan clean on the full diff.
- 44 unit tests / 145 assertions across
  `ManualEntityServiceTest|FileTextControllerManualEntityTest|ChunkTextMatcherTest|GdprEntityMapperTest|EntityRelationMapperTest`
  all green in the live `master-nextcloud-1` container.
- End-to-end smoke test in the dev environment:
  - Empty-match POST → HTTP 200 with `message`, catalogue row created.
  - Match POST → HTTP 201 with `matchCount=4, matchesSkipped=2` (the
    2 skipped were pre-existing detector hits at the same positions).
  - Idempotent re-POST → HTTP 201 with `matchCount=4, matchesSkipped=4`,
    `relations: []` (no duplicates).

Refs: #1593
rjzondervan added a commit that referenced this pull request May 26, 2026
…-side; surface root cause on rollback (#1593)

**Root cause of the operator-reported 500:**
The `oc_openregister_entities` table has TWO `NOT NULL`-with-no-default
columns (`category` and `updated_at`) that the existing detector flow
(`EntityRecognitionHandler::findOrCreateEntity`) always populates. The
new manual-entity flow only populated `detected_at`, so the insert
failed at the DB layer with two consecutive `SQLSTATE[HY000] 1364
Field 'X' doesn't have a default value` errors — one per column.

**Fix #1 — `category` is now server-derived, removed from the API:**
- `EntityRecognitionHandler::getCategoryForType()` is now
  `public static` (was `private`) so producers outside the detector
  pipeline can derive the same mapping without taking on the
  handler's DI graph.
- `ManualEntityService::lookupOrCreateEntity()` calls it on every new
  insert. Rationale: the operator UI rarely has the context to make a
  category decision, the detector flow already derives consistently,
  and exposing the field as operator-input was the original source of
  the bug (omitted = null = NOT NULL violation). Manual-entity rows
  and detector rows now share categories for identical types.
- `category` removed from the request body, the service signature, the
  `writeAuditTrails` signature, the controller body-parsing, and the
  unit tests. The audit trail still records `category` in
  `changed.fields` — read from the entity after insert, so the
  persisted value is captured verbatim.
- Operator-override on category is intentionally NOT exposed in v1; a
  follow-up if a concrete use case emerges.

**Fix #2 — `updated_at` populated alongside `detected_at`:**
- Same root cause class. Mirroring the detector flow's column
  population fixes it.

**Fix #3 — operator-debuggable rollback path:**
The service's `catch (Throwable $error)` block wrapped the original
exception as `ManualEntityException(REASON_INTERNAL_ERROR)` without
logging. Combined with the controller's `mapManualEntityException`
only logging the reason code (not the previous-exception message), a
500 left no debuggable trail in `nextcloud.log` — exactly what the
operator hit. Added a `LoggerInterface::error()` call inside the
service's rollback handler that captures `errorType`, `error`, and
`trace` of the underlying Throwable before re-wrapping. Future
operator-reported 500s will have a stack-trace-level log entry
keyed by reqId.

**Spec + docs updates:**
- `docs/Features/manual-entity-anonymisation.md` — request body shape
  loses the `category` line; added a note explaining the server-side
  derivation.
- `openspec/changes/manual-entity-anonymisation/proposal.md` — example
  body updated; the 400 error hint updated.
- `openspec/changes/manual-entity-anonymisation/design.md` — derivation
  rationale folded into the existing "lookup-or-create" section; the
  open-question on category resolved with a "not in the API at all"
  decision.
- `openspec/changes/manual-entity-anonymisation/specs/entity-relation-grondslagen/spec.md`
  — Requirement updated, "Category mismatch does NOT trigger an
  update" scenario replaced with "New catalogue row carries the
  server-derived category".
- `openspec validate manual-entity-anonymisation` clean.

**Verification:**
- PHPCS clean (4 touched files including the new logger call).
- PHPStan clean on the full diff.
- 44 unit tests / 145 assertions across
  `ManualEntityServiceTest|FileTextControllerManualEntityTest|ChunkTextMatcherTest|GdprEntityMapperTest|EntityRelationMapperTest`
  all green in the live `master-nextcloud-1` container.
- End-to-end smoke test in the dev environment:
  - Empty-match POST → HTTP 200 with `message`, catalogue row created.
  - Match POST → HTTP 201 with `matchCount=4, matchesSkipped=2` (the
    2 skipped were pre-existing detector hits at the same positions).
  - Idempotent re-POST → HTTP 201 with `matchCount=4, matchesSkipped=4`,
    `relations: []` (no duplicates).

Refs: #1593
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant