Skip to content

feat: Retention management — bewaartermijnen and selectielijsten#986

Merged
rubenvdlinde merged 3 commits intodevelopmentfrom
fix/retention-management
Apr 9, 2026
Merged

feat: Retention management — bewaartermijnen and selectielijsten#986
rubenvdlinde merged 3 commits intodevelopmentfrom
fix/retention-management

Conversation

@rubenvdlinde
Copy link
Copy Markdown
Contributor

@rubenvdlinde rubenvdlinde commented Mar 24, 2026

Summary

Context

This PR was previously merged and reverted along with other PRs while investigating a development branch issue.

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report

Repository ConductionNL/openregister
Commit 2b6366f
Branch 986/merge
Event pull_request
Generated 2026-03-24 11:21 UTC
Workflow Run https://github.com/ConductionNL/openregister/actions/runs/23486775316

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (147 total)

Metric Count
Approved (allowlist) 146
Approved (override) 1
Denied 0

npm dependencies (586 total)

Metric Count
Approved (allowlist) 585
Approved (override) 1
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

Base automatically changed from development to beta March 27, 2026 16:28
Copy link
Copy Markdown
Member

@rjzondervan rjzondervan left a comment

Choose a reason for hiding this comment

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

PR shows various PHPstan errors, and is also using a way of calling server variables that is deprecated

Comment on lines +171 to +172
$objectMapper = \OC::$server->get(MagicMapper::class);
$connection = \OC::$server->getDatabaseConnection();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This way of requesting global parameters is being phased out by Nextcloud and should not be added new

Comment on lines +89 to +94
$retentionService = \OC::$server->get(RetentionService::class);
$settingsHandler = \OC::$server->get(ObjectRetentionHandler::class);
$objectMapper = \OC::$server->get(MagicMapper::class);
$auditMapper = \OC::$server->get(AuditTrailMapper::class);
$deleteObject = \OC::$server->get(DeleteObject::class);
$saveObject = \OC::$server->get(\OCA\OpenRegister\Service\Object\SaveObject::class);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my previous comment

@rubenvdlinde rubenvdlinde changed the base branch from beta to development April 9, 2026 10:31
@rubenvdlinde rubenvdlinde requested a review from rjzondervan April 9, 2026 10:37
@rubenvdlinde rubenvdlinde added ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer and removed ready-for-code-review Build complete — awaiting code reviewer ready-for-security-review Code review complete — awaiting security reviewer labels Apr 9, 2026
Copy link
Copy Markdown
Member

@rjzondervan rjzondervan left a comment

Choose a reason for hiding this comment

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

Some style notes for later reference, furthermore ok.

Comment on lines +141 to +148
MagicMapper $objectMapper,
LegalHoldService $legalHoldService,
DeleteObject $deleteObject,
AuditTrailMapper $auditTrailMapper,
IAppConfig $appConfig,
IJobList $jobList,
IUserSession $userSession,
LoggerInterface $logger
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note for updating agents: these can all be done in the PHP 8 style:

private readonly MagicMapper $objectMapper,

And without needing any lines within the {} for the constructor.

*
* @return array<string, mixed> The destruction certificate data.
*/
public function generateCertificate(array $destructionList, array $executionResult): array
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See issue #1261

Comment on lines +2954 to +2963
// Apply archival metadata from schema archive configuration.
try {
$retentionService = \OC::$server->get(\OCA\OpenRegister\Service\RetentionService::class);
$preparedObject = $retentionService->applyArchivalMetadata($preparedObject, $schema);
} catch (\Throwable $e) {
$this->logger->debug(
'[SaveObject] RetentionService not available, skipping archival metadata: '.$e->getMessage()
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When would the retentionservice not be available?

@rubenvdlinde rubenvdlinde merged commit 0c4b108 into development Apr 9, 2026
20 checks passed
@rubenvdlinde rubenvdlinde deleted the fix/retention-management branch April 9, 2026 12: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.

2 participants