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

Support for Deleting Shared Folders as a Share Receiver #1891

Merged
merged 16 commits into from
Jul 22, 2021

Conversation

refs
Copy link
Member

@refs refs commented Jul 14, 2021

@refs refs requested a review from labkode as a code owner July 14, 2021 20:27
@update-docs
Copy link

update-docs bot commented Jul 14, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lgtm-com
Copy link

lgtm-com bot commented Jul 14, 2021

This pull request introduces 1 alert when merging c12e29f into 089745e - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

butonic
butonic previously approved these changes Jul 14, 2021
Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

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

#nice ❤️

@labkode
Copy link
Member

labkode commented Jul 15, 2021

@refs what about putting this logic in the gateway?
If the gateway can catch a Remove request to a share it will call the share manager and set is as decline (not pending, see attached video).

@ishank011 what do you think?

unsharing.mov

@ishank011
Copy link
Contributor

If the gateway can catch a Remove request to a share it will call the share manager and set is as decline (not pending, see attached video)

+1

Instead of calling ListReceivedShares for every delete request, we can instead add that logic here

if s.isShareName(ctx, p) {
log.Debug().Msgf("path:%s points to share name", p)
ref := &provider.Reference{Path: p}
req.Ref = ref
return s.delete(ctx, req)
}

@refs
Copy link
Member Author

refs commented Jul 15, 2021

@refs what about putting this logic in the gateway?
If the gateway can catch a Remove request to a share it will call the share manager and set is as decline (not pending, see attached video).

@ishank011 what do you think?
unsharing.mov

Agree, that'd save some back and forth 👍 !

@refs
Copy link
Member Author

refs commented Jul 15, 2021

@labkode decline seems like a more appropriate state for the share. I'll update the PR to reflect it.

@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2021

This pull request introduces 1 alert when merging 8de3310 into 7df477f - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

@refs
Copy link
Member Author

refs commented Jul 19, 2021

@ishank011, @labkode if this matches the criteria, it should be ready to be merged 🎉

@ishank011
Copy link
Contributor

@refs looks good but I think we can add this parameter to DeleteRequest if we want to skip moving the resource to recycle bin. What do you think? @labkode @butonic

@ishank011 ishank011 merged commit 1cbc542 into cs3org:master Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unshared items are not actually unshared
4 participants