Conversation
WalkthroughAssigns an 8-character ImportRef to newly created inventory items during Create by calling SetImportRef(uuid.New().String()[0:8]) after AssetID assignment. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Security Recommendations
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/data/repo/repo_items.go (1)
631-639: Critical:data.ImportRefis unconditionally overwritten, breaking import idempotency.Line 632 sets
ImportReffromdata.ImportRef, but line 639 immediately overwrites it with a newly generated value. This defeats the PR objective of supporting idempotent re-imports—when importing items with existingHB.import_refvalues, the provided reference is discarded and replaced with a random one.The auto-generation should only apply when no
ImportRefis provided.🐛 Proposed fix: conditionally generate ImportRef
func (e *ItemsRepository) Create(ctx context.Context, gid uuid.UUID, data ItemCreate) (ItemOut, error) { + importRef := data.ImportRef + if importRef == "" { + importRef = uuid.New().String()[0:8] + } + q := e.db.Item.Create(). - SetImportRef(data.ImportRef). + SetImportRef(importRef). SetName(data.Name). SetQuantity(data.Quantity). SetDescription(data.Description). SetGroupID(gid). SetLocationID(data.LocationID). - SetAssetID(int(data.AssetID)). - SetImportRef(uuid.New().String()[0:8]) + SetAssetID(int(data.AssetID))
🧹 Nitpick comments (2)
backend/internal/data/repo/repo_items.go (2)
697-709: Consider adding auto-generated ImportRef toCreateFromTemplatefor consistency.The
Createmethod now auto-generates anImportRef, butCreateFromTemplatedoes not set one. This inconsistency means items created from templates will have emptyImportRefvalues and won't be exportable with proper references until the "ensure refs" tool is run.♻️ Suggested addition
itemBuilder := tx.Item.Create(). SetID(newItemID). SetName(data.Name). SetDescription(data.Description). SetQuantity(data.Quantity). SetLocationID(data.LocationID). SetGroupID(gid). SetAssetID(int(nextAssetID)). + SetImportRef(uuid.New().String()[0:8]). SetInsured(data.Insured).
1376-1400: Same consistency issue:Duplicateshould also auto-generate ImportRef.Duplicated items will not have an
ImportRefset, creating the same export/import gap addressed by this PR for the standardCreatepath.♻️ Suggested addition
itemBuilder := tx.Item.Create(). SetID(newItemID). SetName(options.CopyPrefix + originalItem.Name). SetDescription(originalItem.Description). SetQuantity(originalItem.Quantity). SetLocationID(originalItem.Location.ID). SetGroupID(gid). SetAssetID(int(nextAssetID)). + SetImportRef(uuid.New().String()[0:8]). SetSerialNumber(originalItem.SerialNumber).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/internal/data/repo/repo_items.go
🧰 Additional context used
📓 Path-based instructions (1)
backend/**/*.go
📄 CodeRabbit inference engine (.github/instructions/code.instructions.md)
Backend Go code must pass golangci-lint validation with no errors (6-minute timeout in CI)
Files:
backend/internal/data/repo/repo_items.go
🧬 Code graph analysis (1)
backend/internal/data/repo/repo_items.go (1)
backend/internal/data/repo/asset_id_type.go (1)
AssetID(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build (linux/amd64, ubuntu-latest)
- GitHub Check: build (linux/arm64, ubuntu-24.04-arm)
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 15
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: Backend Server Tests / Go
- GitHub Check: build (linux/amd64, ubuntu-latest)
- GitHub Check: build (linux/amd64, ubuntu-latest)
- GitHub Check: build (linux/arm64, ubuntu-24.04-arm)
- GitHub Check: build (linux/arm64, ubuntu-24.04-arm)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
backend/internal/data/repo/repo_items.go (1)
638-639: The codebase already handles collision checking where needed; 8-character UUID truncation is acceptable for import reference idempotency.The
CheckRef()method already exists (line 351) and is actively used during CSV import to prevent duplicate items with the same ImportRef. The 8-character truncation reduces entropy to ~32 bits, making collisions theoretically possible at scale (≈65k+ items), but this is a non-critical, non-security identifier used solely for import idempotency, so the risk is acceptable for typical inventory systems.
Deploying homebox-docs with
|
| Latest commit: |
d17ffe1
|
| Status: | 🚫 Build failed. |
1 participant
What type of PR is this?
What this PR does / why we need it:
Automatically sets the import ref field for items on creation. Eliminating the need to run the ensure refs tool every single time before export.
Which issue(s) this PR fixes:
Fixes: #793
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.