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 Issue #4402 : Ensure all files are synchronized when setting folder to availab… #4407

Closed
wants to merge 1 commit into from

Conversation

praveen7512
Copy link

…le offline

  • Added a new parameter isActionSetFolderAvailableOffline to the SynchronizeFileUseCase.Params data class to handle forced synchronization when setting a folder to available offline.

  • Updated SynchronizeFileUseCase to check for the isActionSetFolderAvailableOffline parameter and force a refresh of the folder's contents.

  • Implemented the forceRefresh method to initiate a download of the file regardless of the etag comparison when isActionSetFolderAvailableOffline is true.

  • Ensured that locally deleted files are re-downloaded if they still exist on the remote server when the parent folder is set to available offline.

  • Added appropriate logging to track the synchronization process and forced refresh actions.

This fix addresses the issue where setting a folder to available offline did not correctly synchronize all its contents, especially when some files were deleted locally. Now, all files within the folder will be synchronized and set to available offline as expected.

Related Issues

App:

  • 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

…le offline

- Added a new parameter `isActionSetFolderAvailableOffline` to the `SynchronizeFileUseCase.Params` data class to handle forced synchronization when setting a folder to available offline.

- Updated `SynchronizeFileUseCase` to check for the `isActionSetFolderAvailableOffline` parameter and force a refresh of the folder's contents.

- Implemented the `forceRefresh` method to initiate a download of the file regardless of the etag comparison when `isActionSetFolderAvailableOffline` is true.

- Ensured that locally deleted files are re-downloaded if they still exist on the remote server when the parent folder is set to available offline.
- Added appropriate logging to track the synchronization process and forced refresh actions.

This fix addresses the issue where setting a folder to available offline did not correctly synchronize all its contents, especially when some files were deleted locally. Now, all files within the folder will be synchronized and set to available offline as expected.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jesmrec
Copy link
Collaborator

jesmrec commented May 21, 2024

Thanks for your contribution. We have another PR that fixes #4402, that is #4403... we will check yours as well.

@JuancaG05
Copy link
Collaborator

Thanks for your contribution. We have another PR that fixes #4402, that is #4403... we will check yours as well.

That's right 😄, in any case, @praveen7512 you need to sign the Contributor License Agreement to be able to contribute to the ownCloud project (check #4407 (comment)) 👍

@Aitorbp
Copy link
Contributor

Aitorbp commented Jun 7, 2024

Hello @praveen7512! Thank you very much for sending us this PR. I advise you that before you start working on an issue, you see if there is already someone else working on it with another solution. I recommend you take a look at this PR where the solution is more accurate than the one you propose.

I'm afraid I'm going to close this PR because it's invalid, due to the following points:

  1. First of all, you've implemented the development logic in the wrong class. Before the code goes to SynchronizeFileUseCase, it has to go through SynchronizeFolderUseCase. It's in the fileRepository.refreshFolder function where you have to check if we've clicked on the available offline option. If you go back to the case I proposed in the issue, with the implementation you've developed it's not going to work.
  2. On the other hand, the variable isActionSetFolderAvailable Offline is not set to true in any case. Therefore it will always be set to false. Also in the FileOperationsHelper.java class you need to set the value of the parameter isActionSetFolderAvailable, otherwise Android Studio won't compile.
  3. Finally, the forceRefresh function is not necessary, since the problem is not that the data is not downloaded and synchronized, but that the files that are to be made available offline are not being transferred correctly.

So, due to all these errors and that it implies the majority of code in this PR, I'm closing it. In any case, thanks for your PR and do not hesitate to open more PRs, we will be happy to review your code again. 😃

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.

None yet

5 participants