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

feat: getRegisters with expanded schemas, getFiles Twig function, and mass delete fixes#1445

Merged
rubenvdlinde merged 6 commits into
documentationfrom
fix/port-get-files-to-mapping-runtime
May 27, 2026
Merged

feat: getRegisters with expanded schemas, getFiles Twig function, and mass delete fixes#1445
rubenvdlinde merged 6 commits into
documentationfrom
fix/port-get-files-to-mapping-runtime

Conversation

@bbrands02

@bbrands02 bbrands02 commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR includes three independent fixes, one new feature, and one org-filter bugfix targeting the development branch: a new `getRegisters()` API on `ObjectService`, a ported `getFiles` Twig function for the mapping runtime, two mass-delete flow fixes in the frontend, and a fix for NULL-org mappings being unreachable during synchronization.


Changes

1. `ObjectService::getRegisters()` — registers with expanded schemas

Added a new public method `getRegisters(): array` that returns all registers with their schema objects fully expanded (instead of just IDs).

  • Fetches all registers via `registerMapper->findAll()` with multitenancy disabled
  • For each register, resolves each schema ID to its full `jsonSerialize()` representation
  • Schemas that cannot be found are silently skipped
  • Returns an array of register arrays, each with a `schemas` key containing the resolved schema objects

This enables frontend components (e.g. `EditSynchronization` in OpenConnector) to receive register + schema data in a single call without needing a separate schema lookup per register.

2. Port `getFiles` Twig function to OpenRegister mapping runtime

Ported the `getFiles` Twig function from OpenConnector so that mappings executed within OpenRegister can also access files attached to objects.

  • Added `getFiles` function registration in `MappingExtension`
  • Implemented the runtime method in `MappingRuntime`
  • Registered the runtime loader in `MappingRuntimeLoader`
  • Added necessary wiring in `MappingService`

3. Fix: mass delete in Search/Views tab not executing backend API call

The mass delete action in the Search and Views tabs was not calling the backend delete endpoint, causing objects to appear deleted in the UI but remain in the database on refresh.

  • Fixed the delete handler in `SearchIndex.vue` to correctly call the backend API before updating local state

4. Fix: restore soft-delete confirmation dialog in Search/Views mass delete flow

The soft-delete confirmation dialog (asking whether to permanently delete or soft-delete) was missing from the mass delete flow in the Search and Views tabs, causing all deletes to bypass the choice.

  • Restored the `MassDeleteObject` modal in `SearchIndex.vue`
  • Fixed modal trigger and result handling in `MassDeleteObject.vue` to correctly pass the user's choice back to the caller

5. Fix: `executeMapping()` fails for system mappings during synchronization

Calling `executeMapping('some-slug', ...)` from a Twig mapping template would throw "Did expect one result but found none" for any mapping with `organisation = NULL`, even though the mapping exists in the database.

Root cause: `MappingRuntime` called `MappingMapper::find()` without `includeNullOrg: true`. During synchronization there is no active user session, so `applyOrganisationFilter` fell into `applyNoActiveOrgFilter` and appended `AND (1 = 0)` to the query, making it always return zero rows.

  • Fixed by passing `includeNullOrg: true` in `MappingRuntime::executeMapping()` so NULL-org (shared/system) mappings are always resolvable

Test plan

  • Verify `ObjectService::getRegisters()` returns registers with a `schemas` array containing full schema objects (not just IDs)
  • Confirm `{{ getFiles(objectId) }}` works inside a mapping template executed via the OpenRegister mapping runtime
  • Mass-delete objects from the Search tab — confirm the backend DELETE call fires and objects are removed from the database after refresh
  • Mass-delete from Search/Views tab — confirm the soft-delete/permanent-delete dialog appears and the chosen action is applied correctly
  • Run a synchronization that uses `executeMapping('xxllnc-rolerequestor-to-zgw-rol', ...)` — confirm it resolves correctly and no "Did expect one result but found none" error occurs

🤖 Generated with Claude Code

…apping runtime

OpenConnector delegated its mapping engine to OpenRegister (commit 7855a346),
but the getFiles() Twig function was never ported. Templates using getFiles()
in synchronization mappings would fail with "Unknown function" errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 7ccc256

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

❌ Denied composer licenses

Package Version License
dompdf/dompdf v3.1.5 LGPL-2.1

Quality workflow — 2026-05-08 07:59 UTC

Download the full PDF report from the workflow artifacts.

Two bugs prevented deletion from working on the search page:

1. handleMassDelete opened a custom MassDeleteObject dialog on top of
   CnIndexPage's built-in CnMassDeleteDialog, leaving the built-in
   dialog stuck in loading state. After the custom dialog called
   clearSelectedObjects(), the built-in dialog's items watcher fired
   and cleared the list to "No items selected for deletion".
   Fix: handleMassDelete now calls deleteObjects() directly and feeds
   the result back via setMassDeleteResult() so the built-in dialog
   closes correctly.

2. MassDeleteObject.vue called the non-existent massDeleteObject()
   store method, and read selectedObjects (which selectionPlugin stores
   as ID strings) as full objects — causing .map(obj => obj.id) to
   produce [undefined, …] so no DELETE request ever fired.
   Fix: initializeSelection resolves full objects from searchCollection
   by matching IDs; deleteObject calls deleteObjects(type, ids) with
   the correct type slug and IDs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Disables CnIndexPage's built-in mass delete button/dialog
