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

[8.2] Group share etag propagation #21044

Merged
merged 2 commits into from
Dec 10, 2015
Merged

[8.2] Group share etag propagation #21044

merged 2 commits into from
Dec 10, 2015

Conversation

icewind1991
Copy link
Contributor

For #20714

Currently only unit test for it

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @DeepDiver1975, @PVince81 and @LukasReschke to be potential reviewers

@icewind1991 icewind1991 added this to the 8.2.2-current-maintenance milestone Dec 8, 2015
@@ -411,7 +341,7 @@ public function testRecipientUploadInDirectReshare() {
$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER3]);
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);

$this->assertAllUnchaged();
$this->assertAllUnchanged();
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this will have to be forward ported to master too

Copy link
Contributor

Choose a reason for hiding this comment

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

fixing a method name... of tests... in a maintenance branch ....

@PVince81
Copy link
Contributor

PVince81 commented Dec 8, 2015

Thanks a lot!

I guess the new unit tests should later be ported to master as well for additional benefit.

@icewind1991
Copy link
Contributor Author

I'm having problems reproducing the propagation problem from the original issue

@icewind1991
Copy link
Contributor Author

Ok, I managed to reproduce a failure by re-sharing a subfolder with a group

@icewind1991
Copy link
Contributor Author

Ok, fixed the problem, ready for review

@icewind1991
Copy link
Contributor Author

@PVince81
Copy link
Contributor

PVince81 commented Dec 9, 2015

Smashbox still failing on CI for some reason ?

@icewind1991
Copy link
Contributor Author

One less failure at least

@PVince81
Copy link
Contributor

PVince81 commented Dec 9, 2015

I cherry picked locally #20949 (backport pending) to make sure the test actually pass.
lib/owncloud/test_sharePropagationInsideGroups.py does pass.

👍

@PVince81
Copy link
Contributor

PVince81 commented Dec 9, 2015

@karlitschek please approve this non-backport for 8.2.2. This is a 8.2 specific fix for etag propagation for groups. The 9.0 fix is different and cannot be backported as is.

@PVince81
Copy link
Contributor

PVince81 commented Dec 9, 2015

Needs a second reviewer @MorrisJobke @jvillafanez @DeepDiver1975 @nickvergessen @rullzer @schiesbn

@karlitschek
Copy link
Contributor

please backport 👍

@rullzer
Copy link
Contributor

rullzer commented Dec 10, 2015

awesome! Looks good and seems to fix the issue. 👍

PVince81 pushed a commit that referenced this pull request Dec 10, 2015
[8.2] Group share etag propagation
@PVince81 PVince81 merged commit c023812 into stable8.2 Dec 10, 2015
@PVince81 PVince81 deleted the group-propagation-82 branch December 10, 2015 10:24
@PVince81
Copy link
Contributor

@icewind1991 can you forward port the unit test ?

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 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.

8 participants