Skip to content

feat: adding an option to exclude users from the acitivity log#2626

Open
miaulalala wants to merge 5 commits into
masterfrom
feat/excluded-users
Open

feat: adding an option to exclude users from the acitivity log#2626
miaulalala wants to merge 5 commits into
masterfrom
feat/excluded-users

Conversation

@miaulalala

Copy link
Copy Markdown
Collaborator

No description provided.

@miaulalala

Copy link
Copy Markdown
Collaborator Author

@Aiiaiiio can you add some tests and remove the "all" option for the exclusion please?

Aiiaiiio added 4 commits June 25, 2026 13:30
Signed-off-by: Tamás Bari <adaorcpp@gmail.com>
Signed-off-by: Tamás Bari <adaorcpp@gmail.com>
Signed-off-by: Tamás Bari <adaorcpp@gmail.com>
Signed-off-by: Tamás Bari <adaorcpp@gmail.com>
@Aiiaiiio Aiiaiiio force-pushed the feat/excluded-users branch from e8fcfb6 to 2c44647 Compare June 25, 2026 11:30
Signed-off-by: Tamás Bari <adaorcpp@gmail.com>
@Aiiaiiio Aiiaiiio marked this pull request as ready for review June 25, 2026 11:43
Copilot AI review requested due to automatic review settings June 25, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for skipping activity/mailer persistence for events whose author is configured to be excluded from the activity log, and extends the unit tests to validate the new exclusion behavior.

Changes:

  • Introduce shouldSend() / isExcludedAuthor() helpers in OCA\Activity\Data.
  • Apply exclusion logic to send(), storeMail(), and bulkSend() to prevent inserts when excluded.
  • Add PHPUnit coverage for excluded-author behavior across the three code paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lib/Data.php Adds exclusion helpers and gates inserts in activity + mail queue writes based on config.
tests/DataTest.php Adds data provider + tests asserting inserts are skipped/allowed depending on exclusion rules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Data.php
Comment on lines +58 to +72
private function isExcludedAuthor(IEvent $event): bool {
$excludedUsers = $this->config->getSystemValue('activity_log_exclude_users', []);
if (empty($excludedUsers)) {
return false;
}
$author = $event->getAuthor();
if ($author === '' || !isset($excludedUsers[$author])) {
return false;
}
$rule = $excludedUsers[$author];
if (is_array($rule)) {
return in_array($event->getType(), $rule, true);
}
return false;
}
Comment thread tests/DataTest.php
Comment on lines +350 to +363
public static function dataExcludedAuthor(): array {
return [
// author+type match → blocked
['alice', 'target', 'file_created', ['alice' => ['file_created']], false],
// type mismatch → allowed
['alice', 'target', 'file_created', ['alice' => ['file_deleted']], true],
// different user → allowed
['bob', 'target', 'file_created', ['alice' => ['file_created']], true],
// empty config → allowed
['alice', 'target', 'file_created', [], true],
// non-array rule → allowed
['alice', 'target', 'file_created', ['alice' => 'file_created'], true],
];
}

@miaulalala miaulalala left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

  • We already have activity_expire_exclude_users (flat list of user IDs) and now activity_log_exclude_users (map of user => [types]). Same "exclude_users" naming, totally different shape. That's going to bite someone editing config.php by hand. Can we pick a less confusable name or at least doc the shape clearly, either in docs and example config, or just the docs?
  • storeMail(): you dropped the $affectedUser local var but still call $event->getAffectedUser() again for amq_affecteduser. Just keep the var, no need for the extra call.
  • Non-array $rule silently returns "not excluded". Please add a debug log line for that case.

Otherwise the code and test coverage looks good!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants