From cd539311a5b82de4d61753e14da411e65938bcf1 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Wed, 10 Apr 2024 16:57:14 +0200 Subject: [PATCH] fix unmount item from share --- .drone.env | 4 +-- .../unreleased/fix-graph-unmount-item.md | 6 ++++ .../pkg/service/v0/api_drives_drive_item.go | 6 +++- .../service/v0/api_drives_drive_item_test.go | 33 +++++++++++++++++-- .../enableDisableShareSync.feature | 14 ++++---- 5 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/fix-graph-unmount-item.md diff --git a/.drone.env b/.drone.env index 11aabaf5ef..050d7fdfa2 100644 --- a/.drone.env +++ b/.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 diff --git a/changelog/unreleased/fix-graph-unmount-item.md b/changelog/unreleased/fix-graph-unmount-item.md new file mode 100644 index 0000000000..11f8cef7ce --- /dev/null +++ b/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 diff --git a/services/graph/pkg/service/v0/api_drives_drive_item.go b/services/graph/pkg/service/v0/api_drives_drive_item.go index 882132fa0d..c1167bfc5e 100644 --- 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 index 6c7f4684be..2a932ff67b 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item_test.go +++ b/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)) }) }) diff --git a/tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature b/tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature index fe2badcc1a..1e9f7a3596 100644 --- 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 "" 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 "" 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 "" And user "Brian" should have sync enabled for share "" Examples: @@ -196,8 +197,9 @@ Feature: enable or disable sync of incoming shares | shareType | user | | permissionsRole | Viewer | When user "Brian" disables sync of share "" 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 "" 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 "" And user "Brian" should have sync enabled for share "" Examples: