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

Add emitting of hook post_unshareFromSelf to Share 2.0 #28401

Merged
merged 1 commit into from
Jul 17, 2017

Conversation

paulijar
Copy link
Contributor

Description

Emit the hook signal post_unshareFromSelf when the recipient of a share unshares it. This signal used to be present in Share 1.0 but was missing from 2.0. Share 2.0 sent a signal (post_unshare) only in case the usharing was done by the file owner.

Some new key-value pairs are added to the hook parameter data as compared to 1.0. This is to make the hook more useful and to enable unified handling of the hooks post_unshare and post_unshareFromSelf.

Related Issue

#28337

Motivation and Context

There must be a way for applications to detect when a file or folder becomes unavailable. The missing signal emission is regression as compared to Share 1.0.

The parameter data of the post_unshareFromSelf signal was augmented as compared to Share 1.0. The motivation for this was to make the data more similar with the other signal post_unshare and to make the signal more useful: with the parameter content of Share 1.0, it was impossible to obtain the node ID of the unshared file/folder.

How Has This Been Tested?

Tested on top of ownCloud 9.1.3. Connected the hooks post_unshare and post_unshareFromSelf in application code and added debug logging to the connected hook functions. Tested user shares and group shares, and unshared them both from the file owner and the share recipient, and checked in each case from the log that the hooks had fired and the parameter data was as expected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed. (In my test environment, 45 out of 8928 core php test failed both before and after my changes)

- This hook used to be present in Share 1.0 but was missing from 2.0
- Some new key-value pairs have been added to the hook parameter data as compared to 1.0. This is to make the hook more useful and to enable unified handling of the hooks post_unshare and post_unshareFromSelf.
- Fixes owncloud#28337
@CLAassistant
Copy link

CLAassistant commented Jul 14, 2017

CLA assistant check
All committers have signed the CLA.

paulijar added a commit to paulijar/music that referenced this pull request Jul 14, 2017
paulijar added a commit to paulijar/music that referenced this pull request Jul 16, 2017
'itemSource' => $share->getNodeId(),
'shareType' => $shareType,
'shareWith' => $sharedWith,
'itemparent' => method_exists($share, 'getParent') ? $share->getParent() : '',
Copy link
Contributor

Choose a reason for hiding this comment

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

did you find any scenario where the share doesn't have the method getParent ??

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, you just moved the old code. ok

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@PVince81 PVince81 merged commit a36e771 into owncloud:master Jul 17, 2017
@PVince81
Copy link
Contributor

@paulijar can you backport this to stable10 and stable9.1 ? Basically resubmit a PR for stable10 and another one with stable9.1 with the same commits.

@paulijar
Copy link
Contributor Author

@PVince81 Sure, done.

@paulijar paulijar deleted the share20-post_unshareFromSelf branch July 17, 2017 10:47
@PVince81
Copy link
Contributor

stable10: #28413
stable9.1: #28412

@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants