Fix public download activity

[come-nc]

• Resolves: [Bug]: NC31.0.0 not logging view/download activity #51097

Summary

Fix pubilc download activity in 31+, and improve upon it to make sure it’s consistent.

TODO

• Viewing a video triggers several activity events. They are merged in activity feed, but generate a lot of notifications.
• Viewing a photo through preview does not trigger activity, is that an issue?
• Downloading the whole share triggers activity for the folder and then for each file.
• Downloading several files in a zip triggers activity for each file. I think that’s good?
• The manual activity publishing in ShareController should be removed to avoid duplicates. But I think that’s the one publishing the activity for folder download, so we should make sure we do not lose it.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[come-nc]

• Viewing a photo through preview does not trigger activity, is that an issue?

This one is not a regression so let’s postpone for later.
A solution would be to listen to BeforePreviewFetchedEvent, but we’d have to decide at which size of preview we consider that downloading, because opening the file list already loads previews of small sizes.

[come-nc]

/backport to stable31

[come-nc]

I would like to add tests for this, but activity application is needed.
I can add the tests in activity application cypress tests, but that mean a PR breaking the feature on server will have green CI and we will only figure it out later. Still better than no tests I suppose?
Other ideas?

[susnux]

I would like to add tests for this, but activity application is needed.

Is there no OCS API or something you can test with behat?

Otherwise I see two ways:

1. Do it like we do for some Talk tests and install the Activity for some Behat tests (IMHO this makes sense for all things that have integrations in OCP)
2. Only add them to Activity -> would not be directly spotted if something breaks the app.

[daniel-richter]

Just a heads-up: I had the same issue (downloads of individual files from a shared folder weren't showing up in the activities, #51097). I can confirm that the suggested changes solve the problem for me.

[daniel-richter]

• Viewing a video triggers several activity events. They are merged in activity feed, but generate a lot of notifications.
• Viewing a photo through preview does not trigger activity, is that an issue?

Well, that's how it used to be. If you watched a video, it would show up in the "downloaded via public link" log. For images, you had to click "download" for it to show up in the log. Just viewing the image didn't do it.

For me, it's more important to see the explicit downloads (of an image) instead of just viewing it in the gallery. If both are to be logged, I'd prefer to see two different events ("viewed" and "downloaded").

[susnux]

Did not yet had time to test this, but old behavior was also inconsistent as there were some endpoints that never caused a download notification.
Also we should define what this notification means.

Because either it only means real downloads, then we only need to track the DAV endpoints (GET on file or zipfolder plugin).
Or it also means viewing the file, then we also need to track preview endpoints (which is used by the viewer for images).

If I get the code right then this PR implements both. Which is fine but might confuse users as they will see "file downloaded" notifications even if they set "download permission" to false because previews are shown.

[daniel-richter]

I see two options here:

1. Track only real downloads.
2. If we want to track views, too, we should introduce a separate subject, "public_shared_file_viewed" (in contrast to "public_shared_file_downloaded").

[come-nc]

I would like to add tests for this, but activity application is needed.

Is there no OCS API or something you can test with behat?

It’s DAV, not OCS, but that can be tested with behat. But still needs the activity app.

Otherwise I see two ways:

1. Do it like we do for some Talk tests and install the Activity for some Behat tests (IMHO this makes sense for all things that have integrations in OCP)

I will try that.

If I get the code right then this PR implements both. Which is fine but might confuse users as they will see "file downloaded" notifications even if they set "download permission" to false because previews are shown.

No, this PR only logs downloads through DAV, kind of the same as old behavior.
Preview endpoint does not trigger any notifications. (edit: Well, dav and the legacy ocs endpoint as well, any request that reads the file actually)

Actually, something I did not test is if a public link is loaded in which there is a file with no preview yet. Then opening the folder may trigger the preview computing and a download activity. But I’m not 100% sure that is possible in real life. How do you share a folder without triggering preview caching?

[susnux]

Actually, something I did not test is if a public link is loaded in which there is a file with no preview yet. Then opening the folder may trigger the preview computing and a download activity. But I’m not 100% sure that is possible in real life. How do you share a folder without triggering preview caching?

Opening an image in viewer also always triggers new preview generation as the image shown in viewer is basically a big preview. Thats what I meant with "view activity"