Open calendar notifications in the browser.

[camilasan]

Addresses #519.

@nextcloud/calendar @nickvergessen

1. subject_rich always holds extra information about the notification.... so couldn't we have the a link to the calendar in the link of the notification instead of hidden in subject_rich? Maybe I am missing something.
2. Why for your own calendars you get 'event' and for other user's calendars you get 'event1'? Couldn't we always use 'event'?

[camilasan]

/backport to stable-3.5

[nickvergessen]

subject_rich always holds extra information about the notification.... so couldn't we have the a link to the calendar in the link of the notification instead of hidden in subject_rich? Maybe I am missing something.

That should work just fine. It is just not available on delete activities (as we can not link to the event because it was deleted)
https://github.com/nextcloud/server/blob/master/apps/dav/lib/CalDAV/Activity/Backend.php#L484-L487

Why for your own calendars you get 'event' and for other user's calendars you get 'event1'? Couldn't we always use 'event'?

We always "use" event. What you are observing comes from the event merging, e.g. if a user creates two events, there will be only 1 activity entry saying "X created event A and B"

But the mechanism is incremental and when you only modified the same entry twice, you get "X updated event A" with A being event1 instead of event.
https://github.com/nextcloud/server/blob/master/lib/private/Activity/EventMerger.php#L203

But you should never assume the placeholder name. Instead you can loop over the parameters and then you check the type of the parameters to be calendar-event

[codecov]

Codecov Report

Merging #4684 (8ffaf34) into master (8ffaf34) will not change coverage.
The diff coverage is n/a.

❗ Current head 8ffaf34 differs from pull request most recent head 664288d. Consider uploading reports for the commit 664288d to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4684   +/-   ##
=======================================
  Coverage   57.99%   57.99%           
=======================================
  Files         139      139           
  Lines       17728    17728           
=======================================
  Hits        10282    10282           
  Misses       7446     7446           

[mgallien]

if I understand correctly in case of merged events you will get a link to the second one ?

[camilasan]

if I understand correctly in case of merged events you will get a link to the second one ?

Yes, that is what I understood too.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

76.9% Coverage
0.0% Duplication

[allexzander]

@camilasan I've added some nitpicks

[claucambra]

No additional comments from me, just agree with Alex's comments :)

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

78.6% Coverage
0.0% Duplication

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

86.7% Coverage
0.0% Duplication

[claucambra]

One small comment, otherwise looks good :)

[camilasan]

/rebase

[camilasan]

/rebase

[claucambra]

I think this is mergeable, but I have a tiny comment :)

[camilasan]

/rebase

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

86.7% Coverage
0.0% Duplication

[nextcloud-desktop-bot]

AppImage file: nextcloud-PR-4684-b4882ae5c7d2caf5f52f04f231a3f38501472aa7-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.