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 viewer editor permissions #1207

Merged
merged 1 commit into from Sep 6, 2022
Merged

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 6, 2022

image

Fix #315

@skjnldsv skjnldsv self-assigned this Sep 6, 2022
@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews labels Sep 6, 2022
@skjnldsv skjnldsv force-pushed the feat/permissions-editor-viewer branch from 394cdb4 to e736024 Compare September 6, 2022 09:37
@skjnldsv skjnldsv force-pushed the feat/permissions-editor-viewer branch from e736024 to d6a830a Compare September 6, 2022 10:09
Comment on lines +24 to +35
<d:getcontenttype />
<d:getetag />
<d:getlastmodified />
<d:resourcetype />
<nc:face-detections />
<nc:file-metadata-size />
<nc:has-preview />
<nc:realpath />
<oc:favorite />
<d:resourcetype />`
<oc:fileid />
<oc:permissions />
`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for the reordering ? just curious :)

Copy link
Member

Choose a reason for hiding this comment

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

just to mess with us :D

Copy link
Member Author

@skjnldsv skjnldsv Sep 6, 2022

Choose a reason for hiding this comment

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

Sorting alphabetically. Easier to find what you want

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@artonge artonge force-pushed the feat/permissions-editor-viewer branch from d6a830a to 510ab59 Compare September 6, 2022 11:20
@marcelklehr
Copy link
Member

Would this be needed for the tags and people views as well?

@artonge
Copy link
Collaborator

artonge commented Sep 6, 2022

I think so indeed, @skjnldsv ^

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 6, 2022

Would this be needed for the tags and people views as well?

Only if you open media on Viewer from those views 🤷

@skjnldsv skjnldsv merged commit c5f1724 into master Sep 6, 2022
@skjnldsv skjnldsv deleted the feat/permissions-editor-viewer branch September 6, 2022 15:08
@marcelklehr
Copy link
Member

Only if you open media on Viewer from those views

that's probably a yes...

@@ -109,13 +110,36 @@ private function formatData(iterable $nodes): array {
'mime' => $node->getMimetype(),
'size' => $node->getSize(),
'type' => $node->getType(),
'permissions' => $this->formatPermissions($node->getPermissions()),
Copy link
Member

Choose a reason for hiding this comment

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

Ah, and this is not the albums controller but the folders controller, probably, then we may need this for the actual albums as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, but the other Views are importing src/services/DavRequest.js, so they will benefit from that too

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but Albums and Faces dav endpoints do not expose permissions in DAV afaik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete command from sidebar list
3 participants