Conversation
|
Warning Rate limit exceeded@itsLeonB has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughRenames CRUDRepository to Repository and NewCRUDRepository to NewRepository. Introduces a DeletedFilter on Specification and applies its WhereDeleted scope in FindAll and FindFirst. Adds public DeletedFilter presets (ExcludeDeleted, IncludeDeleted, OnlyDeleted) backed by new internal scope implementations. Updates and adds tests for soft-delete filtering and API rename. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Repository
participant GORM_DB as GORM DB
Caller->>Repository: FindAll(spec)
activate Repository
Note over Repository: Build query with Specification
Repository->>GORM_DB: Apply scopes (filters, pagination)
Repository->>GORM_DB: Apply spec.DeletedFilter.WhereDeleted()
GORM_DB-->>Repository: Result set
deactivate Repository
Repository-->>Caller: []T
sequenceDiagram
participant Caller
participant Repository
participant GORM_DB as GORM DB
Caller->>Repository: FindFirst(spec)
activate Repository
Note over Repository: Build query with Specification
Repository->>GORM_DB: Apply scopes (filters, order)
Repository->>GORM_DB: Apply spec.DeletedFilter.WhereDeleted()
GORM_DB-->>Repository: First row or not found
deactivate Repository
Repository-->>Caller: *T / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
test/scopes_test.go (1)
283-325: Test coverage is good; consider adding repository-level assertions for DeletedFilter.Current tests validate DeletedFilter at the DB scope level. Add repository.FindAll/FindFirst tests using Specification.DeletedFilter (Exclude/Include/Only) to ensure end‑to‑end behavior.
Would you like me to draft a small test using repo + SoftDeleteModel to assert Exclude/Include/Only semantics?
gorm_scopes.go (2)
110-121: Use a value receiver for DeletedFilter.WhereDeleted to avoid nil‑receiver edge cases.Pointer receiver isn’t needed; a value receiver simplifies usage and avoids accidental nil pointer calls.
Apply:
- func (df *DeletedFilter) WhereDeleted() func(*gorm.DB) *gorm.DB { + func (df DeletedFilter) WhereDeleted() func(*gorm.DB) *gorm.DB { return func(d *gorm.DB) *gorm.DB { - if df.filterType == nil { + if df.filterType == nil { return d } return df.filterType.WhereDeleted()(d) } }
123-127: Consider brief doc comments on presets.Add one‑liners explaining each preset to aid discoverability in IDEs.
test/repository_test.go (1)
234-251: Add assertions for Specification.DeletedFilter.Augment tests to validate Exclude/Include/Only via the repository to catch regressions.
Example to add:
func TestRepository_DeletedFilter(t *testing.T) { db := setupTestDB(t) repo := crud.NewRepository[SoftDeleteModel](db) ctx := context.Background() // seed m1, _ := repo.Insert(ctx, SoftDeleteModel{Name: "A1"}) m2, _ := repo.Insert(ctx, SoftDeleteModel{Name: "A2"}) _ = db.Delete(&m2).Error // soft delete one // ExcludeDeleted (default behavior) res, err := repo.FindAll(ctx, crud.Specification[SoftDeleteModel]{DeletedFilter: crud.ExcludeDeleted}) assert.NoError(t, err) assert.Len(t, res, 1) // IncludeDeleted res, err = repo.FindAll(ctx, crud.Specification[SoftDeleteModel]{DeletedFilter: crud.IncludeDeleted}) assert.NoError(t, err) assert.Len(t, res, 2) // OnlyDeleted res, err = repo.FindAll(ctx, crud.Specification[SoftDeleteModel]{DeletedFilter: crud.OnlyDeleted}) assert.NoError(t, err) assert.Len(t, res, 1) assert.Equal(t, m2.ID, res[0].ID) _ = db.Unscoped().Delete(&m1).Error }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
gorm_repository.go(11 hunks)gorm_scopes.go(1 hunks)internal/gorm_scopes.go(1 hunks)test/repository_test.go(7 hunks)test/scopes_test.go(2 hunks)test/transactor_test.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
gorm_scopes.go (1)
internal/gorm_scopes.go (4)
DeletedFilterType(5-7)ExcludeDeleted(9-9)IncludeDeleted(17-17)OnlyDeleted(25-25)
test/transactor_test.go (2)
gorm_repository.go (1)
NewRepository(42-48)test/repository_test.go (1)
TestModel(16-23)
test/scopes_test.go (2)
test/repository_test.go (1)
TestModel(16-23)gorm_scopes.go (4)
DeletedFilter(110-112)ExcludeDeleted(124-124)IncludeDeleted(125-125)OnlyDeleted(126-126)
internal/gorm_scopes.go (1)
gorm_scopes.go (3)
ExcludeDeleted(124-124)IncludeDeleted(125-125)OnlyDeleted(126-126)
test/repository_test.go (1)
gorm_repository.go (1)
NewRepository(42-48)
gorm_repository.go (1)
gorm_scopes.go (1)
DeletedFilter(110-112)
🔇 Additional comments (5)
test/transactor_test.go (1)
94-94: API rename usage is correct.NewRepository[T] constructor usage in all three spots looks good and transaction flow remains intact.
Also applies to: 130-130, 167-167
test/scopes_test.go (1)
14-21: SoftDeleteModel and migration LGTM.Schema and DeletedAt index are appropriate for soft deletes; migration includes the model.
Also applies to: 29-29
gorm_repository.go (2)
81-89: Verify DeletedFilter works end‑to‑end after internal Unscoped fix.FindAll applies spec.DeletedFilter.WhereDeleted(), which currently won’t include/only deleted without Unscoped(). With the internal change proposed, this will be correct.
Please re-run repository tests using IncludeDeleted and OnlyDeleted in Specification to confirm results.
106-114: Same DeletedFilter concern applies here.Behavior mirrors FindAll; ensure tests cover FindFirst for Include/Only variants.
test/repository_test.go (1)
37-42: Constructor/API rename usage LGTM across tests.Also applies to: 46-47, 73-74, 107-108, 136-137, 167-168, 195-196, 226-227
1 participant
Summary by CodeRabbit
New Features
Refactor
Tests