[tests-only] Accept both the user and the group share offered by Alice

[aduffeck]

Description

This PR changes a few tests to explicitly accept all shares for the same resource. That is required for the tests to pass in the sharesstorageprovider branch.

Types of changes

• [X] Tests only (no source changes)

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

72.0% Coverage
0.7% Duplication

[ownclouders]

:boom: Acceptance tests pipeline apiShareManagementToShares-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/32956/64/1

[phil-davis]

https://drone.owncloud.com/owncloud/core/32956/64/13

  Scenario: Merging shares for recipient when shared from outside with group and member with different permissions                 # /drone/src/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature:24
    Given user "Alice" has created folder "/merge-test-outside-perms"                                                              # FeatureContext::userHasCreatedFolder()
    When user "Alice" shares folder "/merge-test-outside-perms" with group "grp1" with permissions "read" using the sharing API    # FeatureContext::userSharesFileWithGroupUsingTheSharingApi()
    And user "Alice" shares folder "/merge-test-outside-perms" with user "Brian" with permissions "all" using the sharing API      # FeatureContext::userSharesFileWithUserUsingTheSharingApi()
    And user "Brian" has accepted the first pending share "/merge-test-outside-perms" offered by user "Alice"                      # FeatureContext::userHasAcceptedThePendingShareOfferedBy()
    And user "Brian" has accepted the next pending share "/merge-test-outside-perms" offered by user "Alice"                       # FeatureContext::userHasAcceptedThePendingShareOfferedBy()
      Sharing::userReactsToShareOfferedBy could not find share /Shares/merge-test-outside-perms, offered by Alice to Brian
      Failed asserting that null is not null.

What does the server actually do when there are 2 shares pending for the same resource? When Brian accepts the first share, does the server automagically "accept" both shares and create a merged view of them for Brian?

And what is the server supposed to do? (What is the requirement?)

Maybe after accepting the first share, there should not be any more pending shares. If that is so, then there can be a Then step to check that.

The existing Then steps are expecting to end up with a single "merged" share. It's tricky to write test scenarios after the code has been implemented and when we are not sure what the exact defined/required behavior is supposed to be!

[aduffeck]

@phil-davis the server accepts/rejects the shares by id, it doesn't automagically accept all shares to the same resource if one of them is accepted, for example. But once multiple shares for the same resources have been accepted the permissions will be merged. After the first accept request was sent the according share will no longer be part of the pending shares list so the next one in line can be picked up. So in the end we need two requests to be sent and in https://github.com/owncloud/core/pull/39112 I tried to make the accepts share step do them both at once. @individual-it rightfully claimed it was not the best way of doing it so I'm now using the has accepted the first pending and has accepted the next pending steps which make it clearer what is going on in my opinion.

[butonic]

hm, so in oc10: there is no longer a pending share after the first share was accepted:

797 | And user "Brian" has accepted the next pending share "/merge-test-outside-perms" offered by user "Alice"                       # FeatureContext::userHasAcceptedThePendingShareOfferedBy()
798 | Sharing::userReactsToShareOfferedBy could not find share /Shares/merge-test-outside-perms, offered by Alice to Brian
799 | Failed asserting that null is not null.

@aduffeck so the ocs apps files sharing api implementation needs to 'hide' other pending shares after the first one was accepted .... In other words: as soon as the user accepts a 'resource' (not a share) all incoming shares for it are hidden?

Does that make sense? cc @phil-davis

[aduffeck]

@butonic but the user accepts actual shares not resources, so unless I'm missing something that would mean that calls to the id based API would implicitly affect other resources too which is not very intuitive, rather to the contrary. Do we have to copy that behaviour in ocis?

[butonic]

@aduffeck the ui merges multiple shares for the same resource into a single ui element, reflecting the same underlying resource. What actually happens when the user accepts this 'resource' is that he wants to mount the resource somewhere: the /Shares folder or in oc10 in an arbitrary location. Technically, we 'accept' a share, by sending a POST request that contains a share id. The ocs API does not reflect resource based share management here.

Why would multiple resouces be affected by a single share change?

[aduffeck]

@butonic I was referring to the actual shares which are the resources being handled by the sharing API, not the resource being shared. Changing one share using this API (referenced by its ID) is also implicitly changing other shares. But I get that the discrepancy between workflow and API is something that needs to stay at that point.

[phil-davis]

@aduffeck the acceptance test scenarios and code used in ocis drone CI were all copied to the ocis repo some time ago. So this PR to core is no longer relevant. Close?