feat: getRegisters with expanded schemas, getFiles Twig function, and mass delete fixes#1445
feat: getRegisters with expanded schemas, getFiles Twig function, and mass delete fixes#1445bbrands02 wants to merge 5 commits into
Conversation
…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>
Quality Report — ConductionNL/openregister @
|
| 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>
There was a problem hiding this comment.
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.
| private readonly MappingMapper $mappingMapper, | ||
| ICacheFactory $cacheFactory, | ||
| private readonly LoggerInterface $logger | ||
| private readonly LoggerInterface $logger, |
There was a problem hiding this comment.
🔴 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 |
There was a problem hiding this comment.
🟡 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:
- Wrap and return
[]on failure (and document in the docblock). - 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 |
There was a problem hiding this comment.
🟢 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
left a comment
There was a problem hiding this comment.
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.
…iles-to-mapping-runtime
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Attempted rebase on The PR's resolved version uses store APIs that don't exist on The PR likely needs to either:
Skipping automated rebase — needs manual conflict resolution by author (@bbrands02). Branch left at its current tip. |
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).
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.
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.
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.
Test plan
🤖 Generated with Claude Code