Skip to content

Fix sharing tab state not matching resharing admin settings #24042

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
juliusknorr merged 5 commits into from
Nov 11, 2020
Merged

Fix sharing tab state not matching resharing admin settings #24042

juliusknorr merged 5 commits into from
Nov 11, 2020

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Nov 10, 2020 •
edited
Loading

This fixes a regression introduced probably in Nextcloud 18 when the Sharing tab was moved to Vue (as it worked in Nextcloud 17).

When resharing was disabled in admin settings the menu of a share showed the option to enable or disable resharing for that share, and the share input to search users, groups... to share a file was enabled also for received shares. As resharing was disabled using the menu or the input caused an error.

Besides that I have also added integration tests to check the default share permissions. I thought that there was an issue with the default share permissions, but after checking again I can no longer reproduce 🤷 But as I had already written the tests... I thought that it would be good to add them anyway :-)

It seems that I messed building the JavaScript files... I will change to To review after fixing it.

@danxuliu danxuliu added this to the Nextcloud 21 milestone Nov 10, 2020
@danxuliu
Copy link
Member Author

danxuliu commented Nov 10, 2020

/backport to stable20

@danxuliu
Copy link
Member Author

danxuliu commented Nov 10, 2020

/backport to stable19

@danxuliu
Copy link
Member Author

danxuliu commented Nov 10, 2020

/backport to stable18

@danxuliu danxuliu force-pushed the fix-sharing-tab-state-not-matching-resharing-admin-settings branch from 53022bc to 92770ed Compare November 11, 2020 00:40
@faily-bot
Copy link

faily-bot bot commented Nov 11, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 35351: failure

integration-sharing-v1

  • build/integration/sharing_features/sharing-v1.feature:429
Show full log 
[Wed Nov 11 00:46:28 2020] 127.0.0.1:47384 [200]: /ocs/v1.php/apps/testing/api/v1/app/sharebymail/enforcePasswordProtection
  Scenario: getting all shares of a file with a user with resharing rights but not yourself        # /drone/src/build/integration/sharing_features/sharing-v1.feature:429
[Wed Nov 11 00:46:28 2020] 127.0.0.1:47406 [404]: /ocs/v2.php/cloud/users/user0
[Wed Nov 11 00:46:29 2020] 127.0.0.1:47438 [200]: /ocs/v1.php/cloud/users
[Wed Nov 11 00:46:29 2020] 127.0.0.1:47474 [200]: /ocs/v1.php/cloud/users/user0
[Wed Nov 11 00:46:30 2020] 127.0.0.1:47506 [200]: /ocs/v2.php/cloud/users/user0
    Given user "user0" exists                                                                      # SharingContext::assureUserExists()
[Wed Nov 11 00:46:30 2020] 127.0.0.1:47522 [404]: /ocs/v2.php/cloud/users/user1
[Wed Nov 11 00:46:30 2020] 127.0.0.1:47536 [200]: /ocs/v1.php/cloud/users
[Wed Nov 11 00:46:31 2020] 127.0.0.1:47564 [200]: /ocs/v1.php/cloud/users/user1
[Wed Nov 11 00:46:31 2020] 127.0.0.1:47594 [200]: /ocs/v2.php/cloud/users/user1
    And user "user1" exists                                                                        # SharingContext::assureUserExists()
[Wed Nov 11 00:46:31 2020] 127.0.0.1:47614 [404]: /ocs/v2.php/cloud/users/user2
[Wed Nov 11 00:46:32 2020] 127.0.0.1:47636 [200]: /ocs/v1.php/cloud/users
[Wed Nov 11 00:46:32 2020] 127.0.0.1:47674 [200]: /ocs/v1.php/cloud/users/user2
[Wed Nov 11 00:46:32 2020] 127.0.0.1:47698 [200]: /ocs/v2.php/cloud/users/user2
    And user "user2" exists                                                                        # SharingContext::assureUserExists()
[Wed Nov 11 00:46:32 2020] 127.0.0.1:47714 [404]: /ocs/v2.php/cloud/users/user3
[Wed Nov 11 00:46:33 2020] 127.0.0.1:47732 [200]: /ocs/v1.php/cloud/users
[Wed Nov 11 00:46:33 2020] 127.0.0.1:47774 [200]: /ocs/v1.php/cloud/users/user3
[Wed Nov 11 00:46:33 2020] 127.0.0.1:47804 [200]: /ocs/v2.php/cloud/users/user3
    And user "user3" exists                                                                        # SharingContext::assureUserExists()
[Wed Nov 11 00:46:34 2020] 127.0.0.1:47820 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares?path=textfile0.txt
[Wed Nov 11 00:46:34 2020] 127.0.0.1:47836 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares
[Wed Nov 11 00:46:34 2020] 127.0.0.1:47850 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares?path=textfile0.txt
    And file "textfile0.txt" of user "user0" is shared with user "user1"                           # SharingContext::assureFileIsShared()
[Wed Nov 11 00:46:34 2020] 127.0.0.1:47866 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares/pending/27
    And user "user1" accepts last share                                                            # SharingContext::userAcceptsLastShare()
[Wed Nov 11 00:46:35 2020] 127.0.0.1:47884 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares?path=textfile0.txt
[Wed Nov 11 00:46:35 2020] 127.0.0.1:47910 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares
[Wed Nov 11 00:46:35 2020] 127.0.0.1:47924 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares?path=textfile0.txt
    And file "textfile0.txt" of user "user0" is shared with user "user2"                           # SharingContext::assureFileIsShared()
    And As an "user1"                                                                              # SharingContext::asAn()
[Wed Nov 11 00:46:35 2020] 127.0.0.1:47944 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares?path=textfile0%20(2).txt&reshares=true
    When sending "GET" to "/apps/files_sharing/api/v1/shares?path=textfile0 (2).txt&reshares=true" # SharingContext::sendingTo()
    Then the OCS status code should be "100"                                                       # SharingContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "200"                                                       # SharingContext::theHTTPStatusCodeShouldBe()
    And User "user1" should not be included in the response                                        # SharingContext::checkSharedUserNotInResponse()
    And User "user2" should be included in the response                                            # SharingContext::checkSharedUserInResponse()
      Failed asserting that false matches expected true.
    And User "user3" should not be included in the response                                        # SharingContext::checkSharedUserNotInResponse()
[Wed Nov 11 00:46:36 2020] 127.0.0.1:47960 [200]: /ocs/v1.php/cloud/users/user0
[Wed Nov 11 00:46:36 2020] 127.0.0.1:47988 [404]: /ocs/v2.php/cloud/users/user0
[Wed Nov 11 00:46:37 2020] 127.0.0.1:48008 [200]: /ocs/v1.php/cloud/users/user1
[Wed Nov 11 00:46:37 2020] 127.0.0.1:48032 [404]: /ocs/v2.php/cloud/users/user1
[Wed Nov 11 00:46:37 2020] 127.0.0.1:48044 [200]: /ocs/v1.php/cloud/users/user2
[Wed Nov 11 00:46:38 2020] 127.0.0.1:48062 [404]: /ocs/v2.php/cloud/users/user2
[Wed Nov 11 00:46:38 2020] 127.0.0.1:48074 [200]: /ocs/v1.php/cloud/users/user3
[Wed Nov 11 00:46:38 2020] 127.0.0.1:48092 [404]: /ocs/v2.php/cloud/users/user3

@danxuliu danxuliu force-pushed the fix-sharing-tab-state-not-matching-resharing-admin-settings branch from 92770ed to ee852d7 Compare November 11, 2020 01:35
@danxuliu danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 11, 2020
@danxuliu danxuliu marked this pull request as ready for review November 11, 2020 07:22
rullzer
rullzer approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Seems to do the trick. but we should move a lot of those config stuff to the capabilities I think.

@rullzer
Copy link
Member

rullzer commented Nov 11, 2020

Seems to do the trick. but we should move a lot of those config stuff to the capabilities I think.

But in a future PR of course

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 11, 2020
@juliusknorr juliusknorr merged commit cdfad9a into master Nov 11, 2020
@juliusknorr juliusknorr deleted the fix-sharing-tab-state-not-matching-resharing-admin-settings branch November 11, 2020 08:28
This was referenced Nov 11, 2020
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzer rullzer rullzer approved these changes

@juliusknorr juliusknorr juliusknorr approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: sharing regression

Projects

None yet

Milestone

Nextcloud 21

Development

Successfully merging this pull request may close these issues.

6 participants

@danxuliu @rullzer @juliusknorr @skjnldsv @ChristophWurst