Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Aug 29, 2016

Fix nextcloud/files_accesscontrol#32

Steps

  1. Create files access rule to block by "File tagged with test"
  2. Upload an image
  3. Assign tag test to the image
  4. Go to personal settings, select avatar from folder and select the tagged image

Before

An error occurred: Internal Server Error (500)

After

The selected file cannot be read

@mention-bot
Copy link

@nickvergessen, thanks for your PR! By analyzing the annotation information on this pull request, we identified @MorrisJobke, @rullzer and @Kondou-ger to be potential reviewers

@nickvergessen nickvergessen force-pushed the avatar-files-accesscontrol-fixes branch from 061a41c to e18e266 Compare September 8, 2016 07:07
@rullzer
Copy link
Member

rullzer commented Sep 8, 2016

So the first part we can catch. (If preview endpoint return 403 then just display the mimetype icon).

The second one might be a bit trickier.

@rullzer
Copy link
Member

rullzer commented Sep 8, 2016

Of course all the logic in oc-dialogs is slightly different. Basically we want to call lazyloadpreview on the previews in the dialog. But for some reason this fails hard for me.

I'm not sure howe we can go around not displaying those items. As migcally filtering them out is also kind of weird.

@nickvergessen
Copy link
Member Author

Ready to review, since the open issues have been addressed by #1756

@MorrisJobke @LukasReschke @rullzer

@MorrisJobke
Copy link
Member

Ready to review, since the open issues have been addressed by #1756

Could you post reproduction steps?

@nickvergessen
Copy link
Member Author

Added steps in first post

@MorrisJobke
Copy link
Member

@nickvergessen Unit tests are failing

@nickvergessen nickvergessen force-pushed the avatar-files-accesscontrol-fixes branch from b9f75c4 to 9c3e855 Compare October 18, 2016 14:22
@nickvergessen
Copy link
Member Author

Should be fixed

@MorrisJobke
Copy link
Member

Tested and works 👍

@LukasReschke
Copy link
Member

LGTM

@LukasReschke LukasReschke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 18, 2016
@MorrisJobke
Copy link
Member

Ignore CI (hiccup of todays testing)-> merge

@MorrisJobke MorrisJobke merged commit 3c698c8 into master Oct 18, 2016
@MorrisJobke MorrisJobke deleted the avatar-files-accesscontrol-fixes branch October 18, 2016 18:26
@nickvergessen nickvergessen changed the title Avatar/file-picker fixes for access-control app Avatar fixes for access-control app Oct 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants