Extra options for uploading subfolders by dean36963 · Pull Request #11852 · nextcloud/android

dean36963 · 2023-08-01T20:15:03Z

resolves #11642

Adds some extra options for uploading photos into various patterns of subfolders. yyyy or yyyy/MM or yyyy/MM/dd supported.

Currently just adds an extra field for this, and has to be used in conjunction with the "use subfolder" checkbox, the new option is greyed out and disabled when "use subfolder" is not checked.

Notes

List of considerations to add:

✅ Does dropdown make sense UI/UX-wise? (Andy: fine I would say, while other option for upload behavior uses a dialog with a radio button group)
✅ Does the database migrations make sense? This is the first time I've done that sort of migration so probably needs extra checks.
✅ Unit tests! As there's now more ways of uploading files, should we have some new unit tests for checking new upload location is correct for all new options?
✅ Order of imports and line wrapping settings, I'm not sure if this is already covered by the idea settings files in git and android-studio doing this, or do I need to run somethings?
✅ Order of the dropdown isn't ideal visually, but I wanted to make sure in case after a migration, a 0 in the database should be the first option, so previous behavior is kept (though this makes an assumption that 0 is the default which might be incorrect).
✅ Localisation concerns? There's been a change to prefs_instant_upload_path_use_subfolders_summary (but only in one language), due to a change in behaviour, so text might not make sense any more in other languages.
✅ detectNewJavaFiles: should all new classes be Kotlin? (ah I may have forgotten copyright headers...)
Tests written, or not not needed

AndyScherzinger · 2023-08-02T19:20:49Z

@dean36963 would you mind adding some screenshots, so @nextcloud/designers can have a look? Thanks in advance 👍

dean36963 · 2023-08-02T21:12:01Z

Current screenshots:

Light theme:

All Disabled	Subfolder Disabled	Subfolder Enabled	Dropdown	Selected

Dark theme:

All Disabled	Subfolder Disabled	Subfolder Enabled	Dropdown	Selected

On looking again, it looks like each area has a bold looking title text, with lighter text below. Not sure if the spinner/dropdown text should also look lighter too, but then to me it looks like it might be greyed out and not selectable.

jancborchardt

Looks very nice @dean36963! :) Only one adjustment maybe, to make it a little bit less busy when "Use subfolders" is not checked, the "Subfolder options" could only be shown once that is checked?

dean36963 · 2023-08-03T20:47:37Z

All Disabled	Subfolder Disabled	Subfolder Enabled	Dropdown	Selected

AndyScherzinger · 2023-08-08T14:18:48Z

        AutoMigration(from = 68, to = 69),
-        AutoMigration(from = 69, to = 70)
+        AutoMigration(from = 69, to = 70),
+        AutoMigration(from = 71, to = 72)


@tobiasKaminsky not needed, right? Since there is an actual migration?

I tried reverting this section, and when I moved from a build before my changes, to a build including all my changes (including reverting this - not pushed) I saw that some auto upload folder settings were lost. Keeping this line as is currently committed seemed to keep folder settings as I'd expect when upgrading.

It might be that I'd need to revert this and also change another section, but I am unsure what that change would be.

Can't say for sure and would need to check if and which changes have been made for 70,71,72 to know When to set the Auto Migration. 👍

Logic of Room is, afaik:

if auto migrate is enabled, then it keeps entire DB, but changes/add new column

if no auto migrate and no manual migration -> resets entire DB -> all is lost

72.json is for a new database (new app installation)
--> now the question is, do we need a manual migration, e.g. to add a specific value to hidden or subFolderRule? If not, then it should work with AutoMigration and 72.json as it is now.

now the question is, do we need a manual migration, e.g. to add a specific value to hidden or subFolderRule?

In this regard, I've currently got it so the previous option for this (YYYY/MM) is the first option in the dropdown and appears as the default value post migration (and just in general for new folders). So I think in it's current state, no default value is needing to be set.

