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

#7815 Delayed synchronization #11278

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

Conversation

BatPio
Copy link

@BatPio BatPio commented Jan 19, 2023

Fixes #7815
Fixes #2323

  • Tests written, or not not needed
    Some tests are written. I'm waiting for suggestions on what else can be tested.

Delay configuration:

sync_delay

BatPio added 13 commits January 19, 2023 22:54
#	CHANGELOG.md
#	app/schemas/com.nextcloud.client.database.NextcloudDatabase/66.json
#	app/src/main/java/com/owncloud/android/ui/preview/PreviewMediaFragment.java
#	app/src/main/java/com/owncloud/android/ui/preview/PreviewVideoActivity.kt
#	app/src/main/res/values-b+en+001/strings.xml
#	app/src/main/res/values-bg-rBG/strings.xml
#	app/src/main/res/values-cs-rCZ/strings.xml
#	app/src/main/res/values-da/strings.xml
#	app/src/main/res/values-de/strings.xml
#	app/src/main/res/values-es/strings.xml
#	app/src/main/res/values-fr/strings.xml
#	app/src/main/res/values-hu-rHU/strings.xml
#	app/src/main/res/values-pl/strings.xml
#	app/src/main/res/values-tr/strings.xml
#	app/src/main/res/values-zh-rCN/strings.xml
#	app/src/main/res/values-zh-rHK/strings.xml
#	app/src/main/res/values-zh-rTW/strings.xml
#	app/src/main/res/values/strings.xml

Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #11278 (3d40c3a) into master (aeb1632) will increase coverage by 0.07%.
The diff coverage is 37.20%.

❗ Current head 3d40c3a differs from pull request most recent head 501d1fe. Consider uploading reports for the commit 501d1fe to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #11278      +/-   ##
============================================
+ Coverage     31.44%   31.52%   +0.07%     
- Complexity     3413     3427      +14     
============================================
  Files           575      577       +2     
  Lines         42018    42146     +128     
  Branches       5660     5664       +4     
============================================
+ Hits          13214    13286      +72     
- Misses        26861    26905      +44     
- Partials       1943     1955      +12     
Impacted Files Coverage Δ
...com/nextcloud/client/database/NextcloudDatabase.kt 100.00% <ø> (ø)
...cloud/client/database/entity/SyncedFolderEntity.kt 0.00% <0.00%> (ø)
...java/com/nextcloud/client/di/ComponentsModule.java 0.00% <ø> (ø)
...in/java/com/nextcloud/client/jobs/FilesSyncWork.kt 21.95% <0.00%> (-0.18%) ⬇️
...loud/android/datamodel/FilesystemDataProvider.java 4.39% <0.00%> (-0.10%) ⬇️
...oud/android/datamodel/SyncedFolderDisplayItem.java 35.29% <ø> (ø)
...ncloud/android/datamodel/SyncedFolderProvider.java 10.93% <0.00%> (-0.18%) ⬇️
...ain/java/com/owncloud/android/db/ProviderMeta.java 100.00% <ø> (ø)
...cloud/android/ui/activity/SyncedFoldersActivity.kt 20.43% <0.00%> (-0.55%) ⬇️
.../dialog/SyncedFolderPreferencesDialogFragment.java 0.00% <0.00%> (ø)
... and 11 more

Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
Signed-off-by: batpio <[email protected]>
@BatPio BatPio marked this pull request as ready for review January 21, 2023 22:34
@BatPio BatPio linked an issue Jan 21, 2023 that may be closed by this pull request
Copy link
Member

@AlvaroBrey AlvaroBrey left a comment

Choose a reason for hiding this comment

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

I haven't tested this yet, especially to see how this will behave with existing autoupload folders. This can't be tested without adressing my first comment below, however.

Having said that, this looks quite promising! Thanks for contributing @BatPio

BatPio and others added 17 commits February 19, 2025 21:22
…ified-master-v2

# Conflicts:
#	app/schemas/com.nextcloud.client.database.NextcloudDatabase/87.json
#	app/src/main/java/com/nextcloud/client/database/NextcloudDatabase.kt
Signed-off-by: batpio <[email protected]>
@jancborchardt
Copy link
Member

When reading through the linked issues #7815 and #2323 – it seems that if we implement this delay setting at all, then it should be only there when auto upload is set to "Delete after upload".
Also, instead of a flexible time picker, we should do it like elsewhere in Nextcloud and have some reasonable presets. E.g.:

  • Upload immediately (default)
  • 15 minutes
  • 30 minutes
  • 1 hour
  • 24 hours
  • 1 week

These presets already seem to cover all the cases brought up in the issues, without detailed time configuration needed.

"Send later" in Nextcloud Mail "Clear status after"-picker in online status
image image

Do you see other solutions here, especially ones which don’t add another customizable setting but just do it automatically? @tobiasKaminsky @alperozturk96 @ZetaTom

@PhilLab
Copy link
Contributor

PhilLab commented Mar 3, 2025

Re @jancborchardt

When reading through the linked issues #7815 and #2323 – it seems that if we implement this delay setting at all, then it should be only there when auto upload is set to "Delete after upload".

I think this feature also makes a ton of sense without auto deletion. It allows you to clean up the taken photos before they hit the cloud. Think of taking 5 photos of the same scene, where you then immediately delete those where people have their eyes closed, etc...

So it should be available for all upload types

@jancborchardt
Copy link
Member

@PhilLab ah, thanks for the note, good point.

Then it should just be changed to the suggested intervals as mentioned.

@BatPio
Copy link
Author

BatPio commented Mar 19, 2025

@ZetaTom @jancborchardt
Thank you for the code review, I made changes

@ZetaTom
You are right, the phrase "Delay file upload" can be misleading. I changed it to "Minimum file age"

@jancborchardt
I replaced DurationPicker with a list with fixed values

After combining both ideas:
Screenshot_20250319_214900Screenshot_20250319_214929Screenshot_20250319_214942

Since the sync interval in Android is 15 minutes, I think this value is not useful. Instead, I would add values of 6 hours and 3 days

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Since the sync interval in Android is 15 minutes, I think this value is not useful. Instead, I would add values of 6 hours and 3 days

@BatPio sure, sounds good. :) Also, the wording of "No restrictions" could be changed to "Upload immediately" which might be a bit more clear?

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good from the design side now. :)

@BatPio
Copy link
Author

BatPio commented Mar 25, 2025

@alperozturk96

I've reworked the UI as suggested by @jancborchardt . Could you please review the changes?

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.

Allow setting a delay before auto upload / auto upload on-device deletion delayed / scheduled auto-upload