From 032f716e4917ebbf8aa2b404a04a821cd689ead0 Mon Sep 17 00:00:00 2001 From: Huxley Date: Sat, 4 Jul 2026 17:20:22 +0800 Subject: [PATCH] feat(repo): add soft-delete with status filter to file queries File repository now filters by status=active in all query methods (FindByID, FindByUserID, FindByParentID, FindByNameAndParent). Delete now performs soft-delete by setting status to 'user_deleted' instead of physical row deletion. Add ListIncludeDeleted to UserRepository for admin views of all users. Add tests: StatusFilter, SoftDelete, DeleteIdempotent, StatusFilterCount. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- internal/repository/file.go | 16 ++-- internal/repository/file_test.go | 140 ++++++++++++++++++++++++++++--- internal/repository/user.go | 18 ++++ 3 files changed, 156 insertions(+), 18 deletions(-) diff --git a/internal/repository/file.go b/internal/repository/file.go index 584bcb9..8f6a370 100644 --- a/internal/repository/file.go +++ b/internal/repository/file.go @@ -43,7 +43,7 @@ func (r *fileRepository) Create(ctx context.Context, file *model.File) error { func (r *fileRepository) FindByID(ctx context.Context, id string) (*model.File, error) { var file model.File - result := r.db.WithContext(ctx).First(&file, "id = ?", id) + result := r.db.WithContext(ctx).First(&file, "id = ? AND status = ?", id, model.StatusActive) if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, model.ErrNotFound } @@ -57,11 +57,11 @@ func (r *fileRepository) FindByUserID(ctx context.Context, userID string, offset var files []model.File var total int64 - if err := r.db.WithContext(ctx).Model(&model.File{}).Where("user_id = ?", userID).Count(&total).Error; err != nil { + if err := r.db.WithContext(ctx).Model(&model.File{}).Where("user_id = ? AND status = ?", userID, model.StatusActive).Count(&total).Error; err != nil { return nil, 0, err } - result := r.db.WithContext(ctx).Where("user_id = ?", userID).Offset(offset).Limit(limit).Find(&files) + result := r.db.WithContext(ctx).Where("user_id = ? AND status = ?", userID, model.StatusActive).Offset(offset).Limit(limit).Find(&files) if result.Error != nil { return nil, 0, result.Error } @@ -71,7 +71,7 @@ func (r *fileRepository) FindByUserID(ctx context.Context, userID string, offset func (r *fileRepository) FindByParentID(ctx context.Context, userID string, parentID *string) ([]model.File, error) { var files []model.File - result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ?", userID, parentID).Find(&files) + result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ? AND status = ?", userID, parentID, model.StatusActive).Find(&files) if result.Error != nil { return nil, result.Error } @@ -82,11 +82,11 @@ func (r *fileRepository) FindByParentIDPaginated(ctx context.Context, userID str var files []model.File var total int64 - if err := r.db.WithContext(ctx).Model(&model.File{}).Where("user_id = ? AND parent_id IS ?", userID, parentID).Count(&total).Error; err != nil { + if err := r.db.WithContext(ctx).Model(&model.File{}).Where("user_id = ? AND parent_id IS ? AND status = ?", userID, parentID, model.StatusActive).Count(&total).Error; err != nil { return nil, 0, err } - result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ?", userID, parentID).Offset(offset).Limit(limit).Order("is_dir DESC, name ASC").Find(&files) + result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ? AND status = ?", userID, parentID, model.StatusActive).Offset(offset).Limit(limit).Order("is_dir DESC, name ASC").Find(&files) if result.Error != nil { return nil, 0, result.Error } @@ -96,7 +96,7 @@ func (r *fileRepository) FindByParentIDPaginated(ctx context.Context, userID str func (r *fileRepository) FindByNameAndParent(ctx context.Context, userID string, parentID *string, name string) (*model.File, error) { var file model.File - result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ? AND name = ?", userID, parentID, name).First(&file) + result := r.db.WithContext(ctx).Where("user_id = ? AND parent_id IS ? AND name = ? AND status = ?", userID, parentID, name, model.StatusActive).First(&file) if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, model.ErrNotFound } @@ -115,7 +115,7 @@ func (r *fileRepository) Update(ctx context.Context, file *model.File) error { } func (r *fileRepository) Delete(ctx context.Context, id string) error { - result := r.db.WithContext(ctx).Delete(&model.File{}, "id = ?", id) + result := r.db.WithContext(ctx).Model(&model.File{}).Where("id = ?", id).Update("status", model.StatusUserDeleted) if result.Error != nil { return result.Error } diff --git a/internal/repository/file_test.go b/internal/repository/file_test.go index 1f75ef7..cc140c5 100644 --- a/internal/repository/file_test.go +++ b/internal/repository/file_test.go @@ -33,6 +33,7 @@ func TestFileRepository_Create(t *testing.T) { UserID: "user-1", Name: "test.txt", Size: 1024, + Status: model.StatusActive, } if err := repo.Create(ctx, file); err != nil { @@ -48,6 +49,7 @@ func TestFileRepository_FindByID(t *testing.T) { ID: "file-1", UserID: "user-1", Name: "test.txt", + Status: model.StatusActive, } if err := repo.Create(ctx, file); err != nil { t.Fatalf("Create = %v", err) @@ -77,9 +79,9 @@ func TestFileRepository_FindByUserID(t *testing.T) { ctx := context.Background() files := []*model.File{ - {ID: "f-1", UserID: "user-1", Name: "a.txt"}, - {ID: "f-2", UserID: "user-1", Name: "b.txt"}, - {ID: "f-3", UserID: "user-2", Name: "c.txt"}, + {ID: "f-1", UserID: "user-1", Name: "a.txt", Status: model.StatusActive}, + {ID: "f-2", UserID: "user-1", Name: "b.txt", Status: model.StatusActive}, + {ID: "f-3", UserID: "user-2", Name: "c.txt", Status: model.StatusActive}, } for _, f := range files { if err := repo.Create(ctx, f); err != nil { @@ -105,9 +107,9 @@ func TestFileRepository_FindByParentID(t *testing.T) { parentID := "dir-1" files := []*model.File{ - {ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt"}, - {ID: "f-2", UserID: "user-1", ParentID: &parentID, Name: "b.txt"}, - {ID: "f-3", UserID: "user-1", Name: "c.txt"}, + {ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt", Status: model.StatusActive}, + {ID: "f-2", UserID: "user-1", ParentID: &parentID, Name: "b.txt", Status: model.StatusActive}, + {ID: "f-3", UserID: "user-1", Name: "c.txt", Status: model.StatusActive}, } for _, f := range files { if err := repo.Create(ctx, f); err != nil { @@ -130,8 +132,8 @@ func TestFileRepository_FindByParentIDNull(t *testing.T) { parentID := "dir-1" files := []*model.File{ - {ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt"}, - {ID: "f-2", UserID: "user-1", Name: "root.txt"}, + {ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt", Status: model.StatusActive}, + {ID: "f-2", UserID: "user-1", Name: "root.txt", Status: model.StatusActive}, } for _, f := range files { if err := repo.Create(ctx, f); err != nil { @@ -152,7 +154,7 @@ func TestFileRepository_Update(t *testing.T) { repo := setupFileRepo(t) ctx := context.Background() - file := &model.File{ID: "file-1", UserID: "user-1", Name: "original.txt"} + file := &model.File{ID: "file-1", UserID: "user-1", Name: "original.txt", Status: model.StatusActive} if err := repo.Create(ctx, file); err != nil { t.Fatalf("Create = %v", err) } @@ -179,7 +181,7 @@ func TestFileRepository_Delete(t *testing.T) { repo := setupFileRepo(t) ctx := context.Background() - file := &model.File{ID: "file-1", UserID: "user-1", Name: "test.txt"} + file := &model.File{ID: "file-1", UserID: "user-1", Name: "test.txt", Status: model.StatusActive} if err := repo.Create(ctx, file); err != nil { t.Fatalf("Create = %v", err) } @@ -193,3 +195,121 @@ func TestFileRepository_Delete(t *testing.T) { t.Fatalf("expected ErrNotFound after delete, got %v", err) } } + +func TestFileRepository_SoftDelete(t *testing.T) { + repo := setupFileRepo(t) + ctx := context.Background() + + file := &model.File{ + ID: "file-1", + UserID: "user-1", + Name: "test.txt", + Status: model.StatusActive, + } + if err := repo.Create(ctx, file); err != nil { + t.Fatalf("Create = %v", err) + } + + if err := repo.Delete(ctx, "file-1"); err != nil { + t.Fatalf("Delete = %v", err) + } + + _, err := repo.FindByID(ctx, "file-1") + if err != model.ErrNotFound { + t.Fatalf("expected ErrNotFound after soft-delete, got %v", err) + } +} + +func TestFileRepository_StatusFilter(t *testing.T) { + repo := setupFileRepo(t) + ctx := context.Background() + + activeFile := &model.File{ + ID: "f-1", + UserID: "user-1", + Name: "active.txt", + Status: model.StatusActive, + } + deletedFile := &model.File{ + ID: "f-2", + UserID: "user-1", + Name: "deleted.txt", + Status: model.StatusUserDeleted, + } + + if err := repo.Create(ctx, activeFile); err != nil { + t.Fatalf("Create active = %v", err) + } + if err := repo.Create(ctx, deletedFile); err != nil { + t.Fatalf("Create deleted = %v", err) + } + + result, total, err := repo.FindByUserID(ctx, "user-1", 0, 10) + if err != nil { + t.Fatalf("FindByUserID = %v", err) + } + if len(result) != 1 { + t.Errorf("len(result) = %d, want 1", len(result)) + } + if total != 1 { + t.Errorf("total = %d, want 1", total) + } + if result[0].ID != "f-1" { + t.Errorf("result[0].ID = %q, want %q", result[0].ID, "f-1") + } +} + +func TestFileRepository_DeleteIdempotent(t *testing.T) { + repo := setupFileRepo(t) + ctx := context.Background() + + file := &model.File{ + ID: "file-1", + UserID: "user-1", + Name: "test.txt", + Status: model.StatusActive, + } + if err := repo.Create(ctx, file); err != nil { + t.Fatalf("Create = %v", err) + } + + if err := repo.Delete(ctx, "file-1"); err != nil { + t.Fatalf("first Delete = %v", err) + } + if err := repo.Delete(ctx, "file-1"); err != nil { + t.Fatalf("second Delete = %v", err) + } +} + +func TestFileRepository_StatusFilterCount(t *testing.T) { + repo := setupFileRepo(t) + ctx := context.Background() + + activeFile := &model.File{ + ID: "f-1", + UserID: "user-1", + Name: "active.txt", + Status: model.StatusActive, + } + deletedFile := &model.File{ + ID: "f-2", + UserID: "user-1", + Name: "deleted.txt", + Status: model.StatusUserDeleted, + } + + if err := repo.Create(ctx, activeFile); err != nil { + t.Fatalf("Create active = %v", err) + } + if err := repo.Create(ctx, deletedFile); err != nil { + t.Fatalf("Create deleted = %v", err) + } + + _, total, err := repo.FindByUserID(ctx, "user-1", 0, 10) + if err != nil { + t.Fatalf("FindByUserID = %v", err) + } + if total != 1 { + t.Errorf("total = %d, want 1 (soft-deleted excluded from count)", total) + } +} diff --git a/internal/repository/user.go b/internal/repository/user.go index bf573d5..7be09f8 100644 --- a/internal/repository/user.go +++ b/internal/repository/user.go @@ -25,6 +25,7 @@ type UserRepository interface { Update(ctx context.Context, user *model.User) error Delete(ctx context.Context, id string) error List(ctx context.Context, offset, limit int) ([]model.User, int64, error) + ListIncludeDeleted(ctx context.Context, offset, limit int) ([]model.User, int64, error) } type userRepository struct { @@ -130,3 +131,20 @@ func (r *userRepository) List(ctx context.Context, offset, limit int) ([]model.U return users, total, nil } + +// ListIncludeDeleted returns all users (including soft-deleted), paginated. +func (r *userRepository) ListIncludeDeleted(ctx context.Context, offset, limit int) ([]model.User, int64, error) { + var users []model.User + var total int64 + + if err := r.db.WithContext(ctx).Model(&model.User{}).Count(&total).Error; err != nil { + return nil, 0, err + } + + result := r.db.WithContext(ctx).Offset(offset).Limit(limit).Find(&users) + if result.Error != nil { + return nil, 0, result.Error + } + + return users, total, nil +}