However, if it's more sensible to reorder these options visually (yearly then monthly, then daily for example), it might be required to then do a manual migration?

AndyScherzinger · 2023-08-08T14:19:12Z

-    val hidden: Int?
+    val hidden: Int?,
+    @ColumnInfo(name = ProviderTableMeta.SYNCED_FOLDER_SUBFOLDER_RULE)
+    val subFolderRule: Int?


this one (new column, so no auto migration, right @tobiasKaminsky ?

AndyScherzinger · 2023-08-08T16:24:10Z

Some minor comments and I'll bee needing some feedback from @tobiasKaminsky regarding the room migration code bits.
@jancborchardt are you fine with the dropdown for the folder options, see #11852 (comment) - keeping in mind, the "What if the file already exists?" option uses a dialog with a radio button group.

marcoambrosini

Looks very nice :)

A couple of non blocking comments regarding design:

I think that the complexity of these settings warrant the use of a full page view as opposed to a dialog. It will also work better on smaller mobile screens.
There's a conceptual mismatch between the title of the page and the switch right next to it. I assume that the switch turns auto-upload on and off but the title says "preferences for auto upload", so it's a little confusing. I think we should change the title to "Auto upload" only.

tobiasKaminsky · 2023-08-30T07:27:54Z

Rebased and good to go

Adds some extra options for uploading photos into various patterns of subfolders. yyyy or yyyy/MM or yyyy/MM/dd supported. Currently just adds an extra field for this, and has to be used in conjunction with the "use subfolder" checkbox, the new option is greyed out and disabled when "use subfolder" is not checked. Signed-off-by: Dean Birch <[email protected]>

Signed-off-by: Dean Birch <[email protected]>

Instead of greying out the subfolder rules when not active, hide them. Also reverts changes to make a function out of getAlpha. Signed-off-by: Dean Birch <[email protected]>

Signed-off-by: Dean Birch <[email protected]>

Signed-off-by: Andy Scherzinger <[email protected]>

Signed-off-by: tobiasKaminsky <[email protected]>

github-actions · 2023-08-30T10:18:54Z

Codacy

Lint

Type	master	PR
Warnings	80	80
Errors	0	0

SpotBugs

Category	Base	New
Bad practice	26	26
Correctness	85	85
Dodgy code	433	433
Experimental	2	2
Internationalization	9	9
Malicious code vulnerability	2	2
Multithreaded correctness	9	9
Performance	59	59
Security	18	18
Total	643	643

github-actions · 2023-08-30T10:20:27Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11852.apk

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

AndyScherzinger · 2023-08-30T13:54:18Z

Merged! Nice work @dean36963 🎉 And thanks for building and polishing it 💯

github-actions · 2024-04-04T01:44:17Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

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!

AndyScherzinger added design 3. to review labels Aug 2, 2023

AndyScherzinger marked this pull request as ready for review August 3, 2023 07:24

AndyScherzinger requested review from ZetaTom, jancborchardt, marcoambrosini, nimishavijay and tobiasKaminsky August 3, 2023 07:24

jancborchardt reviewed Aug 3, 2023

View reviewed changes

AndyScherzinger reviewed Aug 8, 2023

View reviewed changes

AndyScherzinger added the feature: auto upload label Aug 8, 2023

marcoambrosini approved these changes Aug 9, 2023

View reviewed changes

AndyScherzinger force-pushed the synced_folder_upload_subfolderoptions_pr branch from 381de90 to 35d125c Compare August 28, 2023 07:59

AndyScherzinger added this to the Nextcloud App 3.26.0 milestone Aug 29, 2023

AndyScherzinger force-pushed the synced_folder_upload_subfolderoptions_pr branch from 4d78049 to c29bfc0 Compare August 30, 2023 07:34

AndyScherzinger approved these changes Aug 30, 2023

View reviewed changes

