feat(#33): cover and fix group delete

api/openapi-spec/openapi.yaml

@@ -425,12 +425,6 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Error'
-        '409':
-          description: Group still contains members other than the owner. Must remove them first to ensure proper notifications.
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/Error'
         '500':
           description: An unexpected error occurred. This includes database errors. A best effort attempt is made to return details in the body.
           content:

internal/application/web/endpoint.go

@@ -36,24 +36,24 @@ func CreateHandler[Req, Res any](endpoint Endpoint[Req, Res],
 		defer func() {
 			err := r.Body.Close()
 			if err != nil {
-				aulogging.ErrorErrf(ctx, err, "Error when closing the request body. [error]: %v", err)
+				aulogging.InfoErrf(ctx, err, "Error when closing the request body. [error]: %v", err)
 			}
 		}()
 
 		request, err := requestHandler(r, w)
 		if err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred while parsing the request. [error]: %v", err)
+			aulogging.InfoErrf(ctx, err, "An error occurred while parsing the request. [error]: %v", err)
 			return
 		}
 
 		response, err := endpoint(ctx, request, w)
 		if err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred during the request. [error]: %v", err)
+			aulogging.InfoErrf(ctx, err, "An error occurred during the request. [error]: %v", err)
 			return
 		}
 
 		if err := responseHandler(ctx, response, w); err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred during the handling of the response. [error]: %v", err)
+			aulogging.InfoErrf(ctx, err, "An error occurred during the handling of the response. [error]: %v", err)
 		}
 	})
 }

internal/controller/v1/groupsctl/groups_delete.go

