fix: itemid is same as path paramater

Author: kavos113

handler/ownership.go

@@ -60,7 +60,7 @@ func (h *handler) EditItemOwners(ctx echo.Context, itemId openapi.ItemIdInPath,
 		Memo:     req.Memo,
 	}
 
-	updated, err := h.ou.UpdateOwnership(ownership, itemId, userID)
+	updated, err := h.ou.UpdateOwnership(ownership, userID)
 	if err != nil {
 		if errors.Is(err, domain.ErrNotFound) {
 			return ctx.NoContent(http.StatusNotFound)

usecase/mock/mock_ownership_usecase.go

@@ -9,7 +9,7 @@ import (
 type OwnershipUseCase interface {
 	GetByItemID(itemID int) ([]*domain.Ownership, error)
 	CreateOwnership(ownership *domain.Ownership) (*domain.Ownership, error)
-	UpdateOwnership(ownership *domain.Ownership, itemID int, userID string) (*domain.Ownership, error)
+	UpdateOwnership(ownership *domain.Ownership, userID string) (*domain.Ownership, error)
 	DeleteOwnership(id int, itemID int, userID string) error
 }
 
@@ -32,16 +32,20 @@ func (u *ownershipUseCase) CreateOwnership(ownership *domain.Ownership) (*domain
 }
 
 // TODO: 管理者も変えられるように
-func (u *ownershipUseCase) UpdateOwnership(ownership *domain.Ownership, itemID int, userID string) (*domain.Ownership, error) {
+func (u *ownershipUseCase) UpdateOwnership(ownership *domain.Ownership, userID string) (*domain.Ownership, error) {
 	o, err := u.ownershipRepo.GetByID(ownership.ID)
 	if err != nil {
 		return nil, err
 	}
 
-	if o.ItemID != itemID {
+	if o.ItemID != ownership.ItemID {
 		return nil, fmt.Errorf("%w: ownership does not belong to the specified item", domain.ErrNotFound)
 	}
 
+	if o.UserID != ownership.UserID {
+		return nil, fmt.Errorf("%w: you can only change the ownership of your own item", ErrForbidden)
+	}
+
 	if o.UserID != userID {
 		return nil, fmt.Errorf("%w: you can only update your own ownership", ErrForbidden)
 	}

usecase/ownership.go

@@ -114,7 +114,6 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 	testCases := []struct {
 		name              string
 		inputOwnership    *domain.Ownership
-		itemID            int
 		userID            string
 		setupMock         func(repo *mock_domain.MockOwnershipRepository)
 		expectedOwnership *domain.Ownership
@@ -129,7 +128,6 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 				Rentable: false,
 				Memo:     "updated memo",
 			},
-			itemID: 1,
 			userID: "owner",
 			setupMock: func(repo *mock_domain.MockOwnershipRepository) {
 				repo.EXPECT().
@@ -153,7 +151,6 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 				Rentable: true,
 				Memo:     "memo",
 			},
-			itemID: 1,
 			userID: "owner",
 			setupMock: func(repo *mock_domain.MockOwnershipRepository) {
 				repo.EXPECT().
@@ -164,15 +161,32 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 			expectedErr: domain.ErrNotFound,
 		},
 		{
-			name: "failure: forbidden",
+			name: "failure: cannot change other user's ownership",
+			inputOwnership: &domain.Ownership{
+				ID:       1,
+				ItemID:   1,
+				UserID:   "owner",
+				Rentable: false,
+				Memo:     "updated memo",
+			},
+			userID: "another-user",
+			setupMock: func(repo *mock_domain.MockOwnershipRepository) {
+				repo.EXPECT().
+					GetByID(1).
+					Return(&domain.Ownership{ID: 1, ItemID: 1, UserID: "owner", Rentable: true, Memo: "before"}, nil).
+					Times(1)
+			},
+			expectedErr: ErrForbidden,
+		},
+		{
+			name: "failure: cannot change other user's ownership, although ownership.userID is matched",
 			inputOwnership: &domain.Ownership{
 				ID:       1,
 				ItemID:   1,
 				UserID:   "owner",
 				Rentable: false,
 				Memo:     "updated memo",
 			},
-			itemID: 1,
 			userID: "another-user",
 			setupMock: func(repo *mock_domain.MockOwnershipRepository) {
 				repo.EXPECT().
@@ -191,7 +205,6 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 				Rentable: false,
 				Memo:     "updated memo",
 			},
-			itemID: 1,
 			userID: "owner",
 			setupMock: func(repo *mock_domain.MockOwnershipRepository) {
 				repo.EXPECT().
@@ -213,7 +226,7 @@ func TestOwnershipUseCase_UpdateOwnership(t *testing.T) {
 
 			ownershipUseCase := NewOwnershipUseCase(mockOwnershipRepo)
 
-			ownership, err := ownershipUseCase.UpdateOwnership(tc.inputOwnership, tc.itemID, tc.userID)
+			ownership, err := ownershipUseCase.UpdateOwnership(tc.inputOwnership, tc.userID)
 
 			assert.Equal(t, tc.expectedOwnership, ownership)
 			assert.True(t, errors.Is(err, tc.expectedErr))