(:show-mass-delete="false") and instead adds a custom Delete button
via the mass-actions slot. This opens MassDeleteObject.vue directly,
restoring the soft-delete info (retention period, deleted objects link)
that was lost in the previous commit.

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

@SudoThijn SudoThijn 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.

I cannot review the FE of this code since the fetching of registers is broken in this PR.

Can another person confirm this?

Turns out my local nextcloud instance was just broken, and it just so happened that it only broke when switching to your branch. It works now.

SudoThijn
SudoThijn previously approved these changes May 8, 2026
private readonly MappingMapper $mappingMapper,
ICacheFactory $cacheFactory,
private readonly LoggerInterface $logger
private readonly LoggerInterface $logger,

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.

🔴 Blocker — adding FileService as a required constructor arg breaks the existing test suite.

Four test files on this branch still instantiate MappingService, MappingRuntime, and MappingRuntimeLoader with the pre-PR arity. They will throw ArgumentCountError on setUp():

File Line Site
tests/Unit/Service/MappingServiceTest.php 31, 497, 802 new MappingService($mapper, $factory, $logger) (3 args, needs 4)
tests/Unit/Service/MappingServiceCoverageTest.php 53, 522 same
tests/Unit/Twig/MappingRuntimeTest.php 25 new MappingRuntime($service, $mapper) (2 args, needs 3)
tests/Unit/Twig/MappingRuntimeLoaderTest.php 23 new MappingRuntimeLoader($service, $mapper) (2 args, needs 3)

The Newman job in CI doesn't run PHPUnit so this didn't surface there, but composer test locally fails immediately.

Fix — add a mocked FileService in each affected setUp() and pass it as the last arg. For example in MappingRuntimeTest.php:

$this->mappingService = $this->createMock(MappingService::class);
$this->mappingMapper = $this->createMock(MappingMapper::class);
$this->fileService = $this->createMock(FileService::class);
$this->runtime = new MappingRuntime($this->mappingService, $this->mappingMapper, $this->fileService);

*
* @return array The formatted file metadata list.
*/
public function getFiles(string $objectId): array

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.

🟡 Concern — getFiles() has no error handling; an exception aborts the whole Twig render.

FileService::getFiles() and formatFile() can throw (NotFoundException, DoesNotExistException) when the object's folder is missing or the UUID does not resolve. Without a try/catch here, the exception bubbles up through Twig and breaks the entire mapping render — not just the getFiles() call.

The openconnector source has the same shape (no error handling), so this preserves parity. But for a Twig helper consumed inside synchronization templates, a deleted/missing object is a normal-enough condition that a 500-level abort feels excessive.

Options:

  1. Wrap and return [] on failure (and document in the docblock).
  2. Document in the docblock that template authors must guard the call site (e.g. {% if obj.id %}{% set files = getFiles(obj.id) %} {% endif %}).

Either is fine — just decide the contract explicitly rather than inheriting it implicitly.

<template #icon>
<TrashCanOutline :size="20" />
</template>
Delete selected

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.

🟢 Minor — i18n: button label not wrapped in t().

Per ADR-007 (Dutch required), user-facing strings should go through t('openregister', '…'):

{{ t('openregister', 'Delete selected') }}

The sibling row-action labels in this file (Edit, Copy, Delete) have the same omission, so this is a pre-existing pattern, not newly introduced. Worth a follow-up to wrap all four together rather than blocking on it here.

@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 — adding FileService as a required constructor arg breaks 4 existing test files (ArgumentCountError at setUp); PHPUnit isn't in CI here so the failure didn't surface. Also a concern about getFiles() error handling and a minor i18n nit. The Twig wiring, getFiles() body, mass-delete fix, and all 14 mechanical gates check out otherwise.

bbrands02 and others added 2 commits May 15, 2026 13:38
@bbrands02 bbrands02 changed the title fix: port getFiles() Twig function to OpenRegister mapping runtime feat: getRegisters with expanded schemas, getFiles Twig function, and mass delete fixes May 15, 2026
@rubenvdlinde

Copy link
Copy Markdown
Contributor

Attempted rebase on development (tip 95aed02c5) but hit a source-code conflict in src/modals/object/MassDeleteObject.vue between the PR's second commit ("fix mass delete in Search/Views tab not executing backend API call") and current dev.

The PR's resolved version uses store APIs that don't exist on development: objectStore.createObjectTypeSlug, objectStore.deleteObjects(type, ids), objectStore.objectTypes, objectStore.clearSelectedObjects(), objectStore.refetchSearchCollection(). Only registerObjectType exists on dev.

The PR likely needs to either:

  1. Rebase onto a branch that has the typed-store refactor first, or
  2. Re-roll the second commit against current objectStore.massDeleteObject API.

Skipping automated rebase — needs manual conflict resolution by author (@bbrands02). Branch left at its current tip.

Base automatically changed from development to documentation May 18, 2026 20:38
…by slug/id

System mappings (organisation = NULL) were unreachable from Twig executeMapping()
calls during synchronization because no user session is active, causing
applyOrganisationFilter to append AND (1 = 0) and return zero rows.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rubenvdlinde rubenvdlinde merged commit f99a117 into documentation May 27, 2026
4 checks passed
@rubenvdlinde rubenvdlinde deleted the fix/port-get-files-to-mapping-runtime branch May 27, 2026 13:17
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