From bff131ba429d2f238d834e7c9a87bcb0d1b81c42 Mon Sep 17 00:00:00 2001 From: Huxley Date: Sat, 4 Jul 2026 17:08:10 +0800 Subject: [PATCH] feat(repo): soft-delete User with status filter on queries --- internal/repository/user.go | 24 ++++-- internal/repository/user_test.go | 141 +++++++++++++++++++++++++++++-- 2 files changed, 153 insertions(+), 12 deletions(-) diff --git a/internal/repository/user.go b/internal/repository/user.go index 18ccdfb..bf573d5 100644 --- a/internal/repository/user.go +++ b/internal/repository/user.go @@ -19,6 +19,7 @@ func isDuplicateKeyError(err error) bool { type UserRepository interface { Create(ctx context.Context, user *model.User) error FindByID(ctx context.Context, id string) (*model.User, error) + FindByIDIncludeDeleted(ctx context.Context, id string) (*model.User, error) FindByEmail(ctx context.Context, email string) (*model.User, error) FindByUsername(ctx context.Context, username string) (*model.User, error) Update(ctx context.Context, user *model.User) error @@ -47,6 +48,19 @@ func (r *userRepository) Create(ctx context.Context, user *model.User) error { } func (r *userRepository) FindByID(ctx context.Context, id string) (*model.User, error) { + var user model.User + result := r.db.WithContext(ctx).First(&user, "id = ? AND status = ?", id, model.StatusActive) + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, model.ErrNotFound + } + if result.Error != nil { + return nil, result.Error + } + return &user, nil +} + +// FindByIDIncludeDeleted finds a user by ID regardless of status. +func (r *userRepository) FindByIDIncludeDeleted(ctx context.Context, id string) (*model.User, error) { var user model.User result := r.db.WithContext(ctx).First(&user, "id = ?", id) if errors.Is(result.Error, gorm.ErrRecordNotFound) { @@ -60,7 +74,7 @@ func (r *userRepository) FindByID(ctx context.Context, id string) (*model.User, func (r *userRepository) FindByEmail(ctx context.Context, email string) (*model.User, error) { var user model.User - result := r.db.WithContext(ctx).First(&user, "email = ?", email) + result := r.db.WithContext(ctx).First(&user, "email = ? AND status = ?", email, model.StatusActive) if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, model.ErrNotFound } @@ -72,7 +86,7 @@ func (r *userRepository) FindByEmail(ctx context.Context, email string) (*model. func (r *userRepository) FindByUsername(ctx context.Context, username string) (*model.User, error) { var user model.User - result := r.db.WithContext(ctx).First(&user, "username = ?", username) + result := r.db.WithContext(ctx).First(&user, "username = ? AND status = ?", username, model.StatusActive) if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, model.ErrNotFound } @@ -94,7 +108,7 @@ func (r *userRepository) Update(ctx context.Context, user *model.User) error { } func (r *userRepository) Delete(ctx context.Context, id string) error { - result := r.db.WithContext(ctx).Delete(&model.User{}, "id = ?", id) + result := r.db.WithContext(ctx).Model(&model.User{}).Where("id = ?", id).Update("status", model.StatusAdminDeleted) if result.Error != nil { return result.Error } @@ -105,11 +119,11 @@ func (r *userRepository) List(ctx context.Context, offset, limit int) ([]model.U var users []model.User var total int64 - if err := r.db.WithContext(ctx).Model(&model.User{}).Count(&total).Error; err != nil { + if err := r.db.WithContext(ctx).Model(&model.User{}).Where("status = ?", model.StatusActive).Count(&total).Error; err != nil { return nil, 0, err } - result := r.db.WithContext(ctx).Offset(offset).Limit(limit).Find(&users) + result := r.db.WithContext(ctx).Where("status = ?", model.StatusActive).Offset(offset).Limit(limit).Find(&users) if result.Error != nil { return nil, 0, result.Error } diff --git a/internal/repository/user_test.go b/internal/repository/user_test.go index 07a1f29..2bbe34d 100644 --- a/internal/repository/user_test.go +++ b/internal/repository/user_test.go @@ -33,6 +33,7 @@ func TestUserRepository_Create(t *testing.T) { Username: "alice", Email: "alice@example.com", PasswordHash: "hash", + Status: model.StatusActive, } if err := repo.Create(ctx, user); err != nil { @@ -44,8 +45,8 @@ func TestUserRepository_CreateDuplicateUsername(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - u1 := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} - u2 := &model.User{ID: "user-2", Username: "alice", Email: "alice2@example.com", PasswordHash: "hash"} + u1 := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} + u2 := &model.User{ID: "user-2", Username: "alice", Email: "alice2@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, u1); err != nil { t.Fatalf("Create = %v", err) @@ -61,7 +62,7 @@ func TestUserRepository_FindByID(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} + user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) } @@ -89,7 +90,7 @@ func TestUserRepository_FindByEmail(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} + user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) } @@ -107,7 +108,7 @@ func TestUserRepository_FindByUsername(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} + user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) } @@ -125,7 +126,7 @@ func TestUserRepository_Update(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} + user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) } @@ -148,7 +149,7 @@ func TestUserRepository_Delete(t *testing.T) { repo := setupUserRepo(t) ctx := context.Background() - user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash"} + user := &model.User{ID: "user-1", Username: "alice", Email: "alice@example.com", PasswordHash: "hash", Status: model.StatusActive} if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) } @@ -173,6 +174,7 @@ func TestUserRepository_List(t *testing.T) { Username: "user" + string(rune('0'+i)), Email: "user" + string(rune('0'+i)) + "@example.com", PasswordHash: "hash", + Status: model.StatusActive, } if err := repo.Create(ctx, user); err != nil { t.Fatalf("Create = %v", err) @@ -190,3 +192,128 @@ func TestUserRepository_List(t *testing.T) { t.Errorf("total = %d, want %d", total, 5) } } + +func TestUserRepository_SoftDelete(t *testing.T) { + repo := setupUserRepo(t) + ctx := context.Background() + + user := &model.User{ + ID: "user-1", + Username: "alice", + Email: "alice@example.com", + PasswordHash: "hash", + Status: model.StatusActive, + } + if err := repo.Create(ctx, user); err != nil { + t.Fatalf("Create = %v", err) + } + + if err := repo.Delete(ctx, "user-1"); err != nil { + t.Fatalf("Delete = %v", err) + } + + // After soft-delete, FindByID should return ErrNotFound + // because the status filter excludes soft-deleted users. + _, err := repo.FindByID(ctx, "user-1") + if err != model.ErrNotFound { + t.Fatalf("expected ErrNotFound after soft-delete, got %v", err) + } +} + +func TestUserRepository_DisabledLogin(t *testing.T) { + repo := setupUserRepo(t) + ctx := context.Background() + + user := &model.User{ + ID: "user-1", + Username: "alice", + Email: "alice@example.com", + PasswordHash: "hash", + Status: model.StatusActive, + } + if err := repo.Create(ctx, user); err != nil { + t.Fatalf("Create = %v", err) + } + + if err := repo.Delete(ctx, "user-1"); err != nil { + t.Fatalf("Delete = %v", err) + } + + // Soft-deleted users should not be findable by email, + // preventing login for disabled accounts. + _, err := repo.FindByEmail(ctx, "alice@example.com") + if err != model.ErrNotFound { + t.Fatalf("expected ErrNotFound for soft-deleted user login, got %v", err) + } +} + +func TestUserRepository_StatusFilterList(t *testing.T) { + repo := setupUserRepo(t) + ctx := context.Background() + + u1 := &model.User{ + ID: "user-1", + Username: "alice", + Email: "alice@example.com", + PasswordHash: "hash", + Status: model.StatusActive, + } + if err := repo.Create(ctx, u1); err != nil { + t.Fatalf("Create u1 = %v", err) + } + + u2 := &model.User{ + ID: "user-2", + Username: "bob", + Email: "bob@example.com", + PasswordHash: "hash", + Status: model.StatusActive, + } + if err := repo.Create(ctx, u2); err != nil { + t.Fatalf("Create u2 = %v", err) + } + + // Soft-delete bob + if err := repo.Delete(ctx, "user-2"); err != nil { + t.Fatalf("Delete u2 = %v", err) + } + + // List with status filter should only return active users. + users, total, err := repo.List(ctx, 0, 10) + if err != nil { + t.Fatalf("List = %v", err) + } + if len(users) != 1 { + t.Fatalf("expected 1 active user, got %d", len(users)) + } + if total != 1 { + t.Fatalf("expected total 1, got %d", total) + } + if users[0].ID != "user-1" { + t.Errorf("expected active user user-1, got %s", users[0].ID) + } +} + +func TestUserRepository_DeleteIdempotent(t *testing.T) { + repo := setupUserRepo(t) + ctx := context.Background() + + user := &model.User{ + ID: "user-1", + Username: "alice", + Email: "alice@example.com", + PasswordHash: "hash", + Status: model.StatusActive, + } + if err := repo.Create(ctx, user); err != nil { + t.Fatalf("Create = %v", err) + } + + // Soft-delete the same user twice should not error. + if err := repo.Delete(ctx, "user-1"); err != nil { + t.Fatalf("first Delete = %v", err) + } + if err := repo.Delete(ctx, "user-1"); err != nil { + t.Fatalf("second Delete = %v", err) + } +}