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

[FEATURE REQUEST] Auto-refresh when a file is uploaded #4199

Merged
merged 12 commits into from
Nov 13, 2023

Conversation

JuancaG05
Copy link
Collaborator

@JuancaG05 JuancaG05 commented Oct 23, 2023

Related Issues

App: #4103

  • Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

Checklist:

#4199 (comment)

Reports:

@JuancaG05 JuancaG05 self-assigned this Oct 23, 2023
@JuancaG05 JuancaG05 linked an issue Oct 23, 2023 that may be closed by this pull request
7 tasks
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

Some change here @JuancaG05

@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch from 296e26c to f37dcb0 Compare October 24, 2023 11:14
@JuancaG05 JuancaG05 changed the base branch from master to stable October 24, 2023 11:14
@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch 2 times, most recently from 314acc7 to 7071cd6 Compare October 24, 2023 11:49
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

LGTM!!

@JuancaG05 JuancaG05 changed the title [FEATURE REQUEST] Show files on main screen after being created [FEATURE REQUEST] Auto-refresh when a file is uploaded Oct 24, 2023
@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch from 017a5f1 to 7c0d73f Compare October 24, 2023 12:12
@jesmrec
Copy link
Collaborator

jesmrec commented Oct 30, 2023

Manual uploads

  • 1 file
  • 10 files
  • 100 files
  • 200 files (updating New content)
  • 500 files (updating New content)
  • 500 files (without updating New content, checking in web)
  • 1000 files (updating new content) -> performance falls a little bit
  • 1000 files (without updating New content, checking in web)
  • 1000 files (outside the target folder)
  • 1000 files (changing tab) -> performance not so good
  • Changing number of refreshes max_uploads_to_refresh
  • "New content" removed if manual refresh happens
  • "New content" removed if browsing back and forth
  • "New content" removed if browsing to another tab

Other (regression):

  • Auto uploads
  • Uploads from camera
  • Uploads from 3rd party

Error:

  • Connection error
  • Server connection error
  • Maintenance mode (oC10)

@JuancaG05 JuancaG05 changed the base branch from stable to release/4.1.2 October 30, 2023 08:55
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

LGTM!!

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 6, 2023

(1)

  1. Upload a number of files > max_uploads_to_refresh
  2. When NEW CONTENT is displayed, browse to another folder without refreshing.

Current: NEW CONTENT stays in the screen
Expected: not totally sure... i'd think for the NEW CONTENT to update the content of the current folder in case new items where uploaded to that location. If i move to another location, it loses the meaning because this place is updated since it was just browsed. So, i'd go to remove the NEW CONTENT if browsing to another place

Pixel 2 Android 11
e2404875

@JuancaG05
Copy link
Collaborator Author

@jesmrec (1) should be fixed now 👍

@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch from 3f77281 to 5382218 Compare November 6, 2023 13:36
@JuancaG05 JuancaG05 changed the base branch from release/4.1.2 to master November 6, 2023 13:36
@jesmrec
Copy link
Collaborator

jesmrec commented Nov 7, 2023

i'd raise the default number of uploads to refresh to 200. In terms of performance is affordable and reduce the number of interactions.

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 7, 2023

(2)

  1. Upload a number of files > max_uploads_to_refresh
  2. Just after submitting the upload, browse to another folder, no matter which one and stay there

Current: After some time (when the number of finished uploads overtakes the limit), the fab to refresh is displayed
Expected: Never shown in that folder since this is not the folder to refresh

Pixel 2 Android 11
d7c56dd61

@JuancaG05
Copy link
Collaborator Author

@jesmrec (2) should be fixed now.
I also updated the maximum number of uploads to refresh automatically to 200, and I introduced a change that I hope helps with performance: manual refresh button will trigger a new refresh only when there is not another one in progress, otherwise it will show a snackbar informing the user there is already a synchronization in progress

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 9, 2023

This is approved on my side.

Just as clarification: for a very big amount of files, performance could be affected since device affords too many operations at the time: uploads management including statuses changes, UI updates and long PROPFINFDs parsing that triggers also DB updates. For a reasonable amount of files (1000 - 1500) at the time, this is affordable for a budget device. But, i can not assure that a much bigger amount is going to be performant, depending on the device.

Open to iterate here somehow.

@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch from 49187b0 to c15a4f6 Compare November 13, 2023 07:23
@JuancaG05 JuancaG05 force-pushed the feature/refresh_after_upload branch 2 times, most recently from 330483d to f42f74c Compare November 13, 2023 07:33
@JuancaG05 JuancaG05 merged commit d0ff5a9 into master Nov 13, 2023
@JuancaG05 JuancaG05 deleted the feature/refresh_after_upload branch November 13, 2023 08:22
Aitorbp pushed a commit that referenced this pull request Feb 5, 2024
[FEATURE REQUEST] Auto-refresh when a file is uploaded
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.

[FEATURE REQUEST] Auto-refresh when a file is uploaded
3 participants