add filter by sharetype in the ocs API by C0rby · Pull Request #2050 · cs3org/reva

C0rby · 2021-09-07T10:20:22Z

Added a query parameter to the OCS API to filter the received shares by type.

labkode

Looks good to me but I need @butonic, @refs to approve as well.

butonic · 2021-09-08T07:40:25Z

The share_type parameter was introduced to oc10 with owncloud/core#38053

butonic

could you add the filter to the sql driver in

reva/pkg/share/manager/sql/sql.go

Line 278 in eacfb8a

if f.Type == collaboration.Filter_TYPE_RESOURCE_ID {

and maybe in

reva/pkg/cbox/share/sql/sql.go

Line 283 in eacfb8a

if f.Type == collaboration.Filter_TYPE_RESOURCE_ID {

as well?

if not track it in a new issue.

C0rby · 2021-09-08T07:59:30Z

could you add the filter to the sql driver in

Ok let me try.

C0rby · 2021-09-08T09:31:57Z

@butonic @labkode, I added the filters to the sql share managers. Please have a look at my changes. Also I couldn't test these changes since I don't know how/what to setup for the sql share managers. If you have some documentation or some tips, that would be great.

lgtm-com · 2021-09-08T09:43:22Z

This pull request introduces 2 alerts when merging 8f0de6e into f2109fc - view on LGTM.com

new alerts:

2 for Useless assignment to local variable

C0rby · 2021-09-08T10:28:40Z

This pull request introduces 2 alerts when merging 8f0de6e into f2109fc - view on LGTM.com

new alerts:
* 2 for Useless assignment to local variable

I resolved this but the comment doesn't disappear and I also can't remove it.

rhafer

LGTM. At least I didn't find something obviously wrong here. (Take this with a grain of salt, as I am still pretty new to the code base)

refs

Just a pedantic comment from mine, nice PR and nice expected failures cleanup! Thanks!

pkg/cbox/share/sql/sql.go

C0rby · 2021-09-20T12:41:36Z

The tests fail now because of this PR #2072. Because of that PR rejected shares are not shown in ListReceivedShares responses and there is no way to re-accept or even list a rejected share.

@labkode, how do we want to go forward with this? I think setting the filter in the ocdav layer would be better and then making that behavior configurable. Then an admin could choose if rejected shares should be shown or not.

cc @micbar @butonic

labkode · 2021-09-28T09:44:41Z

Hi @C0rby that PR should do nothing about accepting or denying shares as we know them today. That PR handles a specific type of share that denies access (DENIAL). That type of share cannot be accepted or rejected by the receiver, the receiver basically can't do anything about it.

The filter set in that PR is to not shown this type of special shares to the receiving user, NOT to filter out reject shares.

Let me know if you need further clarifications.

C0rby · 2021-09-28T09:49:52Z

The filter set in that PR is to not shown this type of special shares to the receiving user, NOT to filter out reject shares.

I see, then I misunderstood this filter. I will rewrite my code then to reflect the correct behavior.

C0rby · 2021-09-28T10:06:06Z

@labkode, how do I recognize such deny shares?
Is a share a deny share when DenyGrant is set to true or how can I filter them out in the json and memory manager?

labkode · 2021-09-28T14:32:59Z

Hi @C0rby, in the SQL manager we set a DenyShare as having permissions=0, so you can do the same on the other drivers, I think is semantically meaningful.

ishank011

Hi @C0rby this is really nice! Just left a couple of comments

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

pkg/cbox/share/sql/sql.go

pkg/share/manager/json/json.go

C0rby · 2021-10-05T12:17:30Z

Let me just add some unit tests for the new functions and then this PR is good to go. :)

C0rby · 2021-10-05T13:03:30Z

It's ready now.

C0rby force-pushed the sharetype-filter branch 3 times, most recently from e593447 to eacfb8a Compare September 7, 2021 16:08

C0rby marked this pull request as ready for review September 7, 2021 16:25

C0rby requested a review from labkode as a code owner September 7, 2021 16:25

C0rby self-assigned this Sep 7, 2021

C0rby requested review from butonic and refs September 7, 2021 16:25

labkode previously approved these changes Sep 8, 2021

View reviewed changes

butonic previously approved these changes Sep 8, 2021

View reviewed changes

C0rby dismissed stale reviews from butonic and labkode via 8f0de6e September 8, 2021 09:29

C0rby requested review from butonic and labkode September 8, 2021 09:30

C0rby force-pushed the sharetype-filter branch 2 times, most recently from 1c991f3 to 5f7b705 Compare September 8, 2021 09:58

rhafer previously approved these changes Sep 8, 2021

View reviewed changes

butonic previously approved these changes Sep 8, 2021

View reviewed changes

refs previously approved these changes Sep 13, 2021

View reviewed changes

pkg/cbox/share/sql/sql.go Outdated Show resolved Hide resolved

pkg/cbox/share/sql/sql.go Show resolved Hide resolved

C0rby dismissed stale reviews from refs, butonic, and rhafer via 216679b September 13, 2021 13:10

C0rby force-pushed the sharetype-filter branch from 5f7b705 to 216679b Compare September 13, 2021 13:10

C0rby requested review from butonic and refs September 13, 2021 13:11

C0rby requested review from butonic and refs September 17, 2021 14:57

C0rby force-pushed the sharetype-filter branch 2 times, most recently from 33b27dd to 06bce1c Compare September 20, 2021 09:41

C0rby force-pushed the sharetype-filter branch from 06bce1c to e0c8e47 Compare September 28, 2021 09:15

C0rby force-pushed the sharetype-filter branch 2 times, most recently from c88c57d to d901f66 Compare September 28, 2021 13:02

C0rby force-pushed the sharetype-filter branch from d901f66 to 65d8cfc Compare September 28, 2021 15:48

ishank011 reviewed Oct 4, 2021

View reviewed changes

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go Show resolved Hide resolved

pkg/cbox/share/sql/sql.go Outdated Show resolved Hide resolved

pkg/share/manager/json/json.go Outdated Show resolved Hide resolved

add filter by sharetype in the ocs API

4ab4bdb

C0rby force-pushed the sharetype-filter branch from 65d8cfc to 46f3b4b Compare October 4, 2021 15:22

enhance filter handling in the sql share drivers

9a2ed3a

C0rby force-pushed the sharetype-filter branch 3 times, most recently from 5c6046b to c39d980 Compare October 5, 2021 11:50

C0rby marked this pull request as draft October 5, 2021 12:16

correct and simplify filter checks

d9dfebe

C0rby force-pushed the sharetype-filter branch from c39d980 to d9dfebe Compare October 5, 2021 12:29

C0rby marked this pull request as ready for review October 5, 2021 12:29

C0rby requested a review from ishank011 October 5, 2021 12:30

ishank011 approved these changes Oct 5, 2021

View reviewed changes

ishank011 merged commit c40fcc8 into cs3org:master Oct 5, 2021

C0rby deleted the sharetype-filter branch October 5, 2021 14:23

Conversation

C0rby commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

labkode left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

butonic commented Sep 8, 2021

Uh oh!

butonic left a comment

Choose a reason for hiding this comment

Uh oh!

C0rby commented Sep 8, 2021

Uh oh!

C0rby commented Sep 8, 2021

Uh oh!

lgtm-com bot commented Sep 8, 2021

Uh oh!

C0rby commented Sep 8, 2021

Uh oh!

rhafer left a comment

Choose a reason for hiding this comment

Uh oh!

refs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

C0rby commented Sep 20, 2021

Uh oh!

labkode commented Sep 28, 2021

Uh oh!

C0rby commented Sep 28, 2021

Uh oh!

C0rby commented Sep 28, 2021

Uh oh!

labkode commented Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ishank011 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

C0rby commented Oct 5, 2021

Uh oh!

C0rby commented Oct 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

C0rby commented Sep 7, 2021 •

edited

Loading

labkode left a comment •

edited

Loading

labkode commented Sep 28, 2021 •

edited

Loading