Check for right media type in auto upload #13255
Conversation
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
JonasMayerDev
force-pushed
the
13035-only-pictures-are-enabled-but-videos-also-auto-synced
branch
from
3db4443
to
700dd54
Compare
There was a problem hiding this comment.
Auto upload, not uploading to the correct path after configure.
f.mp4
How to Reproduce?
- Enable auto upload for new detected media folder
- InstantUpload folder will be used for auto upload
- Configure the auto upload path (e.g. MyFavFolder) for selected folder
- Still uploading to the InstantUpload folder
| @@ -275,7 +277,11 @@ public void setExcludeHidden(boolean excludeHidden) { | |||
| } | |||
|
|
|||
| public boolean containsFile(String filePath){ | |||
There was a problem hiding this comment.
public boolean containsFile(String filePath) {
return filePath.contains(localPath);
}
public boolean isImageOrVideo(String filePath) {
return (getType() == MediaFolderType.IMAGE && MimeTypeUtil.isImage(new File(filePath))) ||
(getType() == MediaFolderType.VIDEO && MimeTypeUtil.isVideo(new File(filePath))) ||
getType() == MediaFolderType.CUSTOM;
}
It would be good to separate the logic because otherwise containsFile() doing two jobs and the name of the function will mislead us.
Why getType() == MediaFolderType.CUSTOM used?
There was a problem hiding this comment.
I think containsFile is fitting since there are "video auto upload folders", "custom auto upload folders" and "image auto upload folders". To decide if the folder contains the file, it needs to know what type the files has to decide whether it belongs to a "video auto upload folders" or a "image auto upload folders" because both correspond to the same local folder. If the user customizes a custom folder, it should upload the files regardless of type since there is no differentiation between videos and images and everything belongs to one "auto upload folder"...
There was a problem hiding this comment.
Yes, we know that this is the case and that is exactly why we need to separate it into two functions. Otherwise, we have to give such a long explanation.
The code should explain itself as much as possible, rather than us explaining it.
I think this is also an issue on master (could you pls test). When you activate auto upload and then afterward change the directory for upload, it already uploaded the files to the wrong directory. Only new files will be uploaded to the newly specified folder. To mitigate this, you can activate and change the path of the folder using the 3 dot menu in one go. |
|
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
…but-videos-also-auto-synced' into 13035-only-pictures-are-enabled-but-videos-also-auto-synced # Conflicts: # app/src/main/java/com/owncloud/android/datamodel/SyncedFolder.java
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/13255.apk |
JonasMayerDev
deleted the
13035-only-pictures-are-enabled-but-videos-also-auto-synced
branch
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Fixes #13087, Fixes #13035, Fixes #12910.
To test this PR, have images and videos in the same folder, activate auto upload for videos OR images and put a new file of the opposite type in the folder. Without this PR it uploads the file regardless. With that PR it first checks if it is the correct type and then uploads.
Master -> Image gets upload even though only video folder is activated
Screen_recording_20240711_192320.webm
PR -> No images only videos get upload when only video folder is activated
WithPRDownscaled.webm