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

[Share 2.0] Add missing post_update_permissions hook #22118

Merged
merged 2 commits into from
Feb 5, 2016

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Feb 4, 2016

@nickvergessen as prommised.

CC: @schiesbn @nickvergessen @PVince81

@rullzer rullzer added this to the 9.0-current milestone Feb 4, 2016
@PVince81
Copy link
Contributor

PVince81 commented Feb 4, 2016

Code looks good 👍

I'll have a look at the failing JS tests separately, seems to be random.

'shareWith' => $share->getSharedWith(),
'uidOwner' => $share->getSharedBy(),
'permissions' => $share->getPermissions(),
'path' => $share->getNode()->getPath(),
Copy link
Contributor

Choose a reason for hiding this comment

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

While I prefer this one, the old hook sent the local path, not the absolute path

8.2

Info    sharing_log     User admin shared "/test2" with the user test1, permissions: READ UPDATE CREATE DELETE SHARE [CLIENT_IP: 127.0.0.1]     2016-02-04T12:11:01+00:00 
Info    sharing_log     User admin updated the permissions for "/test/test2" for the user test1, permissions: READ UPDATE CREATE DELETE [CLIENT_IP: 127.0.0.1]  2016-02-04T12:11:02+00:00

This PR

Info    admin_audit     User admin shared "/test2" with the user test1, permissions: READ UPDATE CREATE DELETE SHARE [CLIENT_IP: 127.0.0.1]     2016-02-04T12:11:53+00:00 
Info    admin_audit     User admin updated the permissions for "/admin/files/test/test2" for the user test1, permissions: READ UPDATE CREATE DELETE [CLIENT_IP: 127.0.0.1]  2016-02-04T12:11:54+00:00

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed... should be the relative path now...

@PVince81
Copy link
Contributor

PVince81 commented Feb 4, 2016

Triple unlucky:

  • JSUnit test unrelated: Fix jsunit filesclient #22130 (please review)
  • Swift test: "MySQL has gone away"
  • Smashbox: hudson.remoting.ChannelClosedException: channel is already closed

@nickvergessen
Copy link
Contributor

👍 Fixes admin_audit

@DeepDiver1975
Copy link
Member

needs rebase

@rullzer rullzer force-pushed the post_update_permissions_hook branch from eb195a3 to 1698749 Compare February 4, 2016 18:19
@rullzer
Copy link
Contributor Author

rullzer commented Feb 4, 2016

Rebased

DeepDiver1975 added a commit that referenced this pull request Feb 5, 2016
[Share 2.0] Add missing post_update_permissions hook
@DeepDiver1975 DeepDiver1975 merged commit c8e136b into master Feb 5, 2016
@DeepDiver1975 DeepDiver1975 deleted the post_update_permissions_hook branch February 5, 2016 09:04
@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 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