Merge pull request 'Re-order lists' (#49) from dev into main

.gitea/workflows/pr-checks.yml

@@ -10,6 +10,9 @@ jobs:
     runs-on: ubuntu-latest
     container:
       image: golang:1.27-bookworm
+      volumes:
+        - go-mod-cache:/go/pkg/mod
+        - go-build-cache:/root/.cache/go-build
     steps:
       - name: Install system dependencies
         run: |
@@ -20,9 +23,6 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Lint
-        run: make lint
-
       - name: Check formatting
         run: |
           unformatted=$(gofmt -l .)
@@ -32,6 +32,12 @@ jobs:
             exit 1
           fi
 
+      - name: Download modules
+        run: go mod download
+
+      - name: Lint
+        run: make lint
+
       - name: Test
         run: make test
 

internal/api/handler/list.go

@@ -1,6 +1,7 @@
 package handler
 
 import (
+	"errors"
 	"log/slog"
 	"net/http"
 
@@ -232,6 +233,52 @@ func (h *ListHandler) RemoveUserFromList(w http.ResponseWriter, r *http.Request)
 	response.Status(w, http.StatusNoContent)
 }
 
+// OrderLists handles re-ordering a users lists
+//
+// @Summary Order lists of a user
+// @Description Re-assigns the display order of all users lists
+// @Tags List
+// @Accept json
+// @Produce json
+// @Param payload body request.OrderListsPayload true "Order lists payload"
+// @Success 204 {object} nil
+// @Error 400 {object} response.ErrorResponse "failed to decode request body"
+// @Error 401 {object} response.ErrorResponse "not authorized"
+// @Error 500 {object} response.ErrorResponse "failed to order lists"
+// @Router /lists/order [post]
+func (h *ListHandler) OrderLists(w http.ResponseWriter, r *http.Request) {
+	ctx := r.Context()
+
+	payload, err := request.DecodeJSON[request.OrderListsPayload](r)
+	if err != nil {
+		slog.ErrorContext(ctx, "failed to decode order lists payload", slog.Any("error", err))
+		response.Error(ctx, w, http.StatusBadRequest, "failed to decode request body")
+		return
+	}
+
+	authContext, err := domain.GetAuthContext(ctx)
+	if err != nil {
+		slog.ErrorContext(ctx, "failed to get auth context", slog.Any("error", err))
+		response.Error(ctx, w, http.StatusUnauthorized, "not authorized")
+		return
+	}
+
+	err = h.ListService.OrderLists(ctx, authContext.UserID, payload.ListIDs)
+	if err != nil {
+		if errors.Is(err, domain.ErrStaleListIDs) {
+			response.Error(ctx, w, http.StatusBadRequest, "stale list IDs")
+		} else {
+			response.Error(ctx, w, http.StatusInternalServerError, "failed to order list items")
+		}
+
+		slog.ErrorContext(ctx, "failed to order lists", slog.Any("error", err))
+		return
+	}
+
+	slog.InfoContext(ctx, "lists re-ordered successfully", slog.String("user_id", authContext.UserID))
+	response.Status(w, http.StatusNoContent)
+}
+
 // CreateListItem handles creation of a new list item on a given list
 //
 // @Summary Create list item

internal/api/request/list.go

@@ -14,6 +14,10 @@ type RemoveUserFromListPayload struct {
 	Email  string `json:"email"`
 }
 
+type OrderListsPayload struct {
+	ListIDs []string `json:"list_ids"`
+}
+
 type CreateListItemPayload struct {
 	ListID string `json:"list_id"`
 	Title  string `json:"title"`

internal/api/router/router.go

@@ -65,6 +65,7 @@ func NewMux(cfg Config) http.Handler {
 	apiMux.Handle("GET /lists", protected(listHandler.GetLists))
 	apiMux.Handle("POST /lists/user", protected(listHandler.AddUserToList))
 	apiMux.Handle("DELETE /lists/user", protected(listHandler.RemoveUserFromList))
+	apiMux.Handle("POST /lists/order", protected(listHandler.OrderLists))
 	apiMux.Handle("POST /lists/items", protected(listHandler.CreateListItem))
 	apiMux.Handle("DELETE /lists/items/{id}", protected(listHandler.DeleteListItem))
 	apiMux.Handle("PUT /lists/items", protected(listHandler.UpdateListItem))

internal/database/migrations/000006_add_list_position.down.sql

@@ -0,0 +1,8 @@
+BEGIN;
+
+DROP INDEX IF EXISTS idx_list_users_position;
+
+ALTER TABLE IF EXISTS list_users
+    DROP COLUMN IF EXISTS position;
+
+COMMIT;

internal/database/migrations/000006_add_list_position.up.sql

@@ -0,0 +1,20 @@
+BEGIN;
+
+ALTER TABLE IF EXISTS list_users
+    ADD COLUMN IF NOT EXISTS position NOT NULL DEFAULT 0;
+
+CREATE INDEX IF NOT EXISTS idx_list_users_user_id_position ON list_users (user_id, position);
+
+UPDATE list_users lu
+    SET position = r.rn - 1
+FROM (SELECT list_id,
+             user_id,
+             row_number() OVER (
+                 PARTITION BY user_id
+                 ORDER BY created_at, list_id
+                 ) AS rn
+      FROM list_users) r
+WHERE lu.list_id = r.list_id
+  AND lu.user_id = r.user_id;
+
+COMMIT;

internal/domain/list.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"log/slog"
+	"strings"
 	"time"
 )
 
@@ -13,6 +14,7 @@ var (
 	ErrListItemIDMissing    = errors.New("list item id is required")
 	ErrListItemTitleMissing = errors.New("list item title is required")
 	ErrUserNotInList        = errors.New("user not in list")
+	ErrStaleListIDs         = errors.New("list ids out of date")
 )
 
 type List struct {
@@ -22,6 +24,7 @@ type List struct {
 	ModifiedAt     time.Time `json:"modified_at"`
 	TotalItems     int       `json:"total_items"`
 	CompletedItems int       `json:"completed_items"`
+	Position       int       `json:"position"`
 }
 
 type ListItem struct {
@@ -39,6 +42,8 @@ type ListRepository interface {
 	GetLists(ctx context.Context, userID string) ([]List, error)
 	AddUserToList(ctx context.Context, listID string, userID string) error
 	RemoveUserFromList(ctx context.Context, listID string, userID string) error
+	OrderLists(ctx context.Context, userID string, listIDs []string) error
+	LockUsersLists(ctx context.Context, userID string) ([]string, error)
 	IsUserInList(ctx context.Context, listID string, userID string) (bool, error)
 	IsUserInListByItemID(ctx context.Context, listItemID string, userID string) (bool, error)
 	CreateListItem(ctx context.Context, listID string, title string) (*ListItem, error)
@@ -131,6 +136,36 @@ func (s *ListService) RemoveUserFromList(ctx context.Context, authUserID string,
 	return s.repo.RemoveUserFromList(ctx, listID, userID)
 }
 
+func (s *ListService) OrderLists(ctx context.Context, authUserID string, listIDs []string) error {
+	if authUserID == "" {
+		return ErrUserIDMissing
+	}
+	if len(listIDs) == 0 {
+		return ErrListIDMissing
+	}
+
+	return s.tx.WithinTx(ctx, func(ctx context.Context) error {
+		serverIDs, err := s.repo.LockUsersLists(ctx, authUserID)
+		if err != nil {
+			return err
+		}
+
+		if !isPermutation(listIDs, serverIDs) {
+			slog.ErrorContext(ctx, "no permutation",
+				slog.Any("client_list_ids", listIDs),
+				slog.Any("server_list_ids", serverIDs))
+			return ErrStaleListIDs
+		}
+
+		err = s.repo.OrderLists(ctx, authUserID, listIDs)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+}
+
 func (s *ListService) CreateListItem(ctx context.Context, authUserID string, listID string, title string) (*ListItem, error) {
 	if listID == "" {
 		return nil, ErrListIDMissing
@@ -257,3 +292,28 @@ func (s *ListService) userAllowedToAccessListItem(ctx context.Context, authUserI
 
 	return nil
 }
+
+func isPermutation(a []string, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	aMap := make(map[string]struct{}, len(a))
+	for _, v := range a {
+		aMap[strings.ToLower(v)] = struct{}{}
+	}
+
+	seen := make(map[string]struct{}, len(a))
+	for _, v := range b {
+		id := strings.ToLower(v)
+		if _, ok := aMap[id]; !ok {
+			return false
+		}
+		if _, dup := seen[id]; dup {
+			return false
+		}
+		seen[id] = struct{}{}
+	}
+
+	return true
+}

internal/domain/list_test.go

@@ -11,6 +11,12 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var (
+	_ Transactor     = (*fakeTransactor)(nil)
+	_ ListRepository = (*mockListRepository)(nil)
+)
+
+type txCtxKey struct{}
 type fakeTransactor struct {
 	calls int
 	err   error
@@ -21,9 +27,14 @@ func (f *fakeTransactor) WithinTx(ctx context.Context, fn func(ctx context.Conte
 	if f.err != nil {
 		return f.err
 	}
-	return fn(ctx)
+	return fn(context.WithValue(ctx, txCtxKey{}, true))
 }
 
+var inTx = mock.MatchedBy(func(ctx context.Context) bool {
+	v, _ := ctx.Value(txCtxKey{}).(bool)
+	return v
+})
+
 type mockListRepository struct {
 	mock.Mock
 }
@@ -55,6 +66,17 @@ func (m *mockListRepository) RemoveUserFromList(ctx context.Context, listID stri
 	return args.Error(0)
 }
 
+func (m *mockListRepository) OrderLists(ctx context.Context, userID string, listIDs []string) error {
+	args := m.Called(ctx, userID, listIDs)
+	return args.Error(0)
+}
+
+func (m *mockListRepository) LockUsersLists(ctx context.Context, userID string) ([]string, error) {
+	args := m.Called(ctx, userID)
+	ids, _ := args.Get(0).([]string)
+	return ids, args.Error(1)
+}
+
 func (m *mockListRepository) IsUserInList(ctx context.Context, listID string, userID string) (bool, error) {
 	args := m.Called(ctx, listID, userID)
 	return args.Bool(0), args.Error(1)
@@ -101,6 +123,7 @@ func newListService(t *testing.T) (*ListService, *mockListRepository, *fakeTrans
 	t.Helper()
 
 	repo := &mockListRepository{}
+	repo.Test(t)
 	t.Cleanup(func() { repo.AssertExpectations(t) })
 
 	tx := &fakeTransactor{}
@@ -108,108 +131,268 @@ func newListService(t *testing.T) (*ListService, *mockListRepository, *fakeTrans
 	return NewListService(tx, repo), repo, tx
 }
 
+func assertCallOrder(t *testing.T, repo *mockListRepository, want ...string) {
+	t.Helper()
+
+	var got []string
+	for _, c := range repo.Calls {
+		got = append(got, c.Method)
+	}
+	assert.Equal(t, want, got)
+}
+
 func TestListService_CreateList(t *testing.T) {
-	t.Run("creates the list and adds the owner", func(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("creates the list and adds the creator in one transaction", func(t *testing.T) {
 		svc, repo, tx := newListService(t)
 
 		created := &List{ID: "list-1", Name: "Groceries", CreatedAt: time.Now()}
-		repo.On("CreateList", mock.Anything, "Groceries").Return(created, nil)
+		repo.On("CreateList", inTx, "Groceries").Return(created, nil)
-		repo.On("AddUserToList", mock.Anything, "list-1", "user-1").Return(nil)
+		repo.On("AddUserToList", inTx, "list-1", "user-1").Return(nil)
 
-		list, err := svc.CreateList(context.Background(), "user-1", "Groceries")
+		list, err := svc.CreateList(ctx, "user-1", "Groceries")
 
 		require.NoError(t, err)
 		assert.Equal(t, created, list)
 		assert.Equal(t, 1, tx.calls)
-	})
+		assertCallOrder(t, repo, "CreateList", "AddUserToList")
-
-	t.Run("empty name is rejected before any repo call", func(t *testing.T) {
-		svc, _, tx := newListService(t)
-
-		list, err := svc.CreateList(context.Background(), "user-1", "")
-
-		assert.Nil(t, list)
-		assert.ErrorIs(t, err, ErrListNameMissing)
-		assert.Zero(t, tx.calls)
 	})
 
 	t.Run("insert error aborts before adding the user", func(t *testing.T) {
 		svc, repo, _ := newListService(t)
 
 		repoErr := errors.New("insert failed")
-		repo.On("CreateList", mock.Anything, "Groceries").Return(nil, repoErr)
+		repo.On("CreateList", inTx, "Groceries").Return(nil, repoErr)
 
-		list, err := svc.CreateList(context.Background(), "user-1", "Groceries")
+		list, err := svc.CreateList(ctx, "user-1", "Groceries")
 
 		assert.Nil(t, list)
 		assert.ErrorIs(t, err, repoErr)
-		repo.AssertNotCalled(t, "AddUserToList", mock.Anything, mock.Anything, mock.Anything)
+		assertCallOrder(t, repo, "CreateList")
 	})
 
-	t.Run("transaction error is returned", func(t *testing.T) {
+	t.Run("membership insert error fails the whole operation", func(t *testing.T) {
-		svc, _, tx := newListService(t)
+		svc, repo, _ := newListService(t)
+
+		repoErr := errors.New("foreign key violation")
+		repo.On("CreateList", inTx, "Groceries").Return(&List{ID: "list-1", Name: "Groceries"}, nil)
+		repo.On("AddUserToList", inTx, "list-1", "user-1").Return(repoErr)
+
+		list, err := svc.CreateList(ctx, "user-1", "Groceries")
+
+		assert.Nil(t, list, "no half-created list may be returned")
+		assert.ErrorIs(t, err, repoErr)
+	})
+
+	t.Run("transaction begin error is returned", func(t *testing.T) {
+		svc, repo, tx := newListService(t)
 
 		tx.err = errors.New("could not begin transaction")
 
-		list, err := svc.CreateList(context.Background(), "user-1", "Groceries")
+		list, err := svc.CreateList(ctx, "user-1", "Groceries")
 
 		assert.Nil(t, list)
 		assert.ErrorIs(t, err, tx.err)
+		assertCallOrder(t, repo)
 	})
 }
 
 func TestListService_DeleteList(t *testing.T) {
-	t.Run("deletes when the user is a member", func(t *testing.T) {
 	svc, repo, _ := newListService(t)
 
 	repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
 	repo.On("DeleteList", mock.Anything, "list-1").Return(nil)
 
-		assert.NoError(t, svc.DeleteList(context.Background(), "user-1", "list-1"))
-	})
-
-	t.Run("refuses when the user is not a member", func(t *testing.T) {
-		svc, repo, _ := newListService(t)
-
-		repo.On("IsUserInList", mock.Anything, "list-1", "intruder").Return(false, nil)
-
-		err := svc.DeleteList(context.Background(), "intruder", "list-1")
-
-		assert.ErrorIs(t, err, ErrUserNotInList)
-		repo.AssertNotCalled(t, "DeleteList", mock.Anything, mock.Anything)
-	})
-
-	t.Run("membership check error is propagated", func(t *testing.T) {
-		svc, repo, _ := newListService(t)
-
-		repoErr := errors.New("connection reset")
-		repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(false, repoErr)
-
 	err := svc.DeleteList(context.Background(), "user-1", "list-1")
 
-		assert.ErrorIs(t, err, repoErr)
+	require.NoError(t, err)
-		assert.NotErrorIs(t, err, ErrUserNotInList)
+	assertCallOrder(t, repo, "IsUserInList", "DeleteList")
-		repo.AssertNotCalled(t, "DeleteList", mock.Anything, mock.Anything)
-	})
 }
 
 func TestListService_GetLists(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("returns the user's lists", func(t *testing.T) {
 		svc, repo, _ := newListService(t)
 
 		want := []List{{ID: "list-1", Name: "Groceries"}, {ID: "list-2", Name: "Reading"}}
 		repo.On("GetLists", mock.Anything, "user-1").Return(want, nil)
 
-	lists, err := svc.GetLists(context.Background(), "user-1")
+		lists, err := svc.GetLists(ctx, "user-1")
 
 		require.NoError(t, err)
 		assert.Equal(t, want, lists)
+	})
+
+	t.Run("user without lists gets an empty result", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		repo.On("GetLists", mock.Anything, "user-1").Return([]List{}, nil)
+
+		lists, err := svc.GetLists(ctx, "user-1")
+
+		require.NoError(t, err)
+		assert.Empty(t, lists)
+	})
+
+	t.Run("repository error is propagated", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		repoErr := errors.New("connection reset")
+		repo.On("GetLists", mock.Anything, "user-1").Return(nil, repoErr)
+
+		lists, err := svc.GetLists(ctx, "user-1")
+
+		assert.Nil(t, lists)
+		assert.ErrorIs(t, err, repoErr)
+	})
+}
+
+func TestListService_AddUserToList(t *testing.T) {
+	svc, repo, _ := newListService(t)
+
+	repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
+	repo.On("AddUserToList", mock.Anything, "list-1", "user-2").Return(nil)
+
+	err := svc.AddUserToList(context.Background(), "user-1", "list-1", "user-2")
+
+	require.NoError(t, err)
+	assertCallOrder(t, repo, "IsUserInList", "AddUserToList")
+}
+
+func TestListService_RemoveUserFromList(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("member removes another member", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
+		repo.On("RemoveUserFromList", mock.Anything, "list-1", "user-2").Return(nil)
+
+		err := svc.RemoveUserFromList(ctx, "user-1", "list-1", "user-2")
+
+		require.NoError(t, err)
+		assertCallOrder(t, repo, "IsUserInList", "RemoveUserFromList")
+	})
+
+	t.Run("member leaves the list", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
+		repo.On("RemoveUserFromList", mock.Anything, "list-1", "user-1").Return(nil)
+
+		err := svc.RemoveUserFromList(ctx, "user-1", "list-1", "user-1")
+
+		require.NoError(t, err)
+	})
+}
+
+func TestListService_OrderLists(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("stores the client's order inside one transaction", func(t *testing.T) {
+		svc, repo, tx := newListService(t)
+
+		repo.On("LockUsersLists", inTx, "user-1").Return([]string{"list-a", "list-b", "list-c"}, nil)
+		repo.On("OrderLists", inTx, "user-1", []string{"list-c", "list-a", "list-b"}).Return(nil)
+
+		err := svc.OrderLists(ctx, "user-1", []string{"list-c", "list-a", "list-b"})
+
+		require.NoError(t, err)
+		assert.Equal(t, 1, tx.calls)
+		assertCallOrder(t, repo, "LockUsersLists", "OrderLists")
+	})
+
+	t.Run("ids are compared case-insensitively", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		server := []string{
+			"3f2a8c1e-7d4b-4e21-9a6f-2b1c0d9e8f7a",
+			"9b1d4e6f-0a2c-4b3d-8e5f-6a7b8c9d0e1f",
+		}
+		client := []string{
+			"9B1D4E6F-0A2C-4B3D-8E5F-6A7B8C9D0E1F",
+			"3F2A8C1E-7D4B-4E21-9A6F-2B1C0D9E8F7A",
+		}
+		repo.On("LockUsersLists", inTx, "user-1").Return(server, nil)
+		// The client's spelling is passed on unchanged; Postgres' uuid cast
+		// doesn't care about case.
+		repo.On("OrderLists", inTx, "user-1", client).Return(nil)
+
+		err := svc.OrderLists(ctx, "user-1", client)
+
+		require.NoError(t, err)
+	})
+
+	t.Run("stale ids are rejected without writing", func(t *testing.T) {
+		tests := []struct {
+			name   string
+			server []string
+			client []string
+		}{
+			{name: "client is missing a list", server: []string{"a", "b", "c"}, client: []string{"a", "b"}},
+			{name: "client has an extra list", server: []string{"a", "b"}, client: []string{"a", "b", "c"}},
+			{name: "client has an unknown list", server: []string{"a", "b"}, client: []string{"a", "x"}},
+			{name: "client repeats a list", server: []string{"a", "b"}, client: []string{"a", "a"}},
+			{name: "user has no lists anymore", server: []string{}, client: []string{"a"}},
+		}
+
+		for _, tt := range tests {
+			t.Run(tt.name, func(t *testing.T) {
+				svc, repo, _ := newListService(t)
+
+				repo.On("LockUsersLists", inTx, "user-1").Return(tt.server, nil)
+
+				err := svc.OrderLists(ctx, "user-1", tt.client)
+
+				assert.ErrorIs(t, err, ErrStaleListIDs)
+				assertCallOrder(t, repo, "LockUsersLists")
+			})
+		}
+	})
+
+	t.Run("lock error is propagated", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		lockErr := errors.New("lock timeout")
+		repo.On("LockUsersLists", inTx, "user-1").Return(nil, lockErr)
+
+		err := svc.OrderLists(ctx, "user-1", []string{"list-a"})
+
+		assert.ErrorIs(t, err, lockErr)
+		assert.NotErrorIs(t, err, ErrStaleListIDs)
+		assertCallOrder(t, repo, "LockUsersLists")
+	})
+
+	t.Run("update error is propagated", func(t *testing.T) {
+		svc, repo, _ := newListService(t)
+
+		updateErr := errors.New("update failed")
+		repo.On("LockUsersLists", inTx, "user-1").Return([]string{"list-a"}, nil)
+		repo.On("OrderLists", inTx, "user-1", []string{"list-a"}).Return(updateErr)
+
+		err := svc.OrderLists(ctx, "user-1", []string{"list-a"})
+
+		assert.ErrorIs(t, err, updateErr)
+	})
+
+	t.Run("transaction begin error is returned", func(t *testing.T) {
+		svc, repo, tx := newListService(t)
+
+		tx.err = errors.New("could not begin transaction")
+
+		err := svc.OrderLists(ctx, "user-1", []string{"list-a"})
+
+		assert.ErrorIs(t, err, tx.err)
+		assertCallOrder(t, repo)
+	})
 }
 
 func TestListService_CreateListItem(t *testing.T) {
-	t.Run("creates the item for a member", func(t *testing.T) {
 	svc, repo, _ := newListService(t)
 
-		created := &ListItem{ID: "item-1", ListID: "list-1", Title: "Milk"}
+	created := &ListItem{ID: "item-1", ListID: "list-1", Title: "Milk", CreatedAt: time.Now()}
 	repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
 	repo.On("CreateListItem", mock.Anything, "list-1", "Milk").Return(created, nil)
 
@@ -217,115 +400,361 @@ func TestListService_CreateListItem(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Equal(t, created, item)
-	})
+	assertCallOrder(t, repo, "IsUserInList", "CreateListItem")
-
-	t.Run("refuses for a non-member", func(t *testing.T) {
-		svc, repo, _ := newListService(t)
-
-		repo.On("IsUserInList", mock.Anything, "list-1", "intruder").Return(false, nil)
-
-		item, err := svc.CreateListItem(context.Background(), "intruder", "list-1", "Milk")
-
-		assert.Nil(t, item)
-		assert.ErrorIs(t, err, ErrUserNotInList)
-		repo.AssertNotCalled(t, "CreateListItem", mock.Anything, mock.Anything, mock.Anything)
-	})
 }
 
-func TestListService_SetListItemCompleted(t *testing.T) {
+func TestListService_DeleteListItem(t *testing.T) {
-	t.Run("checks membership through the item id", func(t *testing.T) {
 	svc, repo, _ := newListService(t)
 
 	repo.On("IsUserInListByItemID", mock.Anything, "item-1", "user-1").Return(true, nil)
-		repo.On("SetListItemCompleted", mock.Anything, "item-1", true).Return(nil)
+	repo.On("DeleteListItem", mock.Anything, "item-1").Return(nil)
 
-		assert.NoError(t, svc.SetListItemCompleted(context.Background(), "user-1", "item-1", true))
+	err := svc.DeleteListItem(context.Background(), "user-1", "item-1")
-	})
 
-	t.Run("refuses for a non-member", func(t *testing.T) {
+	require.NoError(t, err)
+	assertCallOrder(t, repo, "IsUserInListByItemID", "DeleteListItem")
+}
+
+func TestListService_UpdateListItem(t *testing.T) {
 	svc, repo, _ := newListService(t)
 
-		repo.On("IsUserInListByItemID", mock.Anything, "item-1", "intruder").Return(false, nil)
+	repo.On("IsUserInListByItemID", mock.Anything, "item-1", "user-1").Return(true, nil)
+	repo.On("UpdateListItem", mock.Anything, "item-1", "Oat milk", true).Return(nil)
 
-		err := svc.SetListItemCompleted(context.Background(), "intruder", "item-1", false)
+	err := svc.UpdateListItem(context.Background(), "user-1", "item-1", "Oat milk", true)
+
+	require.NoError(t, err)
+	assertCallOrder(t, repo, "IsUserInListByItemID", "UpdateListItem")
+}
+
+func TestListService_SetListItemTitle(t *testing.T) {
+	svc, repo, _ := newListService(t)
+
+	repo.On("IsUserInListByItemID", mock.Anything, "item-1", "user-1").Return(true, nil)
+	repo.On("SetListItemTitle", mock.Anything, "item-1", "Oat milk").Return(nil)
+
+	err := svc.SetListItemTitle(context.Background(), "user-1", "item-1", "Oat milk")
+
+	require.NoError(t, err)
+	assertCallOrder(t, repo, "IsUserInListByItemID", "SetListItemTitle")
+}
+
+func TestListService_SetListItemCompleted(t *testing.T) {
+	tests := []struct {
+		name      string
+		completed bool
+	}{
+		{name: "marks the item as completed", completed: true},
+		{name: "marks the item as open again", completed: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			svc, repo, _ := newListService(t)
+
+			repo.On("IsUserInListByItemID", mock.Anything, "item-1", "user-1").Return(true, nil)
+			repo.On("SetListItemCompleted", mock.Anything, "item-1", tt.completed).Return(nil)
+
+			err := svc.SetListItemCompleted(context.Background(), "user-1", "item-1", tt.completed)
+
+			require.NoError(t, err)
+			assertCallOrder(t, repo, "IsUserInListByItemID", "SetListItemCompleted")
+		})
+	}
+}
+
+func TestListService_GetListItems(t *testing.T) {
+	svc, repo, _ := newListService(t)
+
+	want := []ListItem{
+		{ID: "item-1", ListID: "list-1", Title: "Milk"},
+		{ID: "item-2", ListID: "list-1", Title: "Bread", IsCompleted: true},
+	}
+	repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil)
+	repo.On("GetListItems", mock.Anything, "list-1").Return(want, nil)
+
+	items, err := svc.GetListItems(context.Background(), "user-1", "list-1")
+
+	require.NoError(t, err)
+	assert.Equal(t, want, items)
+	assertCallOrder(t, repo, "IsUserInList", "GetListItems")
+}
+
+type guardedOp struct {
+	name string
+	// byItem is true when membership is resolved through a list item id
+	// (IsUserInListByItemID) instead of a list id (IsUserInList).
+	byItem bool
+	// expectRepo registers the delegated repository call, returning err.
+	expectRepo func(repo *mockListRepository, err error)
+	// call invokes the service method on behalf of userID.
+	call func(svc *ListService, userID string) error
+}
+
+func (op guardedOp) guardMethod() string {
+	if op.byItem {
+		return "IsUserInListByItemID"
+	}
+	return "IsUserInList"
+}
+
+func (op guardedOp) expectGuard(repo *mockListRepository, userID string, inList bool, err error) {
+	if op.byItem {
+		repo.On("IsUserInListByItemID", mock.Anything, "item-1", userID).Return(inList, err)
+		return
+	}
+	repo.On("IsUserInList", mock.Anything, "list-1", userID).Return(inList, err)
+}
+
+func guardedOps() []guardedOp {
+	ctx := context.Background()
+
+	return []guardedOp{
+		{
+			name: "DeleteList",
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("DeleteList", mock.Anything, "list-1").Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.DeleteList(ctx, userID, "list-1")
+			},
+		},
+		{
+			name: "AddUserToList",
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("AddUserToList", mock.Anything, "list-1", "user-2").Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.AddUserToList(ctx, userID, "list-1", "user-2")
+			},
+		},
+		{
+			name: "RemoveUserFromList",
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("RemoveUserFromList", mock.Anything, "list-1", "user-2").Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.RemoveUserFromList(ctx, userID, "list-1", "user-2")
+			},
+		},
+		{
+			name: "CreateListItem",
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("CreateListItem", mock.Anything, "list-1", "Milk").Return(nil, err)
+			},
+			call: func(svc *ListService, userID string) error {
+				item, err := svc.CreateListItem(ctx, userID, "list-1", "Milk")
+				if item != nil {
+					return errors.New("expected no item on failure")
+				}
+				return err
+			},
+		},
+		{
+			name: "GetListItems",
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("GetListItems", mock.Anything, "list-1").Return(nil, err)
+			},
+			call: func(svc *ListService, userID string) error {
+				items, err := svc.GetListItems(ctx, userID, "list-1")
+				if items != nil {
+					return errors.New("expected no items on failure")
+				}
+				return err
+			},
+		},
+		{
+			name:   "DeleteListItem",
+			byItem: true,
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("DeleteListItem", mock.Anything, "item-1").Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.DeleteListItem(ctx, userID, "item-1")
+			},
+		},
+		{
+			name:   "UpdateListItem",
+			byItem: true,
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("UpdateListItem", mock.Anything, "item-1", "Oat milk", true).Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.UpdateListItem(ctx, userID, "item-1", "Oat milk", true)
+			},
+		},
+		{
+			name:   "SetListItemTitle",
+			byItem: true,
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("SetListItemTitle", mock.Anything, "item-1", "Oat milk").Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.SetListItemTitle(ctx, userID, "item-1", "Oat milk")
+			},
+		},
+		{
+			name:   "SetListItemCompleted",
+			byItem: true,
+			expectRepo: func(repo *mockListRepository, err error) {
+				repo.On("SetListItemCompleted", mock.Anything, "item-1", true).Return(err)
+			},
+			call: func(svc *ListService, userID string) error {
+				return svc.SetListItemCompleted(ctx, userID, "item-1", true)
+			},
+		},
+	}
+}
+
+func TestListService_AccessControl(t *testing.T) {
+	for _, op := range guardedOps() {
+		t.Run(op.name, func(t *testing.T) {
+			t.Run("non-member is refused before any write", func(t *testing.T) {
+				svc, repo, _ := newListService(t)
+
+				op.expectGuard(repo, "intruder", false, nil)
+
+				err := op.call(svc, "intruder")
 
 				assert.ErrorIs(t, err, ErrUserNotInList)
-		repo.AssertNotCalled(t, "SetListItemCompleted", mock.Anything, mock.Anything, mock.Anything)
+				assertCallOrder(t, repo, op.guardMethod())
 			})
+
+			t.Run("membership check error is propagated, not masked", func(t *testing.T) {
+				svc, repo, _ := newListService(t)
+
+				repoErr := errors.New("connection reset")
+				op.expectGuard(repo, "user-1", false, repoErr)
+
+				err := op.call(svc, "user-1")
+
+				assert.ErrorIs(t, err, repoErr)
+				assert.NotErrorIs(t, err, ErrUserNotInList)
+				assertCallOrder(t, repo, op.guardMethod())
+			})
+
+			t.Run("repository error is propagated", func(t *testing.T) {
+				svc, repo, _ := newListService(t)
+
+				repoErr := errors.New("write failed")
+				op.expectGuard(repo, "user-1", true, nil)
+				op.expectRepo(repo, repoErr)
+
+				err := op.call(svc, "user-1")
+
+				assert.ErrorIs(t, err, repoErr)
+				assertCallOrder(t, repo, op.guardMethod(), op.name)
+			})
+		})
+	}
 }
 
 func TestListService_ValidationErrors(t *testing.T) {
+	ctx := context.Background()
+
 	tests := []struct {
 		name    string
 		call    func(svc *ListService) error
 		wantErr error
 	}{
+		{
+			name: "CreateList without name",
+			call: func(svc *ListService) error {
+				_, err := svc.CreateList(ctx, "user-1", "")
+				return err
+			},
+			wantErr: ErrListNameMissing,
+		},
 		{
 			name:    "DeleteList without list id",
-			call:    func(svc *ListService) error { return svc.DeleteList(context.Background(), "user-1", "") },
+			call:    func(svc *ListService) error { return svc.DeleteList(ctx, "user-1", "") },
 			wantErr: ErrListIDMissing,
 		},
 		{
 			name:    "AddUserToList without list id",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.AddUserToList(ctx, "user-1", "", "user-2") },
-				return svc.AddUserToList(context.Background(), "user-1", "", "user-2")
-			},
 			wantErr: ErrListIDMissing,
 		},
 		{
 			name:    "AddUserToList without user id",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.AddUserToList(ctx, "user-1", "list-1", "") },
-				return svc.AddUserToList(context.Background(), "user-1", "list-1", "")
-			},
 			wantErr: ErrUserIDMissing,
 		},
 		{
-			name: "RemoveUserFromList without user id",
+			name:    "RemoveUserFromList without list id",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.RemoveUserFromList(ctx, "user-1", "", "user-2") },
-				return svc.RemoveUserFromList(context.Background(), "user-1", "list-1", "")
+			wantErr: ErrListIDMissing,
 		},
+		{
+			name:    "RemoveUserFromList without user id",
+			call:    func(svc *ListService) error { return svc.RemoveUserFromList(ctx, "user-1", "list-1", "") },
 			wantErr: ErrUserIDMissing,
 		},
+		{
+			name:    "OrderLists without user id",
+			call:    func(svc *ListService) error { return svc.OrderLists(ctx, "", []string{"list-1"}) },
+			wantErr: ErrUserIDMissing,
+		},
+		{
+			name:    "OrderLists with nil list ids",
+			call:    func(svc *ListService) error { return svc.OrderLists(ctx, "user-1", nil) },
+			wantErr: ErrListIDMissing,
+		},
+		{
+			name:    "OrderLists with empty list ids",
+			call:    func(svc *ListService) error { return svc.OrderLists(ctx, "user-1", []string{}) },
+			wantErr: ErrListIDMissing,
+		},
+		{
+			name: "CreateListItem without list id",
+			call: func(svc *ListService) error {
+				_, err := svc.CreateListItem(ctx, "user-1", "", "Milk")
+				return err
+			},
+			wantErr: ErrListIDMissing,
+		},
 		{
 			name: "CreateListItem without title",
 			call: func(svc *ListService) error {
-				_, err := svc.CreateListItem(context.Background(), "user-1", "list-1", "")
+				_, err := svc.CreateListItem(ctx, "user-1", "list-1", "")
 				return err
 			},
 			wantErr: ErrListItemTitleMissing,
 		},
 		{
 			name:    "DeleteListItem without item id",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.DeleteListItem(ctx, "user-1", "") },
-				return svc.DeleteListItem(context.Background(), "user-1", "")
+			wantErr: ErrListItemIDMissing,
 		},
+		{
+			name:    "UpdateListItem without item id",
+			call:    func(svc *ListService) error { return svc.UpdateListItem(ctx, "user-1", "", "Milk", false) },
 			wantErr: ErrListItemIDMissing,
 		},
 		{
 			name:    "UpdateListItem without title",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.UpdateListItem(ctx, "user-1", "item-1", "", false) },
-				return svc.UpdateListItem(context.Background(), "user-1", "item-1", "", false)
-			},
 			wantErr: ErrListItemTitleMissing,
 		},
 		{
 			name:    "SetListItemTitle without item id",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.SetListItemTitle(ctx, "user-1", "", "Milk") },