tobiasKaminsky force-pushed the synced_folder_upload_subfolderoptions_pr branch from c29bfc0 to 612aad9 Compare August 30, 2023 07:44

tobiasKaminsky approved these changes Aug 30, 2023

View reviewed changes

tobiasKaminsky force-pushed the synced_folder_upload_subfolderoptions_pr branch from 612aad9 to e5032c9 Compare August 30, 2023 08:37

dean36963 added 5 commits August 30, 2023 11:30

gradle spotless check formatting

a687ca6

Signed-off-by: Dean Birch <[email protected]>

Fix broken unit tests

d30980d

Signed-off-by: Dean Birch <[email protected]>

Convert java to kotlin and add header

13b61e8

Signed-off-by: Dean Birch <[email protected]>

Update en localization of use subfolders summary

605aa14

Signed-off-by: Dean Birch <[email protected]>

dean36963 and others added 4 commits August 30, 2023 11:30

Hide subfolder options when not using subfolders

768171c

Instead of greying out the subfolder rules when not active, hide them. Also reverts changes to make a function out of getAlpha. Signed-off-by: Dean Birch <[email protected]>

Fix unit tests with SubFolderRule in constructor

d90e06f

Signed-off-by: Dean Birch <[email protected]>

Rename header of Auto upload settings to just "Auto upload"

32b696b

Signed-off-by: Andy Scherzinger <[email protected]>

remove unused variables

920c69d

Signed-off-by: Andy Scherzinger <[email protected]>

tobiasKaminsky force-pushed the synced_folder_upload_subfolderoptions_pr branch from e5032c9 to 22b7d23 Compare August 30, 2023 09:30

rebased

f32cb61

Signed-off-by: tobiasKaminsky <[email protected]>

tobiasKaminsky force-pushed the synced_folder_upload_subfolderoptions_pr branch from 22b7d23 to f32cb61 Compare August 30, 2023 10:10

tobiasKaminsky merged commit c00e614 into master Aug 30, 2023

delete-merged-branch Bot deleted the synced_folder_upload_subfolderoptions_pr branch August 30, 2023 12:35

rclough mentioned this pull request Sep 17, 2023

Excessive file upload conflicts after update to 3.26.0 (not before) #11974

Closed

4 tasks done

This was referenced Dec 14, 2023

Auto uploads subfolders based on year only #11144

Closed

Auto Upload with Use Subfolders: Add options for how to organize files #10374

Closed

Instant picture upload subfolder naming #1330

Open

github-actions Bot added the feedback-requested label Apr 4, 2024

Conversation

dean36963 commented Aug 1, 2023 • edited by AndyScherzinger Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Uh oh!

AndyScherzinger commented Aug 2, 2023

Uh oh!

dean36963 commented Aug 2, 2023

Uh oh!

jancborchardt left a comment

Choose a reason for hiding this comment

Uh oh!

dean36963 commented Aug 3, 2023

Uh oh!

AndyScherzinger Aug 8, 2023

Choose a reason for hiding this comment

Uh oh!

dean36963 Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

AndyScherzinger Aug 12, 2023

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky Aug 24, 2023

Choose a reason for hiding this comment

Uh oh!

dean36963 Aug 27, 2023

Choose a reason for hiding this comment

Uh oh!

AndyScherzinger Aug 8, 2023

Choose a reason for hiding this comment

Uh oh!

AndyScherzinger commented Aug 8, 2023

Uh oh!

marcoambrosini left a comment • edited by AndyScherzinger Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented Aug 30, 2023

Uh oh!

github-actions Bot commented Aug 30, 2023

Codacy

Lint

SpotBugs

Uh oh!

github-actions Bot commented Aug 30, 2023

Uh oh!

AndyScherzinger commented Aug 30, 2023

Uh oh!

github-actions Bot commented Apr 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dean36963 commented Aug 1, 2023 •

edited by AndyScherzinger

Loading

marcoambrosini left a comment •

edited by AndyScherzinger

Loading