Skip to content

[stable8.2] Fix post_unshareFromSelf hook parameter format #26364

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.

Sign up for GitHub

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

Jump to bottom
Merged
PVince81 merged 1 commit into from
Oct 18, 2016
Merged

[stable8.2] Fix post_unshareFromSelf hook parameter format #26364

PVince81 merged 1 commit into from
Oct 18, 2016

Conversation

@PVince81
Copy link
Contributor

@PVince81 PVince81 commented Oct 13, 2016

Description

When unsharing from self in a group share situation, the share items passed to the post_unshareFromSelf hook were using the wrong format in which the attribute names (ex: "share_type") have non camel-case format.

This fix makes sure that in group sharing case we use the correct format. It looks like the code was already producing it but in array_merge it was not using it and adding the unprocessed one

Related Issue

Fixes #26346 for OC 8.2 (specifically)

Motivation and Context

Fixes etag propagation that is triggered by said hook that this fixes, see #26346 (comment)

How Has This Been Tested?

Manual testing with group shares and user shares:

  • TEST: recipient unshares from self in user share scenario: no warnings, parent etag changes
  • TEST: recipient unshares from self in group share scenario: no warnings, parent etag changes
  • TEST: sharer unshares from user: no warnings, parent etag of recipient changes

Screenshots (if appropriate):

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.

@DeepDiver1975 @jvillafanez @butonic

@PVince81 PVince81 added this to the 8.2.9 milestone Oct 13, 2016
@mention-bot
Copy link

mention-bot commented Oct 13, 2016

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @schiessle, @icewind1991 and @nickvergessen to be potential reviewers.

@PVince81
Copy link
Contributor Author

PVince81 commented Oct 13, 2016

@SergioBertolinSG mind checking whether we have integration tests for etag propagation (I'm sure we do) and whether they cover these scenarios ? Since we backported most tests it seems that this situation wasn't caught.

This was referenced Oct 13, 2016
Closed
Closed
Closed
@PVince81
Copy link
Contributor Author

PVince81 commented Oct 17, 2016 •
edited
Loading

  • move unit test to the etag propagation tests
  • forward port to stable9

Fix post_unshareFromSelf hook parameter format
e7cb4ac 
When unsharing from self in a group share situation, the share items
passed to the post_unshareFromSelf hook were using the wrong format in
which the attribute names (ex: "share_type") have non camel-case format.

This fix makes sure that in group sharing case we use the correct
format. It looks like the code was already producing it but in
array_merge it was not using it and adding the unprocessed one.
@PVince81 PVince81 force-pushed the stable8.2-fix-unsharefromself-hookvalues branch from 0231100 to e7cb4ac Compare October 17, 2016 07:48
@PVince81
Copy link
Contributor Author

PVince81 commented Oct 17, 2016

Adjusted unit tests, now it looks better.

This was referenced Oct 17, 2016
Merged
Merged
Merged
@PVince81
Copy link
Contributor Author

PVince81 commented Oct 17, 2016

jvillafanez
jvillafanez reviewed Oct 18, 2016
View reviewed changes
lib/private/share/share.php
$shareTmp['fileTarget'] = $groupShare['file_target'];
}
$listOfUnsharedItems = array_merge($listOfUnsharedItems, array($groupShare));
$listOfUnsharedItems = array_merge($listOfUnsharedItems, array($shareTmp));
Copy link
Member

@jvillafanez jvillafanez Oct 18, 2016

Choose a reason for hiding this comment

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

Taking into account we don't really care about the keys in the resulting array, I think $listOfUnsharedItems[] = $shareTmp should be better.

Copy link
Contributor Author

@PVince81 PVince81 Oct 18, 2016

Choose a reason for hiding this comment

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

I only adjusted it to match what we had in the other block above for the user share. Didn't want to make more changes.

Copy link
Member

@jvillafanez jvillafanez Oct 18, 2016

Choose a reason for hiding this comment

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

Ok, fair enough.

@jvillafanez
Copy link
Member

jvillafanez commented Oct 18, 2016

👍

@PVince81 PVince81 merged commit bd081a2 into stable8.2 Oct 18, 2016
@PVince81 PVince81 deleted the stable8.2-fix-unsharefromself-hookvalues branch October 18, 2016 15:30
@lock
Copy link

lock bot commented Aug 4, 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 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jvillafanez jvillafanez jvillafanez left review comments

Assignees

No one assigned

Labels

3 - To Review feature:sharing

Projects

None yet

Milestone

8.2.9

Development

Successfully merging this pull request may close these issues.

4 participants

@PVince81 @mention-bot @jvillafanez