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] Camera Uploads manual retry #3418

Merged
merged 7 commits into from
Nov 3, 2021
Merged

Conversation

abelgardep
Copy link
Contributor

@abelgardep abelgardep commented Oct 26, 2021

Related Issues

App: #3417

Library PR (if needed):

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

QA

@abelgardep abelgardep self-assigned this Oct 26, 2021
@abelgardep abelgardep linked an issue Oct 26, 2021 that may be closed by this pull request
@abelgardep abelgardep force-pushed the fix/camera_uploads_manual_retry branch from a179ae4 to 3ad9176 Compare October 26, 2021 09:25
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

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

💯

@selius
Copy link
Contributor

selius commented Oct 26, 2021

Thanks for the fix, @abelgardep!

One issue I found is that it removed the original file, even though in my settings it's set to keep them. But that's probably because MOVE and COPY constants were mixed up. 🙂

Another thing I noticed is there's no progress bar for the upload in the notifications and the view is not refreshed after the file was successfully uploaded to the server. That is, it's still showing as "failed" and only updated when I switch to another tab and then back to Uploads.

@abelgardep
Copy link
Contributor Author

Another thing I noticed is there's no progress bar for the upload in the notifications and the view is not refreshed after the file was successfully uploaded to the server. That is, it's still showing as "failed" and only updated when I switch to another tab and then back to Uploads.

That's right @selius It's a known issue that affects camera uploads. We need to move forward with the new architecture to fix and unify the process 👍

Thank you so much for your comments and feedback. It's highly appreciated!!

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 2, 2021

(1)

  1. Enable camera uploads
  2. Take some pictures
  3. Switch server off
  4. Wait 15 minutes

Current: Files are in Failed section in uploads view. By clicking on individual rows, nothing happen. The wat to retry is clicking the retry button to retry all at once. Same behaviour with manual uploads is different. If the manual upload fails, clicking on every individual row is posible.

Expected: Same behaviour in both (feasible?)

Pixel2 Android11
786dfa1a

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 2, 2021

(2) [FIXED]

  1. Enable camera uploads
  2. Select "move" in the behaviour
  3. Take some pictures
  4. Switch server off.
  5. Wait 15 minutes
    ...
  6. When uploads fail, then switch server on and retry

Current: files are copied, not moved
Expected: files are moved, removed from internal

NOTE: the move behaviour works fine when uploads run without errors

Pixel2 Android11
786dfa1a

@abelgardep abelgardep force-pushed the fix/camera_uploads_manual_retry branch from 786dfa1 to 074faf5 Compare November 3, 2021 06:52
Copy link
Contributor

@fesave fesave left a comment

Choose a reason for hiding this comment

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

LGTM

@abelgardep
Copy link
Contributor Author

(1)

  1. Enable camera uploads
  2. Take some pictures
  3. Switch server off
  4. Wait 15 minutes

Current: Files are in Failed section in uploads view. By clicking on individual rows, nothing happen. The wat to retry is clicking the retry button to retry all at once. Same behaviour with manual uploads is different. If the manual upload fails, clicking on every individual row is posible.

Expected: Same behaviour in both (feasible?)

Pixel2 Android11 786dfa1a

Actually, if you click on any individual row, they are retried. This is the fix for it. In master they are not retried. The point here is that you can not see actual feedback about it (Only a notification if it fails again) until you enter the uploads view again.

But that is a different issue that is already in master with camera uploads retry all.

@abelgardep
Copy link
Contributor Author

About (2), yes, MOVE behavior was broken for camera upload retries. COPY was used by default, so they actually are not removed after a successful upload. I'll fix it and let you know 👍

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 3, 2021

Approved

@abelgardep abelgardep force-pushed the fix/camera_uploads_manual_retry branch from eb3363f to 0d87e74 Compare November 3, 2021 13:09
@abelgardep abelgardep merged commit 7d5d79b into master Nov 3, 2021
@abelgardep abelgardep deleted the fix/camera_uploads_manual_retry branch November 3, 2021 13:51
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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] Can't manually retry a failed camera upload
5 participants