fix unmount item from share

owncloud · Apr 12, 2024 · 45ec0b8 · 45ec0b8
1 parent 01c2d98
commit 45ec0b8
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 8 deletions.
diff --git a/changelog/unreleased/fix-graph-unmount-item.md b/changelog/unreleased/fix-graph-unmount-item.md
@@ -0,0 +1,6 @@
+Bugfix: Fix unmount item from share
+
+We fixed the issue when unmount item from share will result in 200 status code
+
+https://github.com/owncloud/ocis/pull/8827
+https://github.com/owncloud/ocis/issues/8731
diff --git a/services/graph/pkg/service/v0/api_drives_drive_item.go b/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

diff --git a/services/graph/pkg/service/v0/api_drives_drive_item_test.go b/services/graph/pkg/service/v0/api_drives_drive_item_test.go
@@ -391,6 +391,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) {
@@ -434,7 +437,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)
@@ -643,7 +672,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
 
 			httpAPI.DeleteDriveItem(responseRecorder, request)
 
-			Expect(responseRecorder.Code).To(Equal(http.StatusOK))
+			Expect(responseRecorder.Code).To(Equal(http.StatusNoContent))
 		})
 	})
 

diff --git a/tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature b/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
     """
     {