-				return svc.SetListItemTitle(context.Background(), "user-1", "", "Milk")
-			},
 			wantErr: ErrListItemIDMissing,
 		},
 		{
-			name: "SetListItemCompleted without item id",
+			name:    "SetListItemTitle without title",
-			call: func(svc *ListService) error {
+			call:    func(svc *ListService) error { return svc.SetListItemTitle(ctx, "user-1", "item-1", "") },
-				return svc.SetListItemCompleted(context.Background(), "user-1", "", true)
+			wantErr: ErrListItemTitleMissing,
 		},
+		{
+			name:    "SetListItemCompleted without item id",
+			call:    func(svc *ListService) error { return svc.SetListItemCompleted(ctx, "user-1", "", true) },
 			wantErr: ErrListItemIDMissing,
 		},
 		{
 			name: "GetListItems without list id",
 			call: func(svc *ListService) error {
-				_, err := svc.GetListItems(context.Background(), "user-1", "")
+				_, err := svc.GetListItems(ctx, "user-1", "")
 				return err
 			},
 			wantErr: ErrListIDMissing,
@@ -334,9 +763,41 @@ func TestListService_ValidationErrors(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			svc, _, _ := newListService(t)
+			svc, repo, tx := newListService(t)
 
-			assert.ErrorIs(t, tt.call(svc), tt.wantErr)
+			err := tt.call(svc)
+
+			assert.ErrorIs(t, err, tt.wantErr)
+			assert.Zero(t, tx.calls, "no transaction may be started")
+			assertCallOrder(t, repo)
+		})
+	}
+}
+
+func TestIsPermutation(t *testing.T) {
+	tests := []struct {
+		name string
+		a    []string
+		b    []string
+		want bool
+	}{
+		{name: "both empty", a: nil, b: []string{}, want: true},
+		{name: "same order", a: []string{"a", "b", "c"}, b: []string{"a", "b", "c"}, want: true},
+		{name: "reordered", a: []string{"a", "b", "c"}, b: []string{"c", "a", "b"}, want: true},
+		{name: "different case", a: []string{"ABC", "def"}, b: []string{"DEF", "abc"}, want: true},
+		{name: "first is shorter", a: []string{"a", "b"}, b: []string{"a", "b", "c"}, want: false},
+		{name: "second is shorter", a: []string{"a", "b", "c"}, b: []string{"a", "b"}, want: false},
+		{name: "same length, different element", a: []string{"a", "b"}, b: []string{"a", "c"}, want: false},
+		{name: "duplicate in first", a: []string{"a", "a"}, b: []string{"a", "b"}, want: false},
+		{name: "duplicate in second", a: []string{"a", "b"}, b: []string{"a", "a"}, want: false},
+		{name: "same duplicate on both sides", a: []string{"a", "a"}, b: []string{"a", "a"}, want: false},
+		{name: "duplicate differing only in case", a: []string{"a", "b"}, b: []string{"a", "A"}, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, isPermutation(tt.a, tt.b))
+			assert.Equal(t, tt.want, isPermutation(tt.b, tt.a), "must be symmetric")
 		})
 	}
 }

internal/repository/list.go

@@ -38,7 +38,7 @@ func (r *ListRepo) DeleteList(ctx context.Context, listID string) error {
 
 func (r *ListRepo) GetLists(ctx context.Context, userID string) ([]domain.List, error) {
 	rows, err := r.conn(ctx).QueryContext(ctx,
-		"SELECT l.id, l.name, l.created_at, l.modified_at, (SELECT COUNT(*) FROM list_items WHERE list_id=l.id), (SELECT COUNT(*) FROM list_items WHERE list_id=l.id AND is_completed=true) FROM lists AS l INNER JOIN list_users ON l.id=list_users.list_id WHERE list_users.user_id = $1",
+		"SELECT l.id, l.name, l.created_at, l.modified_at, (SELECT COUNT(*) FROM list_items WHERE list_id=l.id), (SELECT COUNT(*) FROM list_items WHERE list_id=l.id AND is_completed=true), lu.position FROM lists AS l INNER JOIN list_users AS lu ON l.id=lu.list_id WHERE lu.user_id = $1 ORDER BY lu.position",
 		userID,
 	)
 	if err != nil {
@@ -52,7 +52,7 @@ func (r *ListRepo) GetLists(ctx context.Context, userID string) ([]domain.List,
 	lists := make([]domain.List, 0, 16)
 	for rows.Next() {
 		var l domain.List
-		err = rows.Scan(&l.ID, &l.Name, &l.CreatedAt, &l.ModifiedAt, &l.TotalItems, &l.CompletedItems)
+		err = rows.Scan(&l.ID, &l.Name, &l.CreatedAt, &l.ModifiedAt, &l.TotalItems, &l.CompletedItems, &l.Position)
 		if err != nil {
 			return nil, err
 		}
@@ -87,6 +87,42 @@ func (r *ListRepo) RemoveUserFromList(ctx context.Context, listID string, userID
 	return nil
 }
 
+func (r *ListRepo) OrderLists(ctx context.Context, userID string, listIDs []string) error {
+	_, err := r.conn(ctx).ExecContext(ctx,
+		"UPDATE list_users AS lu SET position = o.idx - 1 FROM unnest($2::uuid[]) WITH ORDINALITY AS o(list_id, idx) WHERE lu.list_id = o.list_id AND lu.user_id=$1",
+		userID, listIDs,
+	)
+	if err != nil {
+		return fmt.Errorf("failed to order lists: %w", err)
+	}
+
+	return nil
+}
+
+func (r *ListRepo) LockUsersLists(ctx context.Context, userID string) ([]string, error) {
+	rows, err := r.conn(ctx).QueryContext(ctx,
+		"SELECT list_id FROM list_users WHERE user_id = $1 FOR UPDATE",
+		userID,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to lock users lists: %w", err)
+	}
+	defer rows.Close()
+
+	ids := make([]string, 0, 16)
+	for rows.Next() {
+		var listID string
+		err = rows.Scan(&listID)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read list id: %w", err)
+		}
+
+		ids = append(ids, listID)
+	}
+
+	return ids, nil
+}
+
 func (r *ListRepo) IsUserInList(ctx context.Context, listID string, userID string) (bool, error) {
 	var cnt int