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

Development#1399

Merged
remko48 merged 54 commits into
betafrom
development
May 1, 2026
Merged

Development#1399
remko48 merged 54 commits into
betafrom
development

Conversation

@remko48

@remko48 remko48 commented May 1, 2026

Copy link
Copy Markdown
Member

No description provided.

remko48 and others added 30 commits April 24, 2026 10:45
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…apability refs)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…task refs)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onManager integration)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ethods

Adds REQ-017 to content-versioning spec describing the FileVersioningHandler's
observed behavior: listing and restoring file versions via NC files_versions app,
with graceful degradation when the app is disabled.

Ghost change: retrofit-content-versioning-2026-04-28 (archived)
Methods annotated: FileVersioningHandler::isVersioningEnabled, listVersions,
restoreVersion, getCurrentUserId
Clusters annotated (237 PHP methods total):
- archival-destruction-workflow: 10 methods → task-5, task-6
- graphql-api: 47 methods → task-37, task-38, task-39, task-40, task-41
- edepot-transfer: 37 methods → task-20, task-21, task-23
- event-driven-architecture: 97 methods → task-26, task-27
- faceting-configuration: 48 methods → task-29, task-31, task-33, task-35

Coverage report refreshed on branch retrofit/reverse-spec-openregister-tenant-lifecycle-2026-04-28 (2026-04-28).
New capability spec: openspec/specs/object-lifecycle/spec.md
REQ-001: Object save pipeline (SaveObjects, SaveObject, CrudHandler, etc.)
REQ-002: Object validation pipeline (ValidateObject, ValidationHandler)
REQ-003: Object cache management (CacheHandler, PerformanceHandler)
REQ-004: Bulk object operations (PreparationHandler, ChunkProcessingHandler)
REQ-005: Object metadata hydration (MetadataHydrationHandler, ComputedFieldHandler)

Cross-cap refs: PermissionHandler→task-7, LinkedEntityEnricher/RelationHandler→task-6,
AuditHandler→task-8, FacetHandler→task-9, SearchQueryHandler→task-10,
ExportHandler→task-11, RevertHandler→task-12, FilePropertyHandler→task-13

Scanner misclassification: all methods were in object-interactions Bucket 2a
but implement core CRUD pipeline, not the notes/tasks/files API.

Ghost change: retrofit-object-lifecycle-2026-04-28 (archived)
New capability spec: openspec/specs/calendar-integration/spec.md
REQ-001: Register objects exposed as CalDAV calendar events (ICalendarProvider impl)
REQ-002: Object→VEVENT transformation (all-day vs datetime, template interpolation)

Files: RegisterCalendarProvider, RegisterCalendar, CalendarEventTransformer (19 methods)
Ghost change: retrofit-calendar-integration-2026-04-28 (archived)
…chived)

All Bucket 2b clusters annotated with cross-capability @SPEC tags:
- ActivityService, Listeners, EventListeners → event-driven-architecture tasks
- ObjectReferenceProvider → deep-link-registry#REQ-002
- N8nAdapter, WorkflowResult, ActionScheduleJob → workflow-engine-abstraction tasks
- BulkLegalHoldJob, DestructionCheckJob → archival-destruction-workflow tasks
- BlobMigrationJob → data-import-export#REQ-001
- SolrNightlyWarmupJob, ObjectsProvider, SolrCommands → zoeken-filteren#REQ-001
- GraphQLSubscriptionListener → graphql-api#REQ-007
- TransferCheckJob → edepot-transfer#REQ-001
- WebhookRetryJob → webhook-payload-mapping#REQ-001
- SyncConfigurationsJob → faceting-configuration#REQ-002
- LogCleanUpTask → retention-management#REQ-009
- OrganisationController → tenant-lifecycle#REQ-001
- TenantQuotaException, LanguageMiddleware → tenant-quotas#REQ-001
- ConfigurationCheckJob → notificatie-engine
- ActionListener, FileChangeListener → event-driven-architecture
- FileExtractionController, ObjectTextExtractionJob → object-lifecycle
- Exception, AppInfo, Sections, Settings → object-lifecycle
- AuthenticationService → rbac-scopes#REQ-001
- Twig, ContactsMenuProvider → object-lifecycle
- SchemaTool, RegisterTool, ToolInterface → object-lifecycle (LLM tool layer)

