Skip to content

Conversation

@miaulalala
Copy link
Collaborator

Fixes #2076

Also refactors the query for affected users to avoid using a where overwrite for the query.

@cypress
Copy link

cypress bot commented Jul 7, 2025

Activity  Image  Run #2791

Run Properties:  status check passed Passed #2791  •  git commit 5f2ba0b9c3: feat: use full date and time in activity emails
Project Activity
Branch Review feat/2076/improve-emails
Run status status check passed Passed #2791
Run duration 02m 22s
Commit git commit 5f2ba0b9c3: feat: use full date and time in activity emails
Committer Anna
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 10
View all changes introduced in this branch ↗︎

@miaulalala miaulalala force-pushed the feat/2076/improve-emails branch from aae6fb7 to 08cd416 Compare July 9, 2025 12:30
@miaulalala miaulalala requested a review from kesselb July 9, 2025 12:33
@miaulalala miaulalala requested a review from nickvergessen July 28, 2025 09:42
@miaulalala miaulalala force-pushed the feat/2076/improve-emails branch from 08cd416 to 1c39376 Compare August 12, 2025 18:26
@miaulalala miaulalala force-pushed the feat/2076/improve-emails branch from 1c39376 to d2541c0 Compare August 20, 2025 08:12
@miaulalala miaulalala force-pushed the feat/2076/improve-emails branch from d2541c0 to 0a771b7 Compare August 21, 2025 09:34
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

But otherwise fine by me

Comment on lines +154 to +166

$result = $query->executeQuery();

$affectedUsers = [];
while ($row = $result->fetch()) {
$affectedUsers[] = $row['amq_affecteduser'];
}
$result->closeCursor();

return $affectedUsers;
}

if ($forceSending) {
Copy link
Member

Choose a reason for hiding this comment

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

I prefer simplicity over "early return" which duplicates 12 lines :D

Suggested change
$result = $query->executeQuery();
$affectedUsers = [];
while ($row = $result->fetch()) {
$affectedUsers[] = $row['amq_affecteduser'];
}
$result->closeCursor();
return $affectedUsers;
}
if ($forceSending) {
} elseif ($forceSending) {

@miaulalala miaulalala merged commit a65bdd1 into master Aug 25, 2025
52 checks passed
@miaulalala miaulalala deleted the feat/2076/improve-emails branch August 25, 2025 12:02
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.

Notification E-mails use relative dates in email body

3 participants