Skip to content

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

Open
bbrands02 wants to merge 5 commits into
documentationfrom
fix/port-get-files-to-mapping-runtime
Open

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

Conversation

@bbrands02
Copy link
Copy Markdown
Contributor

@bbrands02 bbrands02 commented May 8, 2026

Summary

This PR includes three independent fixes and one new feature targeting the development branch: a new `getRegisters()` API on `ObjectService`, a ported `getFiles` Twig function for the mapping runtime, and two mass-delete flow fixes in the frontend.


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

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

🤖 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
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

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>
Copy link
Copy Markdown
Contributor

@SudoThijn SudoThijn left a comment

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.

Copy link
Copy Markdown
Contributor

@WilcoLouwerse WilcoLouwerse left a comment

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

4 participants