Merge pull request #8827 from 2403905/issue-8731

fix unmount item from share

.drone.env

@@ -1,3 +1,3 @@
 # The test runner source for UI tests
-WEB_COMMITID=5367936f30b07c0832fb8af09f9f7719cccf7038
-WEB_BRANCH=master
+WEB_COMMITID=9b9616b8429432002d06c4a2c8f252cbe08fb735
+WEB_BRANCH=changeResponseCodeDelete

changelog/unreleased/fix-graph-unmount-item.md

@@ -0,0 +1,6 @@
+Bugfix: Fix unmount item from share
+
+We fixed the status code returned for the request to delete a driveitem.
+
+https://github.com/owncloud/ocis/pull/8827
+https://github.com/owncloud/ocis/issues/8731

services/graph/pkg/service/v0/api_drives_drive_item.go

@@ -102,6 +102,9 @@ func (s DrivesDriveItemService) UnmountShare(ctx context.Context, resourceID sto
 	if err != nil {
 		return err
 	}
+	if len(receivedSharesResponse.GetShares()) == 0 {
+		return errorcode.New(errorcode.InvalidRequest, "invalid itemID")
+	}
 
 	var errs []error
 
@@ -274,7 +277,8 @@ func (api DrivesDriveItemApi) DeleteDriveItem(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	render.Status(r, http.StatusOK)
+	render.Status(r, http.StatusNoContent)
+	render.NoContent(w, r)
 }
 
 // CreateDriveItem creates a drive item

services/graph/pkg/service/v0/api_drives_drive_item_test.go

@@ -392,6 +392,9 @@ var _ = Describe("DrivesDriveItemService", func() {
 					SpaceId:   "4",
 					OpaqueId:  "5",
 				}
+				expectedShareID := collaborationv1beta1.ShareId{
+					OpaqueId: "3:4:5",
+				}
 				gatewayClient.
 					On("GetReceivedShare", mock.Anything, mock.Anything, mock.Anything).
 					Return(func(ctx context.Context, in *collaborationv1beta1.GetReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.GetReceivedShareResponse, error) {
@@ -435,7 +438,33 @@ var _ = Describe("DrivesDriveItemService", func() {
 						Expect(resourceIDs).To(HaveLen(1))
 						Expect(resourceIDs[0]).To(Equal(&expectedResourceID))
 
-						return nil, nil
+						return &collaborationv1beta1.ListReceivedSharesResponse{
+							Shares: []*collaborationv1beta1.ReceivedShare{
+								{
+									State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED,
+									Share: &collaborationv1beta1.Share{
+										Id: &expectedShareID,
+									},
+								},
+							},
+						}, nil
+					})
+				gatewayClient.
+					On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything).
+					Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) {
+						Expect(in.GetUpdateMask().GetPaths()).To(Equal([]string{"state"}))
+						Expect(in.GetShare().GetState()).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_REJECTED))
+						Expect(in.GetShare().GetShare().GetId().GetOpaqueId()).To(Equal(expectedShareID.GetOpaqueId()))
+						return &collaborationv1beta1.UpdateReceivedShareResponse{
+							Status: status.NewOK(ctx),
+							Share: &collaborationv1beta1.ReceivedShare{
+								State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED,
+								Share: &collaborationv1beta1.Share{
+									Id:         &expectedShareID,
+									ResourceId: &expectedResourceID,
+								},
+							},
+						}, nil
 					})
 
 				err := drivesDriveItemService.UnmountShare(context.Background(), driveItemResourceID)
@@ -644,7 +673,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
 
 			httpAPI.DeleteDriveItem(responseRecorder, request)
 
-			Expect(responseRecorder.Code).To(Equal(http.StatusOK))
+			Expect(responseRecorder.Code).To(Equal(http.StatusNoContent))
 		})
 	})
 

tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature

@@ -21,8 +21,9 @@ Feature:  enable or disable sync of incoming shares
       | shareType       | user       |
       | permissionsRole | Viewer     |
     When user "Brian" disables sync of share "<resource>" using the Graph API
-    And user "Brian" lists the shares shared with him using the Graph API
-    Then the HTTP status code of responses on all endpoints should be "200"
+    Then the HTTP status code should be "204"
+    When user "Brian" lists the shares shared with him using the Graph API
+    Then the HTTP status code should be "200"
     And the JSON data of the response should match
     """
     {
@@ -141,7 +142,7 @@ Feature:  enable or disable sync of incoming shares
       | shareType       | group      |
       | permissionsRole | Viewer     |
     When user "Alice" disables sync of share "<resource>" using the Graph API
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "204"
     And user "Alice" should have sync disabled for share "<resource>"
     And user "Brian" should have sync enabled for share "<resource>"
     Examples:
@@ -196,8 +197,9 @@ Feature:  enable or disable sync of incoming shares
       | shareType       | user       |
       | permissionsRole | Viewer     |
     When user "Brian" disables sync of share "<resource>" using the Graph API
-    And user "Brian" lists the shares shared with him using the Graph API
-    Then the HTTP status code of responses on all endpoints should be "200"
+    Then the HTTP status code should be "204"
+    When user "Brian" lists the shares shared with him using the Graph API
+    Then the HTTP status code should be "200"
     And the JSON data of the response should match
     """
     {
@@ -288,7 +290,7 @@ Feature:  enable or disable sync of incoming shares
       | shareType       | group      |
       | permissionsRole | Viewer     |
     When user "Alice" disables sync of share "<resource>" using the Graph API
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "204"
     And user "Alice" should have sync disabled for share "<resource>"
     And user "Brian" should have sync enabled for share "<resource>"
     Examples: