Skip to content

Conversation

@mlojewski-me
Copy link
Contributor

When invoking 'occ activity:send-mails ' correctly download affected users: amq_timestamp = amq_latest_send - batchtime

… download affected users: amq_timestamp = amq_latest_send - batchtime
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.

Quite interesting that no one ever complained about this.

@nickvergessen nickvergessen added this to the Nextcloud 15 milestone Nov 12, 2018
@nickvergessen nickvergessen changed the title Correctly download affected users Correctly restrict affected users when using command to send emails Nov 12, 2018
@nickvergessen nickvergessen merged commit c73cfd2 into nextcloud:master Nov 12, 2018
if ($restrictEmails !== null) {
if ($restrictEmails === UserSettings::EMAIL_SEND_HOURLY) {
$query->where($query->expr()->lte('amq_timestamp', $query->createFunction($query->getColumnName('amq_latest_send') . ' + ' . 3600)));
$query->where($query->expr()->eq('amq_timestamp', $query->createFunction($query->getColumnName('amq_latest_send') . ' - ' . 3600)));
Copy link
Member

Choose a reason for hiding this comment

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

ah, the lte made it execute all emails.

Copy link
Member

Choose a reason for hiding this comment

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

I still don't get it. What if the batch run is one second late. Then it does not execute anymore for those users. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The time of batch run does not matter. amq_timestamp is time of an activity event, amq_latest_send is always amq_timestamp plus seconds that user set for its mail notification interval, so query gets only users who have any notifications and these notifications' amq_timestamp = amq_latest_send - batchtime.

Copy link
Member

Choose a reason for hiding this comment

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

Got it 👍 I misread the amq_timestamp and amq_latest_send as the same name 🙈

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