E-Mail-Reminder to all attendees is sent only by organizer

[christlang]

When the organizer adds the reminder the nextcloud attendees only get 2 mails now.

• one from the organizer
• one for their own reminder

outside-users get only the mail from the organizer, because they have no own reminders

Fixes #21370

[tcitworld]

Seems you need to update a test as well:

There was 1 failure:

1) OCA\DAV\Tests\unit\CalDAV\Reminder\NotificationProvider\EmailProviderTest::testSendWithAttendees
Expectation failed for method name is "validateMailAddress" when invoked at sequence index 1
Parameter 0 for invocation OCP\Mail\IMailer::validateMailAddress('[email protected]'): bool does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[email protected]'
+'[email protected]'

/drone/src/apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php:127
/drone/src/apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php:340

[EhiOnime]

OK I think I found the solution to the problem in 21370 by changing the SQL query in Reminder/Backend.php and adding a GroupBy clause to ensure that even if there are multiple entries in the database, only a single one is returned:

Diff Patch is attached it should be applied in your nextcloud/apps/dav directory

single-notifications.txt

[christlang]

@EhiOnime I like your solution ;)

Have you tested it? Because you get only one reminder back from the sql-query only one will be performed. And in the next run the next run the next will be performed, isn't it?

[christlang]

ah for the tests I have to install phpunit ...
https://docs.nextcloud.com/server/latest/developer_manual/core/unit-testing.html

[EhiOnime]

Thank you for your emails. Yes, unfortunately the single patch against the database query did not seems to have solve the problem completely as subsequent runs of the cron jobs will keep processing them one by one, so the users get multiple notifications just with some delay inbetween. Please find attached a new patch on only the file EmailProvider.php that does a much better job, I think..  Could you also help to test this ? Kind regards

…

On 5/8/21 2:39 PM, cl wrote: ah for the tests I have to install phpunit ... https://docs.nextcloud.com/server/latest/developer_manual/core/unit-testing.html <https://docs.nextcloud.com/server/latest/developer_manual/core/unit-testing.html> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26496 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AT7ATLOKMDSE2DX7AZ5XTF3TMUWHBANCNFSM42WURAPA>.

[EhiOnime]

OK, here is the definite fix. It adds a userManager object to EmailProvider.php and uses that to filter out any atendee email address belonging to a nextcloud user. Best

…

On 5/8/21 5:01 PM, Onime Clement wrote: Thank you for your emails. Yes, unfortunately the single patch against the database query did not seems to have solve the problem completely as subsequent runs of the cron jobs will keep processing them one by one, so the users get multiple notifications just with some delay inbetween. Please find attached a new patch on only the file EmailProvider.php that does a much better job, I think..  Could you also help to test this ? Kind regards On 5/8/21 2:39 PM, cl wrote: > > ah for the tests I have to install phpunit ... > https://docs.nextcloud.com/server/latest/developer_manual/core/unit-testing.html > <https://docs.nextcloud.com/server/latest/developer_manual/core/unit-testing.html> > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#26496 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AT7ATLOKMDSE2DX7AZ5XTF3TMUWHBANCNFSM42WURAPA>. >

[miaulalala]

Hi @christlang , are you planning on updating this PR? If not, would you mind if I took over? What you have done so far looks good, would be a shame to have wasted your time!

[christlang]

your are welcome to finish this work, sorry for late response. You see I have not really the time to bring this to an end ;) thanks for taking over

[miaulalala]

Closing in favour of #34417