Ghost change: retrofit-b2b-crossrefs-2026-04-28 (archived)
Completes the bucket-2b-cross-ref ghost change annotation pass.
Methods in AppInfo, BackgroundJob, Command, Contacts, Controller,
Exception, Listener, Repair, Sections, Service, Settings, and Twig
namespaces annotated with @SPEC tags pointing to existing spec tasks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CacheWarmupJob + NameCacheWarmupJob → object-lifecycle#REQ-003 (task-30)
SolrWarmupJob → zoeken-filteren#REQ-001 (task-31)
ActionRetryJob + ExecutionHistoryCleanupJob → workflow-engine-abstraction#REQ-010 (task-32)
TransferExecutionJob → edepot-transfer#REQ-001 (task-33)

Extends retrofit-b2b-crossrefs-2026-04-28 with 4 new tasks.
PHPCS clean on all 6 files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- .git-blame-ignore-revs: expand 7 truncated SHAs to full 40-char form;
  truncated SHAs are silently ignored by git blame --ignore-revs-file.
  Update section comment to reflect full Bucket 2a/2b pass scope.

- lib/Event/*.php + lib/Service/Configuration/*.php: fix 87 @SPEC
  annotations that were missing a blank comment line before the tag and
  used triple-space (   ) instead of single-space. All now follow the
  established convention used in the other 1,075+ annotations.

- openspec/specs/object-lifecycle/spec.md (live + archive copy):
  - REQ-001: rewrite pipeline description to accurately reflect two
    distinct paths (single: SaveObject::saveObject(); bulk:
    SaveObjects::saveObjects() → SaveObject per item). Update scenario
    WHEN clause from the non-existent CrudHandler::save() to the real
    SaveObject::saveObject() entry point.
  - REQ-005 scenario: fix UUIv4 typo → UUIDv4.

- openspec/archive/retrofit-calendar-integration-2026-04-28/proposal.md:
  correct CalendarEventTransformer method count from 7 to 5 (matches the
  5 annotated methods listed in the parenthetical).

- openspec/archive/retrofit-notificatie-engine-2026-04-28/tasks.md:
  add missing #REQ-001 anchor to task-1 for consistency with all other
  tasks.md files in the PR.

- openspec/coverage-report.json + coverage-report.md:
  restore scope block / ## Scope table removed during report regeneration.
  Updated counts: spec_capabilities=45, spec_reqs_total=393 (reflecting
  new object-lifecycle, calendar-integration, content-versioning REQ-017,
  and tenant-lifecycle REQ-005). PHP/frontend counts unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…register-tenant-lifecycle-2026-04-28

retrofit: Bucket 2a/2b reverse-spec + annotate openregister (16 commits / ~380 methods)
…on-based-on-schema

Added authentication based on schema with condictional settings
  BREAKING CHANGE: @self.files no longer contains full file metadata by
  default. Default is now a lightweight list of integer file IDs (e.g.
  [123, 456, 789]). Add _extend[]=@self.files (or the equivalent shorthand
  _extend[]=_files) to receive the full metadata payload. Migration is one
  query parameter on the consumer side.

  Why: today, show endpoints that route through RenderObject::renderEntity
  unconditionally pay the FileMapper + SystemTag lookup cost on every
  response (even when the consumer never uses the file information), while
  list endpoints that bypass renderEntity emit no @self.files field at
  all. Consumers see different shapes across endpoints and cannot opt in
  or out. The new contract is symmetric and explicit, and the lightweight
  default is strictly cheaper than today's show behavior.

  What changed
  - FileMapper::getFileIdsForObjects(string[] $uuids): new batched lookup
    (two queries regardless of input size) backing the lightweight default.
  - RenderObject::renderEntity now gates renderFiles() on whether _extend
    covers @self.files. Body of renderFiles() is unchanged. When extend is
    absent, setLightweightFileIds() attaches the integer ID list, reusing a
    transient batch cache populated by renderEntities() so list pages do not
    go N+1.
  - normalizeMap gains '_files' => '@self.files' so both spellings are
    equivalent, mirroring the existing _schema/_register normalization.
  - QueryHandler::searchObjectsPaginatedDatabase enriches cheap-path rows
    (no extend / fields / filter / propAuth) with lightweight IDs via a
    single batched query before returning. Complex-rendering rows inherit
    the new behavior automatically through renderEntities.
  - ObjectService::searchObjectsPaginated applies the same enrichment on
    the SOLR/index branch. (SOLR + _extend[]=@self.files falls back to IDs;
    pre-existing limitation that the SOLR path does not route rows through
    renderEntities, documented inline.)
  - attachLightweightFilesToRows() handles both ObjectEntity and array row
    shapes for tolerance across pipelines.

  Performance
  - Default show: strictly cheaper (no file metadata work).
  - Default list: +1 batched query per request, regardless of page size.
  - Extend on show: unchanged.
  - Extend on list: per-row file/tag lookups; documented as heavily
    discouraged with a verbatim warning in docs/api/objects.md.

  Tests
  - Unit tests for FileMapper::getFileIdsForObjects input validation.
  - Integration tests deferred to a follow-up (Newman fixture with
    attachments).

  Out of scope: renderFileProperties (schema-property file hydration),
  PublicationsController::attachments() (separate explicit endpoint), and
  opencatalogi documentation (docs-only follow-up against opencatalogi).

  OpenSpec change: openspec/changes/opt-in-files-extend
  Refs #1358

  A few options if you want it tighter:

  - One-liner subject only (matches several of your recent commits): feat!: opt-in @self.files via _extend
  on rendered objects
  - Drop the ! and use the feat: prefix only if you don't follow conventional-commits strictly here — the
  BREAKING CHANGE: block in the body still surfaces the semver impact.
  - Trim the body to just the BREAKING CHANGE block + Refs line if you prefer pull-request descriptions to
  carry the detail and commit messages to stay terse.

 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Conflict in PermissionHandler.php resolved: evaluateMatchConditions() was
removed in development; kept development version with @SPEC annotation on
getAuthorizedGroups().
Completes the extend → allOf migration so creating an extended schema,
saving it, and adding objects against it all work correctly.

schema.types.ts / EditSchema.vue / SchemaMapper.php (in-flight):
  Replace deprecated single-parent `extend?: string` with JSON-Schema-
  standard composition fields `allOf?/oneOf?/anyOf?: string[]`. Update
  the extend-schema button to emit `allOf: [parentId]` and clean the
  PHP doc comment that still referenced `extend`.

src/entities/schema/schema.ts:
  The TypeScript Schema entity dropped allOf/oneOf/anyOf in its
  constructor, so `setSchemaItem(new Schema(payload))` stripped the
  parent reference before it ever reached the API. Add the three fields
  to the class and copy them through.

src/modals/object/ViewObject.vue:
  When adding an object against a composed schema, the form was empty
  because `schemaStore.schemaItem` came from the schema list endpoint,
  which does not run resolveSchemaExtension and therefore returns empty
  `properties`. Refetch the active schema by id (mounted, and in the
  currentSchema watcher) when it has allOf/oneOf/anyOf but no resolved
  properties — the detail endpoint merges parent properties in. A
  guard on (composition refs && propsCount === 0) prevents recursion
  once the resolved version arrives.
…ethods

Adds pre-destruction notification REQ (objects approaching archiefactiedatum
trigger per-object Nextcloud notifications to archivaris group, configurable
lead time, deduplication via app config). Ghost change archived.

Annotates 11 Bucket 2a methods with @SPEC refs:
- DestructionCheckJob::run, sendObjectNotification → REQ-009 task
- LegalHoldService: placeHold, releaseHold, hasActiveHold,
  hasActiveHoldFromRetention, bulkPlaceHold → REQ-006 (task-5)
- ArchivalController: createLegalHold, releaseLegalHold → REQ-006 (task-5)
- ArchivalService: calculateArchivalDate → REQ-007 (task-6)
- ArchivalService: findObjectsDueForDestruction → REQ-001 (task-1)

9 private/plumbing methods in cluster left unannotated (by design).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gnore-revs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Q-NNN format

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
destroyMultiple() was calling logService->deleteLogs() and returning 200,
while its sibling destroy() correctly returned 405 Method Not Allowed.
Both now enforce the audit-trail-immutable spec: audit trail entries
cannot be deleted via any route.

Also refresh coverage-report.md/.json from 2026-04-30 rescan.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WilcoLouwerse and others added 23 commits April 30, 2026 14:19
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cleanup of leftover deletions from the archive move.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New capability spec openspec/specs/chat-ai/spec.md with 5 REQs drafted
from observed behavior of ChatController, ChatService, ConversationController,
and AgentsController. Ghost change retrofit-chat-ai-2026-04-30 archived.

REQs:
- REQ-001: AI message processing with RAG context
- REQ-002: Conversation lifecycle management (two-stage delete)
- REQ-003: Paginated message history retrieval
- REQ-004: AI agent management and tool discovery
- REQ-005: Chat analytics and user feedback

Annotations: 22 methods across 4 files tagged with @SPEC pointing at
openspec/changes/archive/retrofit-chat-ai-2026-04-30/tasks.md.
No logic changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ster-2026-04-30

retrofit: annotate openregister Bucket 1 (169 methods / 96 REQs)
OpenRegister's magic-table read paths returned object-property values
whose runtime PHP types did not match the JSON-Schema-declared property
type. Two specific defects in MagicStatisticsHandler::convertRowToObjectEntity:

- The unconditional `is_string($value) && isJsonString($value)` decode
  block at lines ~593–600 ran on every property regardless of schema
  type. Once the preceding cast normalised a numeric DB value to
  string ("123"), this block decoded it right back to int 123. Same
  block silently turned literal strings "true" into bool, "null" into
  PHP null, and stripped quotes from escaped strings.
- Only `string` was coerced. `boolean`, `integer`, `number`, `array`,
  and `object` properties returned whatever the driver produced — which
  on MariaDB means TINYINT(1) booleans came back as int 0/1.

The search-side handler already had a complete schema-driven converter
(`MagicSearchHandler::convertValueByType`). Extracted that into a new
shared `lib/Service/Object/SchemaTypeConverter.php` and routed both
read paths through it:

- New SchemaTypeConverter — stateless service, single public
  `convertValue(value, schemaType)`. Mirrors the proven search-side
  semantics for all six JSON-Schema primitive types. The string branch
  retains the historical compatibility window (decode strings starting
  with `[` or `{` that parse) so schemas with mismatched type
  declarations continue to work; scalar JSON literals are NOT decoded.
- MagicStatisticsHandler — replaces the partial coercion + over-eager
  decode with a single converter call. DateTimeNormalizer format
  handling stays where it is (per design D5). `isJsonString` helper
  removed (no remaining call sites).
- MagicSearchHandler — replaces the inline `convertValueByType` call
  with the converter. Six private converter methods deleted.
- MagicMapper handler instantiation now passes the converter via the
  app container (auto-wired, zero deps).

Endpoint coverage matrix (every read returning an ObjectEntity now
flows through the unified converter):

  GET  /api/objects/<uuid>          MagicStatisticsHandler             ✓
  GET  /api/objects (list)          MagicStatisticsHandler             ✓
  GET  /api/search                  MagicSearchHandler                 ✓
  POST /api/objects                 re-fetch via Statistics handler    ✓
  PUT  /api/objects/<uuid>          re-fetch via Statistics handler    ✓

Boolean coercion intentionally rejects the HTML form literal `'on'` —
form normalisation belongs in the controller layer (design D7).
Inline private methods on MagicSearchHandler are deleted, not kept as
deprecation shims, since they had no external consumers (design D8).

Tests:
- New SchemaTypeConverterTest — 55 cases across 8 test methods covering
  every schema type plus the regression suite (numeric-shaped string,
  "true"/"null" literals, quoted-string JSON, MariaDB int 0/1 booleans,
  array-shaped string compat, null preservation, unknown-type fallback).
- Existing MagicStatisticsHandlerTest and MagicSearchHandlerTest
  constructors updated for the new dependency. Date-format ordering
  regression in `testValidDateTimePropertyRendersAsIso8601` confirms
  the converter→DateTimeNormalizer ordering still holds.

Quality: 69/69 unit tests pass, 49/49 magic-mapper tests pass. PHPCS,
PHPMD, and `php -l` clean on touched lib files.

Spec: openspec/changes/fix-magic-table-type-coercion/ — proposal,
design (8 decisions), specs/schema-driven-read-coercion/ (9
requirements), tasks.md, plan.json. Long-lived spec stub in
openspec/specs/schema-driven-read-coercion/.

Refs: #1366
Merged both retrofit annotation SHAs from feature/reverse-spec and
retrofit/reverse-spec-openregister-chat-ai-2026-04-30.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…register-chat-ai-2026-04-30

retrofit: reverse-spec chat-ai cluster (5 REQs / 22 methods)
content-versioning: 4 requirements marked deferred
- draft/published lifecycle (no DraftService found)
- draft promotion (depends on lifecycle above)
- version comparison with visual diffs (no diffing service)
- delta storage for drafts (audit trail full diffs work; draft delta does not)

environment-otap: configuration promotion endpoint not found
- POST /api/organisations/{uuid}/promote not implemented
- environment field + validation IS implemented (TenantLifecycleService)

mock-registers: idempotent import not implemented
- no MockRegisterService or mock seed JSON files found

Source: openspec/coverage-report.md Bucket 3b, generated 2026-04-30.
Fix: schema-driven type coercion on magic-table read paths
Blockers / concerns from @WilcoLouwerse 2026-04-30:

- 🔴 setLightweightFileIds cache-miss returned [] for sub-entities
  (RenderObject.php). The batchFileIdsCache is populated by
  renderEntities() with TOP-LEVEL page UUIDs only; sub-entities reached
  via relation extends are absent from it. Previous `?? []` short-circuit
  silently emitted `@self.files: []` on every linked sub-object that
  had attached files. Fix: use `array_key_exists` to distinguish "in
  the batch with zero files" from "not in the batch at all"; on miss,
  fall through to a single-UUID FileMapper lookup. Top-level N+1
  prevention is preserved.

- 🟡 'all' token removed from shouldExtendFiles. `_extend[]=all`
  propagates to sub-entity renders via array_merge(['all'], $keyExtends),
  which would silently pay full file-metadata cost on every linked
  sub-object whenever a top-level request used the wildcard. The
  proposal pins @self.files as a strict explicit opt-in via
  @self.files / _files only — the wildcard exception was not in spec.

- 🟡 SOLR-path extend_unsupported signal added (ObjectService.php).
  `_extend[]=@self.files` (or `_files`) on a SOLR-backed list endpoint
  cannot deliver full metadata (the SOLR path does not route rows
  through renderEntities). Response now carries
  `@self.extend_unsupported: ['@self.files']` so consumers can detect
  the shape mismatch programmatically. Also documented in CHANGELOG.

- 🟡 Render-contract gate tests added (RenderObjectTest.php). Five
  new test methods cover (a) shouldExtendFiles dispatch including the
  intentional 'all' rejection, (b) lightweight default fires
  getFileIdsForObjects but NOT getFilesForObject when extend is empty,
  (c) explicit opt-in routes through renderFiles, (d) cache-miss
  fall-through (regression test for the blocker), (e) cache-hit
  preserves cached value (no spurious FileMapper calls). The five
  pre-existing testRenderFiles* methods were updated to pass
  `_extend: ['@self.files']` — they exercise the heavy renderFiles
  path and were silently going through the lightweight path after the
  gate landed. Was the source of the PHPUnit ❌ in CI.

- 🟢 FileMapperGetFileIdsForObjectsTest: deliberate off-contract input
  test annotated with @psalm-suppress InvalidArgument so static
  analysis doesn't flag the contract-violating call site. The guard
  is intentional defence against sloppy callers.

- 🟢 CHANGELOG: SOLR limitation explicitly mentioned alongside the
  perf-discouragement note for list+extend.

Quality: 312/312 tests pass in RenderObjectTest, +5 new gate tests.
PHPCS clean on touched lib files. php -l clean on all touched files.
feat!: opt-in @self.files via _extend on rendered objects
retrofit: full Bucket 1 + 2a/2b/3b spec coverage for openregister (~500 methods annotated, 6 new specs)
…asset only) (#1386)

The central Quality workflow (ConductionNL/.github#34) now publishes SBOMs
exclusively as release assets — see SECURITY.md "Software Bill of Materials".

This PR cleans up the per-app remnants:
- delete .github/workflows/sbom.yml (the central job replaces it)
- delete the checked-in sbom.cdx.json (release asset is the source of truth)
- gitignore SBOM files so future generations don't accidentally land in repo

Stable URL for clients:
  https://github.com/ConductionNL/openregister/releases/latest/download/sbom.cdx.json

Co-authored-by: SBOM Cleanup <ops@conduction.nl>
…ame) (#1388)

Per CONVENTIONS.md (ConductionNL/.github#35). The previous filename
collided with the central reusable workflow's name (also quality.yml in
.github/workflows/), which was a recurring source of confusion when
searching across repos. Pure file rename; the workflow's 'name:' field
is unchanged so branch-protection required-status-check identity is
preserved.

Co-authored-by: convention-cleanup <ops@conduction.nl>
)

Updates 8 active-spec references from `.github/workflows/quality.yml`
to `.github/workflows/code-quality.yml` per the canonical filename
established by ConductionNL/.github CONVENTIONS.md (#35).

Files touched (live specs only — archived spec copies under
openspec/changes/archive/ are intentionally NOT modified to preserve
historical accuracy):
- openspec/specs/mariadb-ci-matrix/spec.md (5 refs)
- openspec/changes/api-test-coverage/specs/api-test-coverage/spec.md (3 refs)

The requirement title at line 95 ("Matrix Strategy Configuration in
quality.yml") is intentionally preserved — that string is the requirement's
addressable identity and renaming it would break any cross-references.

Co-authored-by: convention-cleanup <ops@conduction.nl>
…-properties

fix(GIT-1358): Fix extend-schema property visibility and allOf composition
@WilcoLouwerse

Copy link
Copy Markdown
Contributor

🟡 Missing PR description

This PR makes non-trivial changes but has no description. Please add a description covering:

  • What changed and why
  • Any behavioral differences reviewers should know about
  • Links to related issues, specs, or prior art

A reviewer cannot confidently verify intent against the diff without this context.

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 4142b30

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 147/147
npm ✅ 598/598
PHPUnit
Newman
Playwright ⏭️

Quality workflow — 2026-05-01 13:40 UTC

Download the full PDF report from the workflow artifacts.

Comment thread lib/Service/Object/RenderObject.php
Comment thread src/modals/object/ViewObject.vue
Comment thread lib/Db/FileMapper.php

@WilcoLouwerse WilcoLouwerse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 blocker + 3 concerns require fixes before this development→beta promotion.

Blocker: Newman integration tests are failing at head SHA. The feat! commit explicitly defers test fixture updates to a follow-up, but a development→beta merge should not land with knowingly failing CI. Update the Newman fixtures for the new @self.files shape first.

Concerns (inline):

What checks out: SchemaTypeConverter extraction is clean, AuditTrailController::destroyMultiple() correctly enforces immutability (405), FileMapper::getFileIdsForObjects() correctly issues two queries regardless of batch size, and the batchFileIdsCache try/finally lifetime is safe.

@WilcoLouwerse WilcoLouwerse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving after author confirmed the Newman test fixture updates are intentionally deferred to a follow-up PR. The three concerns from the inline comments remain open for that follow-up:

@remko48 remko48 merged commit 5bb5e01 into beta May 1, 2026
23 of 44 checks passed
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.

4 participants