Skip to content
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.

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

fix unmount item from share #8827

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .drone.env
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# The test runner source for UI tests
WEB_COMMITID=5367936f30b07c0832fb8af09f9f7719cccf7038
WEB_BRANCH=master
WEB_COMMITID=9b9616b8429432002d06c4a2c8f252cbe08fb735
WEB_BRANCH=changeResponseCodeDelete
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this really be part of the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. This seems to be needed to resolve a circular dependency with web. Needs to be revert once the related web changes are merged.

6 changes: 6 additions & 0 deletions changelog/unreleased/fix-graph-unmount-item.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Fix unmount item from share

We fixed the status code returned for the request to delete a driveitem.

Copy link
Contributor

Choose a reason for hiding this comment

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

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
6 changes: 5 additions & 1 deletion services/graph/pkg/service/v0/api_drives_drive_item.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
33 changes: 31 additions & 2 deletions services/graph/pkg/service/v0/api_drives_drive_item_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
{
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
"""
{
Expand Down Expand Up @@ -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:
Expand Down