Skip to content

[FIX] Unwanted DELETE operations when synchronization in single file fails#4408

Merged
JuancaG05 merged 5 commits into
masterfrom
fix/propfinds_with_no_name_trigger_delete
May 31, 2024
Merged

[FIX] Unwanted DELETE operations when synchronization in single file fails#4408
JuancaG05 merged 5 commits into
masterfrom
fix/propfinds_with_no_name_trigger_delete

Conversation

@JuancaG05

@JuancaG05 JuancaG05 commented May 22, 2024

Copy link
Copy Markdown
Contributor

When performing a read remote file operation (synchronization over a single file), if the account in the OwncloudClient is null, we'll throw an AccountNotFoundException. This will be handled in 3 different views:

  • In the main file list view
  • In the file details view
  • In the image preview view

In the 3 cases, we'll show a Snackbar telling Sync failed, you need to log in again, suggesting that by repeating the login process, the problem can be solved (since we'll save again the account, which might have been lost in some migration process from a very old version).

Also, we changed the way to handle the 404 error of a PROPFIND. Previously, we deleted locally and remotely the file that we requested in the PROPFIND, now we just remove it locally (since if we receive a 404, the file shouldn't exist in remote), avoiding sending DELETE requests.

Related Issues

App: https://github.com/owncloud/enterprise/issues/6638

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@JuancaG05 JuancaG05 requested a review from joragua May 22, 2024 15:38
@JuancaG05 JuancaG05 self-assigned this May 22, 2024
@JuancaG05 JuancaG05 changed the title [FIX] Unwanted DELETE operations sent by Android app [FIX] Unwanted DELETE operations when synchronization in single file fails May 22, 2024
@TheOneRing

Copy link
Copy Markdown

Sounds good to me

@joragua joragua left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! 😄

@jesmrec

jesmrec commented May 24, 2024

Copy link
Copy Markdown
Contributor

Not able to reproduce the problem of the non-existing account without tricking the code. Users' feedback turns important here, because this is not an expected behaviour and it is not in our hands to control it (OS feature).

About the DELETE sent after a 404, it's fixed. DELETE remote operation is not sent anymore, and local copy is removed 👍

@jesmrec

jesmrec commented May 24, 2024

Copy link
Copy Markdown
Contributor

Just a question here:

In the 3 cases, we'll show a Snackbar telling Sync failed, you need to log in again

When auth is lost due to an invalid token, we use to show a snackbar with the message and a button/link that prompts the user to the login view. How difficult is to add that button to the snackbar in the same way? like:

Screenshot 2024-05-24 at 12 48 38

@JuancaG05

Copy link
Copy Markdown
Contributor Author

Just a question here:

In the 3 cases, we'll show a Snackbar telling Sync failed, you need to log in again

When auth is lost due to an invalid token, we use to show a snackbar with the message and a button/link that prompts the user to the login view. How difficult is to add that button to the snackbar in the same way? like:

Screenshot 2024-05-24 at 12 48 38

Should be added now 😃

@jesmrec

jesmrec commented May 31, 2024

Copy link
Copy Markdown
Contributor

Should be added now 😃

Correctly added and working in list of files, details view and image preview. Ready to go.

@JuancaG05 JuancaG05 force-pushed the fix/propfinds_with_no_name_trigger_delete branch from ca5f5d6 to deb48e7 Compare May 31, 2024 08:27
@JuancaG05 JuancaG05 force-pushed the fix/propfinds_with_no_name_trigger_delete branch from deb48e7 to 07d08dc Compare May 31, 2024 11:45
@JuancaG05 JuancaG05 merged commit 96bc67c into master May 31, 2024
@JuancaG05 JuancaG05 deleted the fix/propfinds_with_no_name_trigger_delete branch May 31, 2024 12:07
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.

4 participants