Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle copy of folders containing live photos #49293

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Nov 14, 2024

We need to recursively look for live photos in the folder, and then handle them as usual.

Fix #49289
Fix #49307

@kesselb
Copy link
Contributor

kesselb commented Nov 15, 2024

getDirectoryListing may return a listing like

  • a.jpg
  • a.mov
  • b.jpg
  • b.mov

Loop:

  1. handleCopy for a.jpg will copy a.mov too.
  2. handleCopy for a.mov will then copy a.jpg again.

It would be nice to skip the copy process for a.mov and a.jpg for the second execution if that's possible without making the whole logic much more complex.

@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch 5 times, most recently from 48eee31 to edf6450 Compare November 20, 2024 14:27
@artonge artonge changed the base branch from master to artonge/chore/update_nc_cypress_beta.11 November 20, 2024 14:27
@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch 2 times, most recently from 5de0228 to c74b1ba Compare November 20, 2024 16:38
@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch from c74b1ba to 892349f Compare November 21, 2024 16:54
@kesselb
Copy link
Contributor

kesselb commented Nov 21, 2024

Regression: It's no longer possible to copy a folder in the current folder. ("Testfiles" is an external local storage)

master:

Screencast.From.2024-11-21.19-25-26.webm

this pr:

Screencast.From.2024-11-21.19-26-00.webm

@kesselb
Copy link
Contributor

kesselb commented Nov 21, 2024

Regression: Copying a folder with live photos is not possible. (Tested on the same external storage and from external storage to primary 3).

master:

Screencast.From.2024-11-21.19-38-17.webm

this pr:

Screencast.From.2024-11-21.19-37-50.webm

@kesselb
Copy link
Contributor

kesselb commented Nov 21, 2024

Test cases:

Scenario Expected Result
Copy live photo "a.jpg" to folder where "a.mov" exists Copy not possible
Copy live photo "a.jpg" to folder where "a.mov" exists (batch action) Copy not possible
Move live photo "a.jpg" to folder where "a.mov" exists Move not possible
Move live photo "a.jpg" to folder where "a.mov" exists (batch action) Move not possible
Copy a folder with live photos (same storage) Copy possible
Copy a folder with live photos (same storage, batch action) Copy possible
Copy a folder with live photos (external local to primary s3) Copy possible
Copy a folder with live photos (external local to primary s3, batch action) Copy possible
Move a folder with live photos (same storage) Live photos work after move
Move a folder with live photos (same storage, batch action) Live photos work after move
Move a folder with live photos (external local to primary s3) Live photos work after move
Move a folder with live photos (external local to primary s3, batch action) Live photos work after move

@artonge artonge force-pushed the artonge/chore/update_nc_cypress_beta.11 branch 5 times, most recently from 5966751 to ebda7b4 Compare November 25, 2024 15:22
@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch from 892349f to 5e692da Compare November 25, 2024 16:52
Base automatically changed from artonge/chore/update_nc_cypress_beta.11 to master November 26, 2024 09:17
We need to recursively look for live photos in the folder,
and then handle them as usual.

Signed-off-by: Louis Chemineau <[email protected]>
@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch from 50b5b5e to 8e81852 Compare November 26, 2024 10:21
…a mov file with the same name

Signed-off-by: Louis Chemineau <[email protected]>
@artonge artonge force-pushed the artonge/fix/handle_folders_copy_live_photos branch 2 times, most recently from 43ed165 to 5b01166 Compare November 28, 2024 16:35
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.

[Bug]: Copying a folder containing live photos causes them to lose their live photo functionality
4 participants