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

Handle error 423 (locked file) #4285

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

parneet-guraya
Copy link
Contributor

Fixes: #4282

What this PR includes?

  • throwing exception when get HTTP 423 (resource locked)
  • handling the exception by showing snackbar
Record_2024-01-17-01-46-07.mp4

@jesmrec The reason I believe this exception occurs is when file is being accessed, like in this case when the video is being fetched and we try to remove the file then it throws the exception.

@parneet-guraya
Copy link
Contributor Author

@JuancaG05 hey could you give it a quick look :-)

@JuancaG05
Copy link
Collaborator

I'll take a look ASAP. In this case @Aitorbp will take a look as well so that he gets familiar with how errors flow from network responses to UI 🙂

Copy link
Contributor

@Aitorbp Aitorbp left a comment

Choose a reason for hiding this comment

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

The development seems correct to me. Follow the logic of the other cases. I only saw a small formatting correction. 👍

@parneet-guraya
Copy link
Contributor Author

parneet-guraya commented Jan 23, 2024

I updated with requested changes 👍

@JuancaG05 JuancaG05 changed the title Handle resource locked 423 Handle error 423 (locked file) Jan 23, 2024
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.

Approved from my side! Good job @parneet-guraya! 🚀

@jesmrec
Copy link
Collaborator

jesmrec commented Jan 24, 2024

Testing the PR i notice a side-effect (not regressive):

  1. Lock a picture file
  2. Open the picture in the app to preview it
  3. Remove the picture from the preview

snackbar is not displayed

Locking a file can be easily done in oC10 servers. Log in an admin account > Settings (top right menu) > General (left menu, admin side) > tick Enable manual file locking in the web interface

Once enabled, every file (not folder) in the list will have an option Lock file available. Operations over the locked file will return 423

This could be fixed here or moved to another PR.

All the places in which i tested and the message appears are correct.

Tested with different kind of files and previews (pictures, txt, video), performing operations: Move, Remove, Move, Rename (operations not allowed for a locked file). All of them fine.

@parneet-guraya
Copy link
Contributor Author

@jesmrec Will check and push the hotfix here if it's simple enough otherwise we can solve it by raising another issue

@parneet-guraya
Copy link
Contributor Author

Hi @jesmrec, I have been using demo.owncloud.com for accessing limited demo server. So, I think this one does not have the privilege to enable/disable file locking and you mentioned admin account and I'm not sure how to get access to that. Can you help out here? :-)

@jesmrec
Copy link
Collaborator

jesmrec commented Jan 25, 2024

demo.owncloud.com can be accessed with admin/admin credentials. Then, the steps mentioned above. Let me know if you run into any trouble.

@parneet-guraya
Copy link
Contributor Author

@jesmrec Hi, the case you said, I was able to reproduce at first attempt. After investigating, the error was getting thrown correctly but at UI level the throwable turns out to be null (couldn't get the chance to see further) that's why it was showing the generic unknown error message.

But, now I can't reproduce it anymore. Let me know if you still see this otherwise we may push this one and later on create separate issue and pickup from there if it ever starts coming again.

Record_2024-01-25-16-41-02.mp4

@jesmrec
Copy link
Collaborator

jesmrec commented Jan 25, 2024

i don't see the unknown message anymore, when i try to remove a picture from its preview, there is no snackbar. I still reproduce this consistently, but, as i commented, it's not a regression.

Let's move this forward and i will create another issue since it seems that reproducing is not so straightforward.

@JuancaG05
Copy link
Collaborator

Need a rebase here if you don't mind @parneet-guraya 😄.

BTW, to automatize changelog, we are using a tool called Calens since some time ago. What we need to do is adding in each PR a plain text file in a certain folder with certain features for it to be recognised. Do you feel like adding them from now on? You can find more info here, and one example here (but you can check all of our PR to see more examples 😉). Just if you want, otherwise we can take care of it, thanks! 👍

@parneet-guraya
Copy link
Contributor Author

Sure, I can do it :-)

Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
@parneet-guraya parneet-guraya force-pushed the handle-http-423 branch 2 times, most recently from 9efee5f to a65868d Compare January 26, 2024 12:07
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
@parneet-guraya
Copy link
Contributor Author

I did add the changelog file in unreleased dir as the doc says. Do I also need to add into Changelog.md file?

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.

Thanks a lot @parneet-guraya! It's perfect like that! CHANGELOG.md will be modified automatically when merged this into master or when creating a new branch, that's the magic of Calens! 😄

@JuancaG05 JuancaG05 merged commit d6ea3b5 into owncloud:master Jan 26, 2024
4 checks passed
@parneet-guraya parneet-guraya deleted the handle-http-423 branch January 28, 2024 00:17
Aitorbp pushed a commit that referenced this pull request Feb 5, 2024
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.

Handle error 423 (locked file)
4 participants