@@ -21,6 +21,9 @@ type DeleteGroupRequest struct {
 // Only Admins or the current group owner can do this.
 func (h *Controller) DeleteGroup(ctx context.Context, req *DeleteGroupRequest, w http.ResponseWriter) (*modelsv1.Empty, error) {
 	err := h.svc.DeleteGroup(ctx, req.groupID)
+	if err != nil {
+		web.SendErrorResponse(ctx, w, err)
+	}
 	return nil, err
 }
 

internal/controller/v1/groupsctl/groups_put.go

@@ -2,12 +2,10 @@ package groupsctl
 
 import (
 	"context"
-	"fmt"
 	aulogging "github.com/StephanHCB/go-autumn-logging"
 	"github.com/eurofurence/reg-room-service/internal/application/web"
 	"github.com/eurofurence/reg-room-service/internal/controller/v1/util"
 	"github.com/go-chi/chi/v5"
-	"github.com/google/uuid"
 	"net/http"
 	"net/url"
 
@@ -43,8 +41,7 @@ func (h *Controller) UpdateGroupRequest(r *http.Request, w http.ResponseWriter)
 	ctx := r.Context()
 
 	groupID := chi.URLParam(r, "uuid")
-	if err := uuid.Validate(groupID); err != nil {
-		web.SendErrorResponse(ctx, w, common.NewBadRequest(ctx, common.GroupIDInvalid, common.Details(fmt.Sprintf("%q is not a vailid UUID", groupID))))
+	if err := validateGroupID(ctx, w, groupID); err != nil {
 		return nil, err
 	}
 

internal/controller/v1/groupsctl/utils.go

@@ -7,12 +7,13 @@ import (
 	"github.com/eurofurence/reg-room-service/internal/application/web"
 	"github.com/google/uuid"
 	"net/http"
+	"net/url"
 )
 
 func validateGroupID(ctx context.Context, w http.ResponseWriter, groupID string) error {
 	if err := uuid.Validate(groupID); err != nil {
 		web.SendErrorResponse(ctx, w,
-			common.NewBadRequest(ctx, common.GroupIDInvalid, common.Details(fmt.Sprintf("%q is not a vailid UUID", groupID))),
+			common.NewBadRequest(ctx, common.GroupIDInvalid, common.Details(fmt.Sprintf("'%s' is not a valid UUID", url.PathEscape(groupID)))),
 		)
 
 		return err

internal/repository/database/inmemorydb/implementation.go

@@ -119,11 +119,8 @@ func (r *InMemoryRepository) GetGroupByID(_ context.Context, id string) (*entity
 }
 
 func (r *InMemoryRepository) DeleteGroupByID(_ context.Context, id string) error {
-	if result, ok := r.groups[id]; ok {
-		result.Group.DeletedAt = gorm.DeletedAt{
-			Time:  r.Now(),
-			Valid: true,
-		}
+	if _, ok := r.groups[id]; ok {
+		delete(r.groups, id)
 		return nil
 	} else {
 		return gorm.ErrRecordNotFound
@@ -259,11 +256,8 @@ func (r *InMemoryRepository) GetRoomByID(ctx context.Context, id string) (*entit
 }
 
 func (r *InMemoryRepository) DeleteRoomByID(ctx context.Context, id string) error {
-	if result, ok := r.rooms[id]; ok {
-		result.Room.DeletedAt = gorm.DeletedAt{
-			Time:  r.Now(),
-			Valid: true,
-		}
+	if _, ok := r.rooms[id]; ok {
+		delete(r.rooms, id)
 		return nil
 	} else {
 		return gorm.ErrRecordNotFound

internal/service/groups/groups.go

@@ -388,12 +388,6 @@ func (g *groupService) DeleteGroup(ctx context.Context, groupID string) error {
 		return errGroupRead(ctx, "error retrieving group - see logs for details")
 	}
 
-	if group.DeletedAt.Valid {
-		// group is already deleted
-		aulogging.Warnf(ctx, "group %s was already marked for deletion", groupID)
-		return nil
-	}
-
 	if validator.IsAdmin() || validator.IsAPITokenCall() {
 		// admins and api token are allowed to make changes to any group
 	} else if validator.IsUser() {
@@ -481,7 +475,7 @@ func errNoGroup(ctx context.Context) error {
 }
 
 func errGroupIDNotFound(ctx context.Context) error {
-	return common.NewNotFound(ctx, common.GroupIDNotFound, common.Details("unable to find group in database"))
+	return common.NewNotFound(ctx, common.GroupIDNotFound, common.Details("group does not exist"))
 }
 
 func errGroupHasNoMembers(ctx context.Context) error {

test/acceptance/groups_delete_test.go

@@ -0,0 +1,128 @@
+package acceptance
+
+import (
+	"github.com/eurofurence/reg-room-service/docs"
+	"github.com/stretchr/testify/require"
+	"net/http"
+	"path"
+	"testing"
+)
+
+func TestGroupsDelete_AdminSuccess(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an attendee with an active registration who is in a group with two members")
+	id1 := setupExistingGroup(t, "kittens", true, "101", "202")
+
+	docs.Given("Given an authorized admin (a different user)")
+	token := tstValidAdminToken(t)
+
+	docs.When("When they delete the group")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", id1), token)
+
+	docs.Then("Then the group is successfully deleted and all its members are now no longer in a group")
+	require.Equal(t, http.StatusNoContent, response.status, "unexpected http response status")
+
+	user101group := tstPerformGet("/api/rest/v1/groups/my", tstValidUserToken(t, 101))
+	require.Equal(t, http.StatusNotFound, user101group.status, "unexpected http response status")
+	user202group := tstPerformGet("/api/rest/v1/groups/my", tstValidUserToken(t, 202))
+	require.Equal(t, http.StatusNotFound, user202group.status, "unexpected http response status")
+}
+
+func TestGroupsDelete_AnonymousDeny(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an attendee with an active registration who is in a group")
+	id1 := setupExistingGroup(t, "kittens", true, "101")
+
+	docs.Given("Given an unauthenticated user")
+	token := tstNoToken()
+
+	docs.When("When they try to delete the group")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", id1), token)
+
+	docs.Then("Then the request is denied with the appropriate error")
+	tstRequireErrorResponse(t, response, http.StatusUnauthorized, "auth.unauthorized", "you must be logged in for this operation")
+}
+
+func TestGroupsDelete_UserNotOwnerDeny(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an attendee with an active registration who is in a group, but NOT its owner")
+	id1 := setupExistingGroup(t, "kittens", false, "101", "202")
+	token := tstValidUserToken(t, 202)
+
+	docs.When("When they attempt to delete the group")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", id1), token)
+
+	docs.Then("Then the request fails with the appropriate error message")
+	tstRequireErrorResponse(t, response, http.StatusForbidden, "auth.forbidden", "only the group owner or an admin can delete a group")
+}
+
+func TestGroupsDelete_UserNotMemberDeny(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an attendee with an active registration who is in a group")
+	id1 := setupExistingGroup(t, "kittens", false, "101")
+	_ = setupExistingGroup(t, "kittens", false, "202")
+	token := tstValidUserToken(t, 202)
+
+	docs.When("When they attempt to delete a different group they are not a member of")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", id1), token)
+
+	docs.Then("Then the request fails with the appropriate error message")
+	tstRequireErrorResponse(t, response, http.StatusForbidden, "auth.forbidden", "only the group owner or an admin can delete a group")
+}
+
+func TestGroupsDelete_UserOwnerSuccess(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an authorized user with an active registration who is the owner of a group")
+	id1 := setupExistingGroup(t, "kittens", true, "101")
+	token := tstValidUserToken(t, 101)
+
+	docs.When("When they delete the group")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", id1), token)
+
+	docs.Then("Then the request is successful and the group has been deleted")
+	require.Equal(t, http.StatusNoContent, response.status, "unexpected http response status")
+
+	deletedResponse := tstPerformGet(path.Join("/api/rest/v1/groups/", id1), tstValidAdminToken(t))
+	require.Equal(t, http.StatusNotFound, deletedResponse.status, "group was not correctly deleted")
+}
+
+func TestGroupsDelete_InvalidID(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an authorized user with an active registration")
+	_ = setupExistingGroup(t, "kittens", true, "101")
+	token := tstValidUserToken(t, 101)
+
+	docs.When("When they try to delete a group, but specify an invalid id")
+	response := tstPerformDelete("/api/rest/v1/groups/kittycats", token)
+
+	docs.Then("Then the request fails with the expected error")
+	tstRequireErrorResponse(t, response, http.StatusBadRequest, "group.id.invalid", "'kittycats' is not a valid UUID")
+}
+
+func TestGroupsDelete_NotFound(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an authorized user with an active registration")
+	registerSubject("101")
+	token := tstValidUserToken(t, 101)
+
+	wrongId := "7ec0c20c-7dd4-491c-9b52-025be6950cdd"
+	docs.When("When they try to delete a group, but specify a valid id for a group that does not exist")
+	response := tstPerformDelete(path.Join("/api/rest/v1/groups/", wrongId), token)
+
+	docs.Then("Then the request fails with the expected error")
+	tstRequireErrorResponse(t, response, http.StatusNotFound, "group.id.notfound", "group does not exist")
+}

test/acceptance/groups_update_test.go

@@ -140,3 +140,39 @@ func TestGroupsUpdate_InvalidData(t *testing.T) {
 	docs.Then("Then the request fails with the expected error")
 	tstRequireErrorResponse(t, response, http.StatusBadRequest, "group.data.invalid", url.Values{"name": []string{"group name cannot be empty"}, "flags": []string{"no such flag 'invalid'"}})
 }
+
+func TestGroupsUpdate_InvalidID(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an authorized user with an active registration")
+	id1 := setupExistingGroup(t, "kittens", true, "101")
+	token := tstValidUserToken(t, 101)
+
+	docs.When("When they try to update the group but supply invalid information")
+	getGroup := tstReadGroup(t, path.Join("/api/rest/v1/groups/", id1))
+	response := tstPerformPut("/api/rest/v1/groups/kittycats", tstRenderJson(getGroup), token)
+
+	docs.Then("Then the request fails with the expected error")
+	tstRequireErrorResponse(t, response, http.StatusBadRequest, "group.id.invalid", "'kittycats' is not a valid UUID")
+}
+
+func TestGroupsUpdate_NotFound(t *testing.T) {
+	tstSetup(tstDefaultConfigFileRoomGroups)
+	defer tstShutdown()
+
+	docs.Given("Given an authorized user with an active registration")
+	id1 := setupExistingGroup(t, "kittens", true, "101")
+	token := tstValidUserToken(t, 101)
+
+	wrongId := "7ec0c20c-7dd4-491c-9b52-025be6950cdd"
+	if wrongId == id1 {
+		wrongId = "7ec0c20c-7dd4-491c-9b52-025be6950cef"
+	}
+	docs.When("When they try to update a group, but specify a valid id for a group that does not exist")
+	getGroup := tstReadGroup(t, path.Join("/api/rest/v1/groups/", id1))
+	response := tstPerformPut(path.Join("/api/rest/v1/groups/", wrongId), tstRenderJson(getGroup), token)
+
+	docs.Then("Then the request fails with the expected error")
+	tstRequireErrorResponse(t, response, http.StatusNotFound, "group.id.notfound", "group does not exist")
+}