Skip to content

refactor(admin_audit): migrate to IAppConfig#58750

Open
joshtrichards wants to merge 8 commits intomasterfrom
jtr/migrate-AuditLogger-to-IAppConfig
Open

refactor(admin_audit): migrate to IAppConfig#58750
joshtrichards wants to merge 8 commits intomasterfrom
jtr/migrate-AuditLogger-to-IAppConfig

Conversation

@joshtrichards
Copy link
Copy Markdown
Member

@joshtrichards joshtrichards commented Mar 6, 2026

Summary

🧹

TODO

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added technical debt 🧱 🤔🚀 ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Mar 6, 2026
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 34 milestone Mar 6, 2026
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards marked this pull request as ready for review March 6, 2026 15:55
@joshtrichards joshtrichards requested a review from a team as a code owner March 6, 2026 15:55
@joshtrichards joshtrichards requested review from Altahrim, ArtificialOwl, come-nc and leftybournes and removed request for a team March 6, 2026 15:55
Comment on lines 84 to 86
$context->registerService(IAuditLogger::class, function (ContainerInterface $c) {
return new AuditLogger($c->get(ILogFactory::class), $c->get(IConfig::class));
return new AuditLogger($c->get(ILogFactory::class), $c->get(IAppConfig::class), $c->get(IConfig::class));
});
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.

Could simply be an alias I believe?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Still need the factory/closure to create the IAuditLogger service AFAIK. In this case the factory/closure is creating the IAuditLogger:class as a service, by returning an AuditLogger.

Just the interface IAuditLogger is being published as the service directly.

I guess we could do both, but I don't see any reason to register the service against both the implementation and the interface. Existing behavior too. ;-)

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.

The DI is able to magically build classes based on their attributes.
So you can just do

$context->registerServiceAlias(IAuditLogger::class, AuditLogger::class);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews feature: admin audit ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants