feat: Add item templates feature (#435) #1099
Conversation
Add ability to create and manage item templates for quick item creation. Templates store default values and custom fields that can be applied when creating new items. Backend changes: - New ItemTemplate and TemplateField Ent schemas - Template CRUD API endpoints - Create item from template endpoint Frontend changes: - Templates management page with create/edit/delete - Template selector in item creation modal - 'Use as Template' action on item detail page - Templates link in navigation menu
WalkthroughAdds backend item-templates repository, v1 controller and routes with full CRUD plus an endpoint to create an item from a template; introduces frontend templates UI (dashboard, detail, card, selector, create modal), integrates templates into item creation flows, adds types, tests, i18n, and ESM frontend config. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant V1Controller
participant Services as services.Context
participant TemplatesRepo as TemplatesRepo
participant ItemsRepo as ItemsRepo
Client->>V1Controller: POST /api/v1/.../templates/:id/items (create-item request)
V1Controller->>Services: extract GID/auth context
V1Controller->>TemplatesRepo: GetOne(gid, templateID)
TemplatesRepo-->>V1Controller: template (defaults, fields)
V1Controller->>ItemsRepo: Create(item built from template defaults + minimal fields)
ItemsRepo-->>V1Controller: created item (id)
V1Controller->>ItemsRepo: Update(createdID, apply template fields + overrides)
alt update success
ItemsRepo-->>V1Controller: updated item
V1Controller-->>Client: 201 Created (item)
else update fails
ItemsRepo-->>V1Controller: update error
V1Controller->>ItemsRepo: Delete(createdID) (rollback)
ItemsRepo-->>V1Controller: delete result
V1Controller-->>Client: error response (original error)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Security Recommendations
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (92)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**backend/internal/data/ent/attachment.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/attachment_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/attachment_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/attachment_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/attachment_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authroles.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authroles_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authroles_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authroles_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authroles_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authtokens.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authtokens_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authtokens_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authtokens_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/authtokens_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/client.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/ent.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group/group.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/group_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/groupinvitationtoken.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/groupinvitationtoken_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/groupinvitationtoken_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/groupinvitationtoken_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/groupinvitationtoken_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/has_id.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/hook/hook.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemfield.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemfield_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemfield_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemfield_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemfield_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate/itemtemplate.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/itemtemplate_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/label.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/label_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/label_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/label_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/label_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/location.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/location_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/location_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/location_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/location_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/migrate/schema.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/mutation.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/notifier.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/notifier_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/notifier_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/notifier_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/notifier_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/predicate/predicate.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/group.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/item_template.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/template_field.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield/templatefield.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/templatefield_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/tx.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/user.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/user_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/user_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/user_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/user_update.gois excluded by!backend/internal/data/ent/**
📒 Files selected for processing (2)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go(1 hunks)backend/app/api/routes.go(1 hunks)
⏰ 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). (18)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/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: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: Frontend Tests / Integration Tests PGSQL 15
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: Backend Server Tests / Go
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
🔇 Additional comments (3)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go (2)
104-110: LGTM!The validation rules are appropriate: name is required with reasonable length limits, description has a max length, locationId is required, and quantity is optional with pointer for nil detection.
127-130: I attempted to verify the security concern raised in your review comment, but unfortunately the repository failed to clone, preventing me from examining the codebase directly.Since I cannot access the repository files to:
- Check the
ItemTemplates.GetOneimplementation- Verify if it enforces group authorization filtering
- Examine the handler's group context availability
- Compare with other handlers' authorization patterns
I cannot definitively confirm or refute the security concern about whether
ItemTemplates.GetOneverifies group ownership.Security: Verify group authorization when fetching template.
Similar to the Get handler, this template fetch at line 127 does not explicitly verify that the template belongs to the authenticated user's group. This could allow a user to create items from another group's template if they know the template ID.
If
ItemTemplates.GetOnedoes not enforce group ownership, apply this diff:// Get the template -template, err := ctrl.repo.ItemTemplates.GetOne(r.Context(), templateID) +template, err := ctrl.repo.ItemTemplates.GetOneByGroup(r.Context(), auth.GID, templateID) if err != nil { return repo.ItemOut{}, err }backend/app/api/routes.go (1)
143-149: LGTM!The item templates routes follow standard REST conventions and are properly protected with the user authentication middleware. The routing structure is consistent with existing patterns in the codebase.
- Add `CreateFromTemplate` method to ItemsRepository that creates items with all template data (including custom fields) in a single atomic transaction, replacing the previous two-phase create-then-update pattern - Fix `GetOne` to require group ID parameter so templates can only be accessed by users in the owning group (security fix) - Simplify `HandleItemTemplatesCreateItem` handler using the new transactional method
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go(1 hunks)
⏰ 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). (13)
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- 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 4/4
- GitHub Check: build (linux/arm64)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
🔇 Additional comments (6)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go (6)
21-28: LGTM! Clean implementation with proper authorization.The handler correctly extracts the authenticated group ID and passes it to the repository layer for authorization.
39-46: Authorization fix confirmed - well done!This handler now correctly passes
auth.GIDtoGetOne(line 42), ensuring that users can only access templates belonging to their own group. This addresses the previous security concern.
57-64: LGTM! Proper authorization and status code.The create handler correctly uses
auth.GIDfor group-scoped creation and returns 201 Created as appropriate for resource creation.
76-84: LGTM! Correct ID handling and authorization.The update handler properly assigns the path parameter ID to the request body (line 79) and uses
auth.GIDfor group-scoped updates.
95-103: LGTM! Proper delete implementation.The delete handler correctly uses
auth.GIDfor authorization and returns 204 No Content, which is the appropriate status for successful deletions.
123-163: Excellent refactoring - single atomic operation implemented!This handler now uses
CreateFromTemplate(line 148), which addresses the previous concern about the two-phase create-then-update pattern with flawed rollback. The new implementation:
- Single transaction: Creates the item with all template data atomically
- No rollback complexity: Eliminates the need for manual rollback logic
- Proper authorization: Fetches the template with
auth.GID(line 127)- Clean field mapping: Efficiently copies template fields (lines 138-145)
The refactoring significantly improves reliability by removing the window where an incomplete item could exist.
Updated type annotations in CreateModal.vue to use specific ItemTemplate types instead of 'any'. Improved code formatting for template fields and manufacturer display. Also refactored warranty field logic in item details page for better readability. This resolves the linter issues as well that the bot in github keeps nagging at.
|
I've gone through and updated this PR to hopefully satisfy the bot and resolve the lint errors. |
Introduces an 'id' property to each field object in CreateModal.vue and item details page to support unique identification of fields. This change prepares the codebase for future enhancements that may require field-level identification.
|
I'll look at this deeper tomorrow, hopefully we can get it merged for the next release. This PR will basically have to make it in before the next release really (and I'll see that it does), as we intend to implement a feature freeze on item/location related things after that release, so that the item/location merge can get completed without dealing with major merge conflicts every week. |
backend/internal/data/migrations/postgres/20251128120101_drop_item_templates.sql
Outdated
Show resolved
Hide resolved
Removed redundant SQL migrations per @tankerkiller125's findings.
|
@tankerkiller125 I removed those extra migration files. Thanks! |
|
So trying to play around with it locally, I get the following error when attempting to create a template from the templates list page. 
It seems to happen most notably when a custom field is added. And I think that's because the GUID in the request is empty. 
|
|
I swear I had that issue and resolved it, maybe I deleted the fix for that. It is the GUID not passing through in the request properly. |
Regarding pull sysadminsmedia#1099. Fixed an issue causing some conflict with GUIDs and old rows in the migration files.
|
@tankerkiller125 I found an issue with the migration files. They had some old rows still there that were not being used. I also found the GUID was sending as a nil string which was a no no to the server. I changed this to a proper GUID styled zeroed out string for each ID and also revised the migrations to remove those old rows. I have tested several different ways of creating the template and they all are passing now. If you wouldn't mind to give it a try and let me know if its good for you. |
tankerkiller125
left a comment
tankerkiller125
left a comment
There was a problem hiding this comment.
Everything seems to be working correctly now, and the database entries look good.
@tonyaellie not sure if you want to do a front end double check (given it's my area of weakness), but it is functional at the minimum.
|
I should be free tomorrow sometime to give it a look over :) |
tonyaellie
left a comment
tonyaellie
left a comment
There was a problem hiding this comment.
Templates should also be able to include:
labels, location, item name, item description, maybe model num
Would also be nice if template info on the item creation popup was collapsible
Template should be remembered between creations, similar to location
Don't love this ui for selecting the template, it suggests that the template is part of them item
I think move it so its next to the barcode stuff, e.g.
Not sure about having templates in the sidebar, but it works for now
The buttons here should be sqaure, there should also be a duplicate option
To many places to highlight them all, but all strings should be translated.
Overall I like where this is going but needs some work, for things where the same issue is repeated i did give up on commenting it each time, e.g. space vs gap.
|
@tonyaellie I appreciate the feedback. I think you make some good points. I'll do a comb over the code and fix the spacing, etc. I've never delved into the translation stuff so I will have to review how ya'll are handling that. |
|
No prob :), was a bit of a rush job BC I apparently have no time this week so take what I say with a grain of salt |
|
@tonyaellie So I re-organized the create item screen and implemented your recommendations.
|
Addresses recommendations from @tonyaellie. * Relocated add template button * Added more default fields to the template * Added translation of all strings (think so?) * Make oval buttons round * Added duplicate button to the template (this required a rewrite of the migration files, I made sure only 1 exists per DB type) * Added a Save as template button to a item detail view (this creates a template with all the current data of that item) * Changed all occurrences of space to gap and flex where applicable. * Made template selection persistent after item created. * Collapsible template info on creation view.
|
tonyaellie
left a comment
tonyaellie
left a comment
There was a problem hiding this comment.
Love the template button, seems like location and labels arent being saved for templates?, i think maybe move the Using template above the parent location?
Screencast.from.2025-12-03.22-56-34.mp4
Overall i think the main thing is just translating the missing strings, otherwise love the ui, maybe worth adding on the tips and tricks page about templates
|
@tonyaellie You're killing me with these translations 😂 |
I also added a test in here because I keep missing small function tests. That should prevent that from happening again.
|
OK..... I think I have gotten all of the translations (@tonyaellie please don't look lol) |
|
I think just needs linting and then im happy with this :) |
Done |
…item-templates # Conflicts: # backend/go.sum # backend/internal/data/ent/client.go # backend/internal/data/ent/group.go # backend/internal/data/ent/group_create.go # backend/internal/data/ent/group_query.go # backend/internal/data/ent/group_update.go # backend/internal/data/ent/has_id.go # backend/internal/data/ent/user.go # backend/internal/data/ent/user_create.go # backend/internal/data/ent/user_update.go
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
frontend/components/Item/CreateModal.vue (1)
137-137: Hardcoded string should be translated.
no-results-text="Type to search..."is a hardcoded English string.- no-results-text="Type to search..." + :no-results-text="$t('components.item.selector.search_placeholder')"Based on coding guidelines, hardcoded strings in UI components should be translatable.
🧹 Nitpick comments (12)
frontend/components/Template/Card.vue (1)
46-46: Consider extractingNIL_UUIDto a shared constant.
NIL_UUIDis defined identically in multiple files (CreateModal.vue,Card.vue,item/[id]/index.vue,template/[id].vue). Consider extracting to a shared utility to ensure consistency and reduce duplication.// In a shared constants file, e.g., lib/constants.ts export const NIL_UUID = "00000000-0000-0000-0000-000000000000";frontend/components/Template/Selector.vue (1)
116-122: Silent error handling may confuse users.When
api.templates.getAll()fails, the error is silently swallowed and an empty array is returned. Users won't know if there are no templates or if loading failed. Consider showing a toast or error state.const { data: templates } = useAsyncData("templates-selector", async () => { const { data, error } = await api.templates.getAll(); if (error) { + // Optionally notify user of load failure + console.warn("Failed to load templates for selector"); return []; } return data; });Alternatively, expose an error state to show inline feedback in the UI.
frontend/components/Item/CreateModal.vue (2)
109-112: Use proper type instead ofany.The
l: anytype annotation bypasses type safety. UseTemplateLabelSummarywhich is already imported viaItemTemplateOut.- {{ templateData.defaultLabels.map((l: any) => l.name).join(", ") }} + {{ templateData.defaultLabels.map((l) => l.name).join(", ") }}TypeScript should infer the type from
templateData.defaultLabelswhich isTemplateLabelSummary[].
369-447: Refactor: Extract duplicate form pre-filling logic.
handleTemplateSelected(lines 389-406) andrestoreLastTemplate(lines 429-446) contain nearly identical form pre-filling logic. Consider extracting to a shared helper function.+ function applyTemplateToForm(data: ItemTemplateOut) { + form.quantity = data.defaultQuantity; + if (data.defaultName) { + form.name = data.defaultName; + } + if (data.defaultDescription) { + form.description = data.defaultDescription; + } + if (data.defaultLocation) { + const found = locations.value.find(l => l.id === data.defaultLocation!.id); + if (found) { + form.location = found; + } + } + if (data.defaultLabels && data.defaultLabels.length > 0) { + form.labels = data.defaultLabels.map(l => l.id); + } + } async function handleTemplateSelected(template: ItemTemplateSummary | null) { // ... template null check and API call ... templateData.value = data; - form.quantity = data.defaultQuantity; - // ... rest of form pre-fill ... + applyTemplateToForm(data); localStorage.setItem(LAST_TEMPLATE_KEY, template.id); toast.success(t("components.template.toast.applied", { name: data.name })); } async function restoreLastTemplate() { // ... template load logic ... selectedTemplate.value = { id: data.id, name: data.name, description: data.description } as ItemTemplateSummary; templateData.value = data; - form.quantity = data.defaultQuantity; - // ... rest of form pre-fill ... + applyTemplateToForm(data); }backend/internal/data/repo/repo_item_templates_test.go (8)
12-30: Centralized test-data factory is solid; consider making edge-cases easier to exercise
templateFactorykeeps tests DRY and consistent. If you later need edge-case coverage (e.g., zero quantities, very long strings, toggling include flags), consider parameterizing this helper (options/overrides) so tests can easily construct specific scenarios without duplicating struct literals.
32-51: Avoid deleting zero-value templates in cleanup for a slightly cleaner helper
useTemplatesworks, but if a failure happens mid-loop, some entries intemplateswill remain zero-valued andDeletewill be called with a zero UUID. It’s harmless since errors are ignored, but you could make this a bit tighter by appending to the slice only after a successful create, and iterating over that int.Cleanup.
53-59: Clarify expectations forGetAllresult sizeAsserting
len(all) >= 5is safe if other fixtures may exist intGroup. IftGroupis guaranteed to be isolated per test run, asserting equality (== 5) would catch accidental leakage of templates between tests. Either way, a brief comment explaining why>=is intentional would help future readers.
72-91:Createtest is strong; consider asserting a couple more defaultsThe
Createtest already validates a good subset of fields and non-nil ID. If you want to harden it further, you might also assert default booleans (e.g.,IncludeWarrantyFields,IncludePurchaseFields,IncludeSoldFields) to ensure they persist as expected fromtemplateFactory.
93-112: Field-ordering assumptions may be brittle; verify content, not just positionsBoth
CreateWithFieldsandUpdateWithFieldsimplicitly assume a deterministic order oftemplate.Fields:
CreateWithFieldsindexes[0]and[1]and checks name/value.UpdateWithFieldsonly assertsLen == 2without checking which field was updated vs. newly inserted.If the repository ever changes how it orders fields (e.g., DB default order, explicit sort), these tests may become flaky or fail for reasons unrelated to correctness.
Consider:
- Asserting based on IDs/names regardless of position (e.g., loop and find by
NameorID).- In
UpdateWithFields, also asserting that:
- The original field’s ID is preserved and its name/value are updated.
- The new field has a distinct ID and expected name/value.
This gives stronger guarantees about field-update semantics without tying tests to ordering details.
Also applies to: 157-187
189-201: Strengthen delete behavior assertion with a specific not-found error (if available)Right now the test only requires that
GetOnereturns “some error” after delete. If the repo exposes a specific not-found sentinel (e.g.,ErrNotFound), it’s worth asserting that exact error to avoid conflating genuine delete success with other failures (DB outage, group mismatch, etc.).
203-221:DeleteCascadesFieldsname suggests cascade validation—consider checking fields explicitlyThis test currently verifies that the template itself is gone, which is necessary but doesn’t fully prove that its fields were cascade-deleted. If you have a way to query template fields directly (via repo or ent client), consider asserting that no
TemplateFieldrows remain for the deleted template ID. That would align the test more tightly with its name and protect against regressions in cascade configuration.
223-249: Add negative tests for cross-group associations to harden multi-tenant isolationThe location/label association tests for creates look good for the happy path and validate that related entities are wired correctly. For security in a multi-tenant setup, it would be valuable to add tests that:
- Attempt to create an item template in
tGroupwhile referencing aLocationorLabelbelonging to a different group.- Assert that the repository rejects these with a clear error (and does not persist the template).
That kind of test helps enforce group isolation and reduces the risk of IDOR-style cross-group associations if a caller ever manages to send foreign IDs.
Also applies to: 251-282
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**backend/internal/data/ent/migrate/schema.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/mutation.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime.gois excluded by!backend/internal/data/ent/**docs/en/api/openapi-2.0.jsonis excluded by!docs/en/api/**docs/en/api/openapi-2.0.yamlis excluded by!docs/en/api/**docs/en/api/openapi-3.0.jsonis excluded by!docs/en/api/**docs/en/api/openapi-3.0.yamlis excluded by!docs/en/api/**docs/en/api/swagger-2.0.jsonis excluded by!docs/en/api/**docs/en/api/swagger-2.0.yamlis excluded by!docs/en/api/**
📒 Files selected for processing (14)
backend/app/api/routes.go(1 hunks)backend/internal/data/repo/repo_item_templates_test.go(1 hunks)frontend/components/Item/CreateModal.vue(9 hunks)frontend/components/Template/Card.vue(1 hunks)frontend/components/Template/CreateModal.vue(1 hunks)frontend/components/Template/Selector.vue(1 hunks)frontend/lib/api/__test__/factories/index.ts(3 hunks)frontend/lib/api/__test__/user/templates.test.ts(1 hunks)frontend/lib/api/types/data-contracts.ts(11 hunks)frontend/locales/en.json(3 hunks)frontend/package.json(1 hunks)frontend/pages/item/[id]/index.vue(4 hunks)frontend/pages/template/[id].vue(1 hunks)frontend/pages/templates.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/api/routes.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,vue}
⚙️ CodeRabbit configuration file
**/*.{ts,vue}: Check for hardcoded strings in UI components that should be translatable.
Look for:
- String literals in Vue components (e.g. Click me)
- Alert messages, error messages, and user-facing text
- Placeholder text and labels
Files:
frontend/lib/api/__test__/factories/index.tsfrontend/pages/templates.vuefrontend/components/Template/Selector.vuefrontend/components/Template/CreateModal.vuefrontend/lib/api/__test__/user/templates.test.tsfrontend/components/Template/Card.vuefrontend/pages/template/[id].vuefrontend/components/Item/CreateModal.vuefrontend/lib/api/types/data-contracts.tsfrontend/pages/item/[id]/index.vue
🧠 Learnings (1)
📚 Learning: 2025-07-07T19:05:42.824Z
Learnt from: crumbowl
Repo: sysadminsmedia/homebox PR: 818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:05:42.824Z
Learning: In frontend/components/Item/BarcodeModal.vue, the user crumbowl considers DOMPurify sanitization of external API product data to be overkill for the barcode product import feature, preferring to keep the implementation simpler.
Applied to files:
frontend/components/Item/CreateModal.vue
🧬 Code graph analysis (3)
frontend/lib/api/__test__/factories/index.ts (1)
frontend/lib/api/types/data-contracts.ts (1)
ItemTemplateCreate(703-736)
frontend/lib/api/__test__/user/templates.test.ts (4)
frontend/lib/api/user.ts (1)
UserClient(17-51)frontend/lib/api/types/data-contracts.ts (1)
ItemTemplateOut(738-763)frontend/lib/api/__test__/factories/index.ts (1)
factories(113-124)frontend/lib/api/__test__/test-utils.ts (1)
sharedUserClient(14-41)
frontend/lib/api/types/data-contracts.ts (3)
backend/internal/data/repo/repo_item_templates.go (7)
ItemTemplateCreate(40-66)TemplateField(23-28)ItemTemplateOut(105-134)TemplateLabelSummary(30-33)TemplateLocationSummary(35-38)ItemTemplateSummary(97-103)ItemTemplateUpdate(68-95)backend/app/api/handlers/v1/controller.go (1)
OIDCStatus(103-108)backend/app/api/handlers/v1/v1_ctrl_item_templates.go (1)
ItemTemplateCreateItemRequest(105-111)
⏰ 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). (18)
- GitHub Check: build (linux/arm64)
- GitHub Check: Frontend Tests / Integration Tests PGSQL 15
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: Backend Server Tests / Go
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: build (linux/arm/v7)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: build (linux/amd64)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: build (linux/arm/v7)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (23)
frontend/package.json (1)
3-3: Verify ESM compatibility across build scripts, dependencies, and existing code.Adding
"type": "module"topackage.jsonis required for Nuxt 4.1.0, which is ESM-first. While this is a necessary change aligned with the framework's design, ensure:
- All build and development scripts (dev, build, postinstall, lint, test) execute without errors.
- CommonJS dependencies are imported using interop patterns (default export + destructure).
- Existing frontend code follows ESM syntax.
Confirm that build and test pipelines pass without module resolution errors before merging.
frontend/lib/api/__test__/factories/index.ts (1)
58-78: LGTM! Factory follows established patterns.The
template()factory correctly implements all required fields fromItemTemplateCreateand follows the same conventions as existing factories (user,location,label). The use of faker for generating test data and sensible defaults for boolean/array fields is appropriate.frontend/pages/item/[id]/index.vue (2)
590-596: Verify boolean logic for price fields.The
includePurchaseFieldsandincludeSoldFieldsflags use truthiness checks on price values. A price of0is falsy in JavaScript, so a purchase/sold section withpurchasePrice: 0but with apurchaseFromorpurchaseTimeset would still work correctly. However, if only the price is set to a non-zero value, this logic is fine.Just confirming this is the intended behavior—if the user explicitly sets a price of
0, the section flags depend on other fields being populated.
711-733: Good UX consolidation of actions into dropdown menu.The dropdown menu implementation addresses the UI clutter concern raised in previous reviews. Proper use of destructive styling for the delete action and consistent iconography throughout.
frontend/components/Template/CreateModal.vue (1)
1-88: Well-structured form with proper i18n and accessibility.All user-facing strings use
$t()for translation, form controls have proper label associations viaid/forattributes, and the dynamic custom fields section is cleanly implemented.frontend/pages/templates.vue (1)
1-69: Clean implementation of templates listing page.The page correctly implements auth middleware, responsive grid layout, empty state handling, and proper i18n usage throughout. The
handleDuplicatedwrapper maintains the event signature compatibility while triggering refresh.frontend/components/Template/Card.vue (1)
38-82: Duplication logic is well-implemented.The approach of fetching the full template before duplicating ensures all fields are captured correctly. Proper null coalescing for optional fields (
defaultLocation?.id,defaultLabels?.map) and comprehensive field mapping.frontend/components/Template/Selector.vue (1)
124-133: Good toggle selection pattern.The selection logic properly handles both selecting a new template and deselecting the current one (by clicking it again). Both
update:modelValueandtemplate-selectedevents are emitted appropriately.frontend/lib/api/__test__/user/templates.test.ts (2)
7-198: Comprehensive test coverage for template CRUD lifecycle.The test suite thoroughly covers:
- Create with validation of all fields
- GetAll with array verification
- Update with full field set
- Delete with 404 verification
- Custom fields create and update
Good use of the
useTemplatehelper for setup/cleanup and proper assertions on both HTTP status codes and data integrity.
200-297: Good integration tests for template relationships.Tests properly verify:
- Creating templates with default location
- Creating templates with default labels
- Updating templates to remove location
Resource cleanup is handled in the correct order (template → dependent resources).
frontend/pages/template/[id].vue (2)
248-331: Well-structured detail view with proper i18n.The template detail view is cleanly organized with:
- Responsive header with actions
- Conditional rendering for optional fields
- Two-column grid for default values and custom fields
- Proper use of
$t()for all user-facing strings
142-246: Update dialog form is consistent with CreateModal.The update form mirrors the create form structure, maintaining UI consistency. Proper use of dialog components and form controls with accessibility attributes.
frontend/locales/en.json (2)
233-291: Comprehensive i18n coverage for the template feature.The new localization keys under
components.templateprovide thorough coverage for all template-related UI elements including cards, modals, forms, selectors, and toast messages. The structure follows existing patterns in the codebase.
617-624: Menu and page strings added correctly.The
templatesmenu item andpages.templatessection are properly structured and consistent with other menu/page entries.frontend/lib/api/types/data-contracts.ts (4)
20-22: Template field type enum is intentionally limited.The
TemplatefieldTypeenum currently only supportsTypeText, which differs fromItemfieldTypethat includesnumber,boolean, andtime. This appears intentional for the initial template implementation. Future iterations may expand template field types.
310-361: Entity interfaces align with backend schema.The
EntItemTemplateandEntItemTemplateEdgesinterfaces correctly model the backend Ent schema with all expected fields for default values, metadata flags, and relationships.
703-807: Template CRUD interfaces are well-defined.The
ItemTemplateCreate,ItemTemplateOut,ItemTemplateSummary, andItemTemplateUpdateinterfaces correctly mirror the backend repository structs with appropriate validation annotations (@minLength,@maxLength).
1119-1130:ItemTemplateCreateItemRequestaligns with backend DTO.This interface matches the backend
ItemTemplateCreateItemRequeststruct used for creating items from templates, with proper validation constraints onname(1-255) anddescription(max 1000).frontend/components/Item/CreateModal.vue (3)
351-357: Security note: localStorage template ID storage is acceptable.Storing only the template ID (a UUID) in localStorage is a safe practice. The ID itself doesn't contain sensitive data and requires authentication to fetch template details. However, be aware that localStorage persists across sessions and is accessible to any JavaScript on the same origin.
647-655: Form and template state reset after creation.The form fields and template-related state are properly cleared after successful item creation. Note that
localStorage.removeItem(LAST_TEMPLATE_KEY)is not called here, which means the template preference persists for the next item creation. Verify this is the intended UX behavior per the PR discussion mentioning "template selection persist after item creation".
583-612: Verify ifparentIdshould be passed to template-based item creation.The template and standard item creation flows are logically separated correctly. However, template creation omits the
parentIdfield that's included in standard item creation. Confirm whether:
- The backend
api.templates.createItemendpoint supportsparentId- Sub-item creation via templates should inherit
parentIdfrom the form- This omission is intentional or requires a fix
backend/internal/data/repo/repo_item_templates_test.go (2)
61-70: Good coverage forGetOnehappy pathThe
GetOnetest exercises ID, name, and description, which is enough to validate basic mapping from the repository. Looks good as-is.
284-320:UpdateRemoveLocationcleanly validates disassociation behaviorThis test nicely checks the “remove default location” behavior by setting
DefaultLocationID: niland confirmingDefaultLocationbecomesnilon the returned template. That’s a good guard against regressions in update logic.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/pages/item/[id]/index.vue (1)
772-772: Hardcoded string should be translated.The "Show Empty" label text is hardcoded English. Per coding guidelines, UI strings should be translatable.
- Show Empty + {{ $t("items.show_empty") }}
♻️ Duplicate comments (1)
frontend/components/Item/CreateModal.vue (1)
426-428: Type assertion still missing required fields.The
ItemTemplateSummarytype requirescreatedAtandupdatedAtfields per the data contracts, but onlyid,name, anddescriptionare provided. This could cause runtime issues if these fields are accessed elsewhere.- selectedTemplate.value = { id: data.id, name: data.name, description: data.description } as ItemTemplateSummary; + selectedTemplate.value = { + id: data.id, + name: data.name, + description: data.description, + createdAt: data.createdAt, + updatedAt: data.updatedAt, + };
🧹 Nitpick comments (3)
frontend/components/Template/CreateModal.vue (1)
137-137: Consider adding validation for empty field names.Custom fields with empty names could lead to confusing template data. Consider adding client-side validation before submission.
async function create() { if (loading.value) return; + + // Validate custom fields have names + const invalidFields = form.fields.filter(f => !f.name.trim()); + if (invalidFields.length > 0) { + toast.error(t("components.template.toast.field_name_required")); + return; + } + loading.value = true;frontend/components/Item/CreateModal.vue (1)
111-111: Consider typing the label mapping parameter.Using
anyloses type safety. The label type fromtemplateData.defaultLabelsshould have a known shape.- {{ templateData.defaultLabels.map((l: any) => l.name).join(", ") }} + {{ templateData.defaultLabels.map((l) => l.name).join(", ") }}Since
templateDatais typed asItemTemplateOut, thedefaultLabelsarray should already have proper typing from the data contracts.backend/internal/data/repo/repo_item_templates.go (1)
263-310: Consider wrapping Create in a transaction for atomicity.The
Createmethod performs multiple database operations (template creation + field creations). If a field creation fails after the template is created, you'll have a partial template without all its fields.// Consider using a transaction: tx, err := r.db.Tx(ctx) if err != nil { return ItemTemplateOut{}, err } defer tx.Rollback() // Use tx instead of r.db for all operations // ... if err := tx.Commit(); err != nil { return ItemTemplateOut{}, err }This ensures either all operations succeed or none do.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**docs/en/api/openapi-3.0.jsonis excluded by!docs/en/api/**docs/en/api/openapi-3.0.yamlis excluded by!docs/en/api/**docs/en/api/swagger-2.0.jsonis excluded by!docs/en/api/**docs/en/api/swagger-2.0.yamlis excluded by!docs/en/api/**
📒 Files selected for processing (7)
backend/internal/data/repo/repo_item_templates.go(1 hunks)backend/internal/data/repo/repo_item_templates_test.go(1 hunks)frontend/components/Item/CreateModal.vue(9 hunks)frontend/components/Template/CreateModal.vue(1 hunks)frontend/lib/api/types/data-contracts.ts(11 hunks)frontend/locales/en.json(4 hunks)frontend/pages/item/[id]/index.vue(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/internal/data/repo/repo_item_templates_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,vue}
⚙️ CodeRabbit configuration file
**/*.{ts,vue}: Check for hardcoded strings in UI components that should be translatable.
Look for:
- String literals in Vue components (e.g. Click me)
- Alert messages, error messages, and user-facing text
- Placeholder text and labels
Files:
frontend/components/Template/CreateModal.vuefrontend/pages/item/[id]/index.vuefrontend/lib/api/types/data-contracts.tsfrontend/components/Item/CreateModal.vue
🧠 Learnings (5)
📚 Learning: 2025-09-22T18:57:53.157Z
Learnt from: tonyaellie
Repo: sysadminsmedia/homebox PR: 998
File: frontend/components/Item/View/table/columns.ts:43-57
Timestamp: 2025-09-22T18:57:53.157Z
Learning: In Vue.js, camelCase props like `ariaLabel` are automatically converted to kebab-case HTML attributes like `aria-label` when rendered to the DOM. This means accessibility attributes work correctly even when written in camelCase in component props.
Applied to files:
frontend/components/Item/CreateModal.vue
📚 Learning: 2025-09-23T09:13:06.831Z
Learnt from: tonyaellie
Repo: sysadminsmedia/homebox PR: 998
File: frontend/components/Item/View/table/columns.ts:199-205
Timestamp: 2025-09-23T09:13:06.831Z
Learning: In the Homebox codebase, date fields from the API (like createdAt and updatedAt) are guaranteed to be Date objects, so runtime type checking is unnecessary when casting to Date in components like frontend/components/Item/View/table/columns.ts.
Applied to files:
frontend/components/Item/CreateModal.vue
📚 Learning: 2025-09-23T17:41:14.271Z
Learnt from: tonyaellie
Repo: sysadminsmedia/homebox PR: 998
File: frontend/components/Maintenance/EditModal.vue:123-131
Timestamp: 2025-09-23T17:41:14.271Z
Learning: In the Homebox maintenance system, MaintenanceEntry and MaintenanceEntryWithDetails interfaces define completedDate and scheduledDate as "Date | string" (non-nullable), so null checks are not needed when creating Date objects from these fields in frontend/components/Maintenance/EditModal.vue.
Applied to files:
frontend/components/Item/CreateModal.vue
📚 Learning: 2025-09-23T17:41:14.271Z
Learnt from: tonyaellie
Repo: sysadminsmedia/homebox PR: 998
File: frontend/components/Maintenance/EditModal.vue:123-131
Timestamp: 2025-09-23T17:41:14.271Z
Learning: In the Homebox codebase, MaintenanceEntry and MaintenanceEntryWithDetails objects always have non-null completedDate and scheduledDate fields, so null checks before calling new Date() on these fields in frontend/components/Maintenance/EditModal.vue are unnecessary.
Applied to files:
frontend/components/Item/CreateModal.vue
📚 Learning: 2025-07-07T19:05:42.824Z
Learnt from: crumbowl
Repo: sysadminsmedia/homebox PR: 818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:05:42.824Z
Learning: In frontend/components/Item/BarcodeModal.vue, the user crumbowl considers DOMPurify sanitization of external API product data to be overkill for the barcode product import feature, preferring to keep the implementation simpler.
Applied to files:
frontend/components/Item/CreateModal.vue
⏰ 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). (13)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
🔇 Additional comments (10)
frontend/pages/item/[id]/index.vue (2)
568-613: Well-structured template creation from item data.The
saveAsTemplatefunction properly constructs the template payload from item values. The use ofNIL_UUIDfor new field IDs aligns with the GUID handling fix discussed in PR comments.One consideration: the function navigates away immediately after creation. If the user wanted to stay on the item page, they'd need to navigate back.
711-733: Good consolidation of actions into dropdown menu.The "More actions" dropdown reduces header clutter (as noted in past review comments) and properly translates the aria-label. The destructive delete action is visually distinguished with
text-destructive.frontend/components/Template/CreateModal.vue (1)
174-210: Proper explicit payload construction.The
createDataobject is now explicitly constructed without spreading the form, which correctly excludesdefaultLocationObjectfrom the API request. This addresses the previous review feedback about potential API validation issues.frontend/locales/en.json (2)
234-292: Comprehensive template translations added.Good coverage of all template-related UI strings including card actions, form fields, selector, and toast messages. The use of placeholders like
{name}for dynamic content follows i18n best practices.
618-625: Menu and page translations properly added.The templates menu item and page title translations are in place, supporting the new navigation entry.
frontend/components/Item/CreateModal.vue (2)
46-128: Well-designed collapsible template banner.The template info display is thoughtfully implemented with:
- Clear visual distinction using primary color accent
- Collapsible details to reduce clutter
- Accessible clear button with translated aria-label
- Comprehensive display of template defaults
The UX of showing/hiding defaults keeps the modal clean while providing full transparency.
583-612: Template-based creation properly branches API calls.Good separation between template-based and standard item creation paths. The template request correctly includes only the fields needed for the
createItemendpoint.frontend/lib/api/types/data-contracts.ts (1)
1-12: Auto-generated types file - no manual changes needed.This file is generated from Swagger definitions. The new template-related types (
ItemTemplateCreate,ItemTemplateOut,ItemTemplateSummary, etc.) align with the backend API contracts.backend/internal/data/repo/repo_item_templates.go (2)
226-243: Group-scoped queries provide proper tenant isolation.The
GetAllquery correctly filters by group ID, ensuring templates from other groups are not accessible. This is good security practice for multi-tenant applications.
429-451: Delete verifies group ownership before deletion.Good security pattern: the delete operation first verifies the template belongs to the requesting group before allowing deletion. This prevents unauthorized cross-tenant deletions.
| // Delete fields that are no longer present | ||
| for _, field := range existingFields { | ||
| if !updatedFieldIDs[field.ID] { | ||
| err = r.db.TemplateField.DeleteOne(field).Exec(ctx) | ||
| if err != nil { | ||
| log.Err(err).Msg("failed to delete template field") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Field deletion errors are logged but not propagated.
If deleting a field fails during Update, the error is logged but the operation continues. This could leave the template in an inconsistent state with fields that should have been removed.
for _, field := range existingFields {
if !updatedFieldIDs[field.ID] {
err = r.db.TemplateField.DeleteOne(field).Exec(ctx)
if err != nil {
log.Err(err).Msg("failed to delete template field")
+ return ItemTemplateOut{}, err
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Delete fields that are no longer present | |
| for _, field := range existingFields { | |
| if !updatedFieldIDs[field.ID] { | |
| err = r.db.TemplateField.DeleteOne(field).Exec(ctx) | |
| if err != nil { | |
| log.Err(err).Msg("failed to delete template field") | |
| } | |
| } | |
| } | |
| // Delete fields that are no longer present | |
| for _, field := range existingFields { | |
| if !updatedFieldIDs[field.ID] { | |
| err = r.db.TemplateField.DeleteOne(field).Exec(ctx) | |
| if err != nil { | |
| log.Err(err).Msg("failed to delete template field") | |
| return ItemTemplateOut{}, err | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In backend/internal/data/repo/repo_item_templates.go around lines 415 to 423,
the loop that deletes template fields logs deletion errors but swallows them;
update it to propagate failures so the Update operation aborts on error: when
DeleteOne.Exec(ctx) returns an error, return that error (or wrap it with
context) immediately instead of only logging, ensuring the caller can roll back
or handle the failed delete and preventing a partially-updated template state;
if the surrounding Update runs inside a transaction, make sure to return the
error so the transaction can be rolled back.
|
@Wrr2216 please share an OpenCollective username, and we'll get the bounty stuff sorted. Thanks. |
|
4 participants
What type of PR is this?
Feature
What this PR does / why we need it
Adds Item Templates so users can save reusable templates with default values and custom fields for quick item creation.
Which issue(s) this PR fixes
Fixes #435
Special notes for your reviewer
Backend:
item_template.go/template_field.go– New Ent schemas for templates and their custom fieldsgroup.go– Added templates edgerepo_item_templates.go– Repository with CRUDv1_ctrl_item_templates.go/routes.go– API handlers and routesFrontend:
templates.vue/template/[id].vue– List and detail pagesTemplate/components – Card, CreateModal, SelectorItem/CreateModal.vue– Integrated template selectoritem/[id]/index.vue– "Use as Template" actiondefault.vue– Nav linktemplates.ts– API clientAPI Endpoints:
GET/POST /api/v1/templates– List and createGET/PUT/DELETE /api/v1/templates/{id}– Read, update, deletePOST /api/v1/templates/{id}/create-item– Create item from templateTesting
Both builds pass. Manually tested template CRUD, creating items from templates, and the "Use as Template" flow.
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.