Skip to content

Conversation

@pulsejet
Copy link
Member

The current behavior when trying to copy a shared file to an album is meaningless. Since the copyInto call returns false, DAV falls back to trying to create the file again with the same content (see this fallback). As a result,

  1. The shared file gets copied over to the current user.
  2. It gets added to the album.
  3. The call fails anyway because the name of the destination node (that is copied inside the album) has changed, so the subsequent call to get the node from the album fails.

If we throw while copying, the user can't add shared files to albums anymore with a proper error message, and files don't get copied unnecessarily.

One thing to note here, I do think ideally there should be some way to add shared files to albums, as long as the file as resharing permissions. Of course then we might need to check later if the file is unshared and remove it from the album. Thoughts?

@pulsejet pulsejet requested review from artonge and skjnldsv March 30, 2023 00:48
@pulsejet pulsejet added bug Something isn't working 3. to review Waiting for reviews feature: albums Related to the albums section labels Mar 30, 2023
@szaimen szaimen added this to the Nextcloud 27 milestone Mar 30, 2023
Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Indeed, much cleaner.
One small nitpick though

@artonge
Copy link
Collaborator

artonge commented Mar 30, 2023

One thing to note here, I do think ideally there should be some way to add shared files to albums, as long as the file as resharing permissions. Of course then we might need to check later if the file is unshared and remove it from the album. Thoughts?

I don't remember why the limitation exists, but it was probably to reduce the complexity of the first version.
Could be a nice addition to support shared files 👍

@artonge
Copy link
Collaborator

artonge commented Apr 12, 2023

/backport to stable26

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Apr 12, 2023
@artonge
Copy link
Collaborator

artonge commented Apr 12, 2023

/backport to stable25

@artonge
Copy link
Collaborator

artonge commented Apr 12, 2023

/rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working feature: albums Related to the albums section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants