-
-
Notifications
You must be signed in to change notification settings - Fork 315
Feat/barcode refactoring #920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds barcode support end-to-end: DB migration and model fields, backend barcode repository and HTTP handler for external lookups, frontend UI/components for barcode input, display and selection, tests, docs, and sample test data. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant FE as Frontend
participant BE as Backend
participant API as External Barcode API
U->>FE: Enter/scan barcode
FE->>BE: GET /v1/products/search-from-barcode?productEAN=...
BE->>API: Parallel queries to UPCItemDB & BarcodeSpider
API-->>BE: JSON product responses
BE->>BE: Merge results, fetch/validate image (HTTPS, size cap), base64-encode
BE-->>FE: 200 OK with products (or 204 No Content)
FE->>U: Render selectable product cards
U->>FE: Select product & confirm
FE->>BE: POST /items (payload includes barcode, manufacturer, modelNumber)
BE-->>FE: Created item response
FE->>U: Show created item with barcode details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Security Recommendations
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 21
🔭 Outside diff range comments (2)
backend/internal/data/migrations/sqlite3/20250723163847_add_barcode.sql (1)
1-5: Consider adding validation constraints for enhanced security and data integrity.While the migration correctly adds the barcode column, consider implementing database-level validation to ensure data quality and security:
-- +goose Up ALTER TABLE items ADD COLUMN barcode TEXT CHECK ( barcode IS NULL OR (LENGTH(barcode) BETWEEN 8 AND 48 AND barcode GLOB '[0-9]*') );This adds constraints to:
- Limit barcode length to reasonable bounds (8-48 characters)
- Ensure numeric-only content to prevent injection attacks
- Allow NULL values for items without barcodes
Security recommendation: Database-level validation helps prevent malicious data insertion and maintains data consistency across all application entry points.
frontend/components/global/DetailsSection/types.ts (1)
46-64: Missing barcode case in filterZeroValues function.The
filterZeroValuesfunction doesn't include a case for the new "barcode" type, which could lead to barcodes being filtered out unexpectedly.Apply this diff to handle the barcode type:
export function filterZeroValues(details: Details): Details { return details.filter(detail => { switch (detail.type) { case "date": return validDate(detail.text); case "currency": return !!detail.text; case "link": return !!detail.text && !!detail.href; + case "barcode": + return !!detail.text; case undefined: case "text": case "markdown": return detail.text !== null && detail.text !== "" && detail.text !== undefined; default: console.warn("Unknown detail type (this should never happen)", detail); return false; } }); }
🧹 Nitpick comments (5)
docs/en/user-guide/tips-tricks.md (1)
18-30: Great documentation! Fix markdown formatting for consistency.The barcode autofill feature documentation is clear and comprehensive. However, please address the markdown formatting issues identified by static analysis.
Apply this diff to use consistent dash formatting for unordered lists:
-* From the *Add item* dialog: click on one of the barcode icons on the top right. You can either manually enter a barcode, or scan it using the QRCode scanner. -* Directly from *QR code scanner* dialog: once a barcode is detected, Homebox will ask you if you want to fetch the product information. +- From the *Add item* dialog: click on one of the barcode icons on the top right. You can either manually enter a barcode, or scan it using the QRCode scanner. +- Directly from *QR code scanner* dialog: once a barcode is detected, Homebox will ask you if you want to fetch the product information.Security Note: Ensure that the barcode API integration (barcodespider.com and UPCItemDB) validates external data properly to prevent potential injection attacks or malformed data from compromising the system.
backend/internal/data/repo/repo_barcodes_test.go (1)
11-16: Simplify error handling with testing.T methods.The custom
checkErrorhelper can be replaced with the standardt.Fatal()ort.Fatalf()methods for cleaner test code.-func checkError(t *testing.T, err error) { - if err != nil { - t.Helper() // mark as helper so failure line points to caller - t.Fatalf("unexpected error: %v", err) - } -}Then replace usage:
-checkError(t, err) +if err != nil { + t.Fatalf("unexpected error: %v", err) +}backend/app/api/handlers/v1/v1_ctrl_barcodes.go (1)
20-24: LGTM! Proper query parameter structure and validation.The query struct is well-defined with appropriate validation constraints. The 80-character limit aligns with the longest non-2D barcode format (GS1-128).
Security Note: Consider adding additional validation to ensure the EAN contains only valid barcode characters (typically numeric) to prevent injection attacks or malformed queries to external APIs.
type query struct { // 80 characters is the longest non-2D barcode length (GS1-128) - EAN string `schema:"productEAN" validate:"required,max=80"` + EAN string `schema:"productEAN" validate:"required,max=80,numeric"` }frontend/components/Item/BarcodeModal.vue (1)
43-49: Security consideration: Frontend data sanitizationWhile I understand from previous discussions that backend validation is comprehensive, consider adding a lightweight sanitization layer for external product data displayed in the frontend as defense-in-depth against potential XSS attacks. This is especially important since the data comes from external APIs.
backend/internal/data/repo/repo_barcodes.go (1)
302-346: Consider implementing rate limiting for external API callsTo prevent hitting API rate limits and ensure responsible usage, consider implementing rate limiting for external API calls using a library like
golang.org/x/time/rate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
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/go.sumis excluded by!**/*.sumbackend/internal/data/ent/item.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item/item.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item/where.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_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/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/item.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/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
backend/app/api/demo.go(1 hunks)backend/app/api/handlers/v1/v1_ctrl_barcodes.go(1 hunks)backend/app/api/handlers/v1/v1_ctrl_product_search.go(0 hunks)backend/app/api/routes.go(1 hunks)backend/internal/core/services/reporting/io_row.go(1 hunks)backend/internal/data/migrations/sqlite3/20250723163847_add_barcode.sql(1 hunks)backend/internal/data/repo/repo_barcodes.go(1 hunks)backend/internal/data/repo/repo_barcodes_test.go(1 hunks)backend/internal/data/repo/repo_items.go(7 hunks)backend/internal/data/repo/repo_items_search_test.go(2 hunks)backend/internal/data/repo/repo_product_search.go(0 hunks)backend/internal/data/repo/repos_all.go(2 hunks)backend/pkgs/textutils/normalize.go(1 hunks)docs/en/configure/index.md(1 hunks)docs/en/user-guide/tips-tricks.md(1 hunks)frontend/assets/css/main.css(1 hunks)frontend/components/App/CreateModal.vue(2 hunks)frontend/components/App/ScannerModal.vue(1 hunks)frontend/components/Form/TextField.vue(3 hunks)frontend/components/Item/BarcodeCard.vue(1 hunks)frontend/components/Item/BarcodeModal.vue(3 hunks)frontend/components/Item/CreateModal.vue(5 hunks)frontend/components/Item/View/Selectable.vue(3 hunks)frontend/components/Item/View/Table.types.ts(1 hunks)frontend/components/Item/View/Table.vue(10 hunks)frontend/components/global/Barcode.vue(1 hunks)frontend/components/global/DetailsSection/DetailsSection.vue(1 hunks)frontend/components/global/DetailsSection/types.ts(2 hunks)frontend/components/ui/dialog/Dialog.vue(2 hunks)frontend/components/ui/input/Input.vue(2 hunks)frontend/composables/use-preferences.ts(1 hunks)frontend/lib/api/__test__/user/items.test.ts(7 hunks)frontend/lib/api/types/data-contracts.ts(5 hunks)frontend/locales/en.json(1 hunks)frontend/package.json(1 hunks)frontend/pages/home/index.vue(1 hunks)frontend/pages/item/[id]/index.vue(3 hunks)frontend/pages/item/[id]/index/edit.vue(1 hunks)frontend/pages/label/[id].vue(1 hunks)frontend/pages/location/[id].vue(1 hunks)
💤 Files with no reviewable changes (2)
- backend/app/api/handlers/v1/v1_ctrl_product_search.go
- backend/internal/data/repo/repo_product_search.go
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: crumbowl
PR: sysadminsmedia/homebox#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.
Learnt from: crumbowl
PR: sysadminsmedia/homebox#818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/components/App/CreateModal.vue (1)
Learnt from: crumbowl
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.
frontend/components/App/ScannerModal.vue (1)
Learnt from: crumbowl
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.
frontend/pages/home/index.vue (1)
Learnt from: crumbowl
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.
frontend/components/Item/CreateModal.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/lib/api/__test__/user/items.test.ts (1)
Learnt from: crumbowl
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.
frontend/lib/api/types/data-contracts.ts (2)
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
Learnt from: crumbowl
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.
backend/internal/data/migrations/sqlite3/20250723163847_add_barcode.sql (1)
Learnt from: mcarbonne
PR: #869
File: backend/internal/data/migrations/sqlite3/20250706190000_fix_old_timestamps.sql:0-0
Timestamp: 2025-07-06T18:51:05.524Z
Learning: Goose database migrations are automatically wrapped in transactions by the migration framework, eliminating the need for explicit BEGIN TRANSACTION and COMMIT statements in migration scripts.
frontend/components/Item/BarcodeModal.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/components/Item/View/Selectable.vue (1)
Learnt from: crumbowl
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.
frontend/package.json (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/pages/label/[id].vue (1)
Learnt from: crumbowl
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.
frontend/components/Item/View/Table.vue (1)
Learnt from: crumbowl
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.
frontend/components/global/DetailsSection/DetailsSection.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/pages/item/[id]/index/edit.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/components/Item/BarcodeCard.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/components/global/Barcode.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/locales/en.json (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
frontend/pages/item/[id]/index.vue (2)
Learnt from: crumbowl
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.
Learnt from: crumbowl
PR: #818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
🧬 Code Graph Analysis (6)
backend/internal/data/repo/repo_barcodes_test.go (1)
backend/internal/sys/config/conf.go (1)
BarcodeAPIConf(74-76)
backend/internal/data/repo/repo_items.go (1)
backend/internal/data/repo/asset_id_type.go (1)
AssetID(9-9)
frontend/composables/use-preferences.ts (1)
frontend/components/Item/View/Table.types.ts (1)
TableHeaderType(5-13)
backend/app/api/handlers/v1/v1_ctrl_barcodes.go (2)
backend/app/api/handlers/v1/controller.go (1)
V1Controller(67-77)backend/internal/web/adapters/decoders.go (1)
DecodeQuery(17-30)
frontend/components/Item/View/Table.types.ts (2)
frontend/lib/api/types/data-contracts.ts (3)
ItemSummary(586-605)BarcodeProduct(456-465)ItemCreate(507-523)backend/internal/data/repo/repo_items.go (2)
ItemSummary(124-147)ItemCreate(59-74)
backend/internal/data/repo/repos_all.go (1)
backend/internal/data/repo/repo_barcodes.go (1)
BarcodeRepository(19-20)
🪛 markdownlint-cli2 (0.17.2)
docs/en/user-guide/tips-tricks.md
28-28: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
29-29: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
🔇 Additional comments (43)
backend/pkgs/textutils/normalize.go (2)
25-25: LGTM - Code formatting improvement.The removal of extraneous blank lines improves code consistency and readability without affecting functionality.
31-31: LGTM - Code formatting improvement.The removal of extraneous blank lines improves code consistency and readability without affecting functionality.
frontend/locales/en.json (1)
286-286: LGTM - Localization key addition.The barcode localization key addition is clean and supports the new barcode functionality.
Security recommendation: Ensure all user-facing strings are properly escaped when rendered in the UI to prevent XSS attacks, especially if barcode data from external APIs is ever displayed using these localization keys.
frontend/composables/use-preferences.ts (1)
15-15: LGTM - Consistent type extension for barcode preferences.The addition of
tableHeadersBarcodefollows the same pattern as the existingtableHeadersproperty and properly supports the new barcode functionality.Security recommendation: Ensure that any user preferences stored in localStorage are validated when loaded to prevent potential XSS through malicious preference data injection.
frontend/pages/location/[id].vue (1)
220-220: LGTM - Type-specific prop addition.The
item-typeprop addition correctly specifies the data type for the component, enabling proper handling of different item types in the barcode feature enhancement.Security recommendation: Ensure that the
ItemViewSelectablecomponent properly validates theitem-typeprop value to prevent potential component confusion attacks or unexpected behavior from malicious prop values.frontend/pages/label/[id].vue (1)
195-195: LGTM! Clean UI enhancement for item type distinction.The addition of the
item-typeprop with value'itemsummary'properly categorizes items for theItemViewSelectablecomponent, enabling consistent handling across different contexts.backend/app/api/demo.go (1)
14-21: LGTM! Demo data properly structured with secure content.The CSV header correctly includes the new
HB.barcodecolumn, and all data rows maintain consistent structure with empty barcode fields. The demo data contains realistic IoT device information without any security concerns.Security recommendation: The demo data is properly sanitized with no injection risks or malicious content detected.
backend/internal/core/services/reporting/io_row.go (1)
36-36: LGTM! Properly positioned barcode field in CSV export structure.The
Barcodefield is correctly added with appropriate CSV tag and positioned logically with other identification fields.Security recommendation: Ensure that CSV exports containing barcode data are properly access-controlled and consider implementing data sanitization for barcode fields to prevent potential information leakage through exports.
frontend/pages/home/index.vue (1)
45-45: LGTM! Standardizing item type specification.The addition of the
item-typeprop with value'itemsummary'properly standardizes how item types are specified across the application. This aligns well with the barcode feature implementation.Security Note: While this change is low-risk, ensure that the
ItemViewTablecomponent properly validates and sanitizes theitem-typeprop to prevent any potential XSS vulnerabilities if this prop is ever rendered directly to the DOM.frontend/pages/item/[id]/index/edit.vue (1)
202-207: LGTM! Barcode field addition is well-structured.The barcode field is properly configured with appropriate constraints (255 character limit) and follows the same pattern as other text fields in the form.
Security Recommendation: Since barcodes will be stored and potentially displayed, ensure that:
- Input validation restricts barcode values to expected formats (numeric/alphanumeric patterns)
- The backend performs additional validation to prevent injection attacks
- Consider implementing barcode format validation (EAN-13, UPC-A, etc.) on both frontend and backend
docs/en/configure/index.md (1)
56-56: LGTM! Clear documentation for barcode API integration.The documentation clearly describes the new environment variable and includes a helpful reference link to the user guide.
Critical Security Recommendations:
- API Key Protection: Ensure this API key is never logged, cached, or exposed in client-side code
- Environment Security: Document best practices for securely storing this token (e.g., using secrets management, proper file permissions)
- Rate Limiting: Consider implementing rate limiting for barcode API calls to prevent abuse
- Fallback Handling: Ensure the application gracefully handles API failures/unavailability
- Key Rotation: Document procedures for API key rotation if compromised
Consider adding a security warning in the documentation about protecting API credentials.
frontend/components/App/CreateModal.vue (2)
13-13: LGTM! Proper conditional rendering implementation.The conditional rendering of the DialogFooter based on the
displayShortcutprop is correctly implemented.
54-63: LGTM! Clean props definition with proper defaults.The use of
withDefaultswith proper TypeScript typing and default values follows Vue 3 best practices.Security Note: While low-risk, ensure that the parent components passing this prop validate the boolean type to prevent potential type coercion issues that could affect UI behavior.
frontend/lib/api/__test__/user/items.test.ts (1)
43-45: LGTM! Consistent test coverage for new barcode fields.The addition of
barcode,manufacturer, andmodelNumberfields with empty string defaults across all item creation tests ensures proper test coverage. From a security perspective, using empty strings as defaults prevents potential undefined field vulnerabilities and maintains data consistency.Also applies to: 81-83, 139-141, 194-196, 222-224, 235-237, 255-257
frontend/components/ui/input/Input.vue (1)
9-10: Well-implemented input enhancements for barcode functionality.The addition of
inputmodeandpatternprops correctly supports numeric input validation for barcode entry. The TypeScript typing is appropriate and the props are properly bound to the input element.Security recommendation: Remember that client-side validation (including HTML5 pattern attribute) should never be relied upon for security. Always implement proper server-side validation for barcode input to prevent injection attacks and ensure data integrity. The pattern attribute is purely for user experience enhancement.
Also applies to: 33-34
frontend/assets/css/main.css (1)
969-977: Clean CSS implementation for barcode card selection states.The new selectable card styles properly use CSS custom properties and provide clear visual feedback for selected states. From a security perspective, these are standard CSS properties that don't introduce any attack vectors.
Security note: While CSS is generally safe, ensure that any content displayed within these selectable cards (especially barcode product data from external APIs) is properly sanitized to prevent XSS attacks.
frontend/components/global/DetailsSection/DetailsSection.vue (1)
15-25: LGTM! Clean integration of barcode detail type.The implementation follows the established pattern for detail types and properly integrates the Barcode component with consistent copyable functionality.
backend/internal/data/repo/repo_items_search_test.go (3)
7-7: LGTM! Improved import organization.The import reordering improves code organization and readability.
13-17: LGTM! Better struct field formatting.The consistent field alignment improves code readability and maintainability.
167-167: Excellent assertion improvement!Changing from
assert.NotEqual(t, "", normalizedSearch, ...)toassert.NotEmpty(t, normalizedSearch, ...)is more semantically correct and expressive for checking non-empty strings.Security Note: While these are test improvements, ensure that the search normalization logic properly sanitizes user input to prevent potential injection attacks or malformed queries from affecting the database.
frontend/components/ui/dialog/Dialog.vue (2)
4-6: LGTM! Proper typing for class attribute.The addition of the optional
classprop with proper HTMLAttributes typing enhances component flexibility while maintaining type safety.
16-22: Excellent prop forwarding refinement!The
delegatedPropscomputed property cleanly excludesclassanddialogIdfrom forwarding, ensuring only relevant props are passed touseForwardPropsEmits. This is a well-implemented separation of concerns.Security Note: When forwarding props in UI components, ensure that any user-provided class names or attributes are properly sanitized to prevent XSS attacks through malicious CSS or HTML injection.
frontend/components/global/DetailsSection/types.ts (2)
8-12: LGTM! Well-structured barcode type definition.The
Barcodetype properly extendsBaseDetailwith appropriate properties and follows the established pattern of other detail types.
42-42: Proper union type integration.The
AnyDetailunion type is correctly updated to include the newBarcodetype, maintaining type safety across the application.Security Note: When handling barcode text data throughout the application, ensure proper input validation and sanitization to prevent potential security issues from malicious barcode content, especially when displaying or processing this data in UI components.
frontend/components/Form/TextField.vue (1)
56-56: LGTM! Clean implementation of native HTML input attributes.The addition of
inputmodeandpatternprops follows Vue.js best practices with proper TypeScript typing and consistent application across both rendering modes.Security Note: While these changes are safe, ensure that any regex patterns used with the
patternattribute are validated server-side as well, since client-side validation can be bypassed.Also applies to: 85-94
frontend/pages/item/[id]/index.vue (3)
209-214: LGTM! Proper integration of barcode field.The barcode field is correctly added to the item details with appropriate properties - marked as copyable and with the specific "barcode" type for proper rendering.
Security Note: Ensure that barcode data is properly sanitized when copied to clipboard to prevent any potential XSS if barcodes contain unexpected characters.
490-492: LGTM! Consistent field inclusion in duplication.The barcode field is properly included in the item duplication logic alongside manufacturer and model number, maintaining data consistency.
777-777: LGTM! Explicit item type specification.Adding the explicit
item-typeprop improves component clarity and ensures proper handling of different data types.backend/internal/data/repo/repo_items.go (3)
67-69: LGTM! Comprehensive barcode field integration.The barcode field is properly integrated across all item data structures (
ItemCreate,ItemUpdate,ItemOut) with consistent JSON serialization and database mapping.Security Note: Ensure that barcode values are properly sanitized before database storage to prevent SQL injection, especially if barcode data originates from external APIs or user input.
Also applies to: 95-95, 159-159, 292-292
381-381: LGTM! Proper search integration.The barcode field is correctly added to the search functionality with case-insensitive matching, maintaining consistency with other searchable fields.
Security Consideration: The search uses
BarcodeContainsFoldwhich should be safe from SQL injection, but verify that the underlying ORM properly escapes the search parameters.
612-615: LGTM! Consistent database operations.The barcode field is properly handled in both create and update operations, ensuring data consistency across the application lifecycle.
Also applies to: 667-667
frontend/components/Item/BarcodeCard.vue (1)
58-94: LGTM: Well-structured component implementationThe component follows Vue best practices with proper TypeScript usage, clear prop definitions, and clean selection logic.
frontend/lib/api/types/data-contracts.ts (3)
198-199: LGTM: Consistent barcode field additionThe barcode field addition to EntItem interface follows the established pattern and maintains type consistency.
463-464: LGTM: Enhanced BarcodeProduct interfaceThe addition of search engine URL fields provides useful metadata for barcode product sources while maintaining backward compatibility.
507-523: LGTM: Complete ItemCreate interface updateAll necessary fields (barcode, manufacturer, modelNumber) are properly added to support the barcode product autofill feature.
frontend/components/Item/View/Table.types.ts (1)
1-25: LGTM: Well-designed multi-type supportThe type extensions elegantly support both ItemSummary and BarcodeProduct types using union types and discriminator patterns. The addition of selection capabilities and optional URL fields provides good flexibility for table functionality.
frontend/components/Item/View/Selectable.vue (2)
2-22: LGTM: Clean component architecture with type safetyThe use of
useForwardPropsEmitsand proper TypeScript typing provides excellent type safety and clean prop/emit forwarding. The component structure follows Vue best practices.
82-93: LGTM: Proper conditional rendering with type guardsThe conditional rendering correctly handles different item types with appropriate type assertions and maintains good separation of concerns between ItemCard and ItemBarcodeCard components.
frontend/components/Item/BarcodeModal.vue (3)
115-119: Good implementation of numeric validationThe regex validation
/^[0-9]+$/correctly restricts barcode input to numeric characters only, addressing one of the PR's key requirements. The error message is also properly internationalized.
8-14: Well-implemented error display for scanner issuesThe error message display with proper accessibility attributes (
role="alert") and visual indicators effectively addresses the PR requirement to display scanner errors, including camera initialization failures.
22-23: Excellent mobile optimizationThe addition of
inputmode="numeric"andpattern="[0-9]*"attributes provides an optimal user experience on smartphones by displaying the numeric keypad, directly addressing the PR's mobile UI improvement goals.frontend/components/Item/View/Table.vue (1)
321-333: Well-implemented row selection logicThe selection handling correctly toggles selection state and emits appropriate events. The guard clause for
selectionModeensures selection only works when intended.backend/internal/data/repo/repo_barcodes.go (1)
241-246: Good HTTPS validation for image URLsThe HTTPS-only validation for image URLs is a crucial security measure that prevents mixed content issues and potential MITM attacks.
| products, err := ctrl.repo.Barcode.RetrieveProductsFromBarcode(ctrl.config.Barcode, q.EAN) | ||
|
|
||
| if err != nil { | ||
| return server.JSON(w, http.StatusInternalServerError, nil) | ||
| } | ||
|
|
||
| if len(products) != 0 { | ||
| return server.JSON(w, http.StatusOK, products) | ||
| } | ||
|
|
||
| return server.JSON(w, http.StatusNoContent, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling granularity.
Currently, all repository errors return HTTP 500, which may not be appropriate for all error types (e.g., network timeouts, invalid API keys, malformed responses).
Security Concerns:
- Returning generic 500 errors could mask security issues like API key problems
- Consider logging specific errors for monitoring while returning appropriate HTTP status codes
- Validate that external API responses don't contain malicious content before returning
products, err := ctrl.repo.Barcode.RetrieveProductsFromBarcode(ctrl.config.Barcode, q.EAN)
if err != nil {
+ // Log the specific error for monitoring
+ ctrl.log.Error().Err(err).Str("ean", q.EAN).Msg("barcode product search failed")
+
+ // Return appropriate status codes based on error type
+ switch {
+ case isNetworkError(err):
+ return server.JSON(w, http.StatusServiceUnavailable, nil)
+ case isAuthError(err):
+ return server.JSON(w, http.StatusUnauthorized, nil)
+ default:
return server.JSON(w, http.StatusInternalServerError, nil)
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/app/api/handlers/v1/v1_ctrl_barcodes.go around lines 32 to 42,
improve error handling by distinguishing error types instead of always returning
HTTP 500. Inspect the error returned from
ctrl.repo.Barcode.RetrieveProductsFromBarcode to identify specific cases like
network timeouts, authentication failures, or malformed responses. Return
appropriate HTTP status codes such as 401 for invalid API keys or 400 for bad
requests, and log detailed error information internally for monitoring.
Additionally, validate and sanitize the products data from the external API
before returning it in the response to prevent potential security risks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the moment, I think returning Unauthorized or StatusServiceUnavailable does not reflect that the issue is an internal API call failure.
This might be improved in the future but I think it is enough for this version.
EDIT: +1 for the error logged into the console.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crumbowl That's a fair point! You're absolutely right that returning 401 Unauthorized or 503 Service Unavailable for external API call failures could be misleading to clients. From the Homebox application's perspective, these are indeed internal API call failures, and 500 Internal Server Error accurately represents that something went wrong on the server side during the barcode lookup process.
Your current approach provides a clear and consistent error response to clients while keeping the implementation straightforward for this version. The external API complexity is properly abstracted away from the client's perspective.
Security Note: The current error handling approach is also good from a security standpoint as it doesn't leak information about which specific external service failed or why, maintaining a clean separation of concerns.
✏️ Learnings added
Learnt from: crumbowl
PR: sysadminsmedia/homebox#920
File: backend/app/api/handlers/v1/v1_ctrl_barcodes.go:32-42
Timestamp: 2025-08-24T21:12:46.558Z
Learning: In the Homebox barcode API handler (backend/app/api/handlers/v1/v1_ctrl_barcodes.go), the user prefers to return HTTP 500 for all external barcode API call failures rather than distinguishing between different error types like network failures or authentication errors. Their reasoning is that from the application's perspective, these are internal API call failures, and returning codes like 401 or 503 would be misleading to clients about where the actual problem lies.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| func (r *BarcodeRepository) UPCItemDB_Search(_ config.BarcodeAPIConf, iBarcode string) ([]BarcodeProduct, error) { | ||
| client := &http.Client{Timeout: FOREIGN_API_CALL_TIMEOUT_SEC * time.Second} | ||
| resp, err := client.Get("https://api.upcitemdb.com/prod/trial/lookup?upc=" + iBarcode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL-encode barcode parameter to prevent injection
The barcode parameter should be URL-encoded before concatenation to prevent potential injection attacks.
func (r *BarcodeRepository) UPCItemDB_Search(_ config.BarcodeAPIConf, iBarcode string) ([]BarcodeProduct, error) {
client := &http.Client{Timeout: FOREIGN_API_CALL_TIMEOUT_SEC * time.Second}
- resp, err := client.Get("https://api.upcitemdb.com/prod/trial/lookup?upc=" + iBarcode)
+ resp, err := client.Get("https://api.upcitemdb.com/prod/trial/lookup?upc=" + url.QueryEscape(iBarcode))🤖 Prompt for AI Agents
In backend/internal/data/repo/repo_barcodes.go around lines 120 to 122, the
barcode parameter iBarcode is concatenated directly into the URL string, which
risks injection attacks. Fix this by URL-encoding iBarcode using an appropriate
function from the net/url package before appending it to the URL query string.
This ensures the barcode is safely included in the request URL.
| if err := json.Unmarshal(body, &result); err != nil { // Parse []byte to go struct pointer | ||
| log.Error().Msg("Can not unmarshal JSON") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return JSON unmarshal errors instead of just logging
JSON parsing failures should be returned as errors to properly handle malformed API responses.
var result UPCITEMDBResponse
if err := json.Unmarshal(body, &result); err != nil { // Parse []byte to go struct pointer
log.Error().Msg("Can not unmarshal JSON")
+ return nil, fmt.Errorf("failed to parse API response: %w", err)
}🤖 Prompt for AI Agents
In backend/internal/data/repo/repo_barcodes.go around lines 146 to 148, the code
currently logs JSON unmarshal errors but does not return them. Modify the code
to return the error from json.Unmarshal instead of only logging it, so that the
caller can handle malformed API responses properly. Ensure the function
signature supports returning an error and propagate the error upward
accordingly.
| <FormTextField | ||
| v-if="form.barcode?.length" | ||
| ref="barcodeInput" | ||
| v-model="form.barcode" | ||
| :trigger-focus="focused" | ||
| :autofocus="true" | ||
| label="Item barcode" | ||
| :max-length="255" | ||
| :min-length="1" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Implement barcode input validation and use i18n
Two issues:
- The barcode field lacks input validation. Per PR objectives, it should accept only numeric characters.
- Uses hardcoded label.
<FormTextField
v-if="form.barcode?.length"
ref="barcodeInput"
v-model="form.barcode"
:trigger-focus="focused"
:autofocus="true"
- label="Item barcode"
+ :label="$t('items.barcode')"
:max-length="255"
:min-length="1"
+ pattern="[0-9]*"
+ inputmode="numeric"
+ @input="validateBarcodeInput"
/>Add validation method:
function validateBarcodeInput(event: Event) {
const input = event.target as HTMLInputElement;
input.value = input.value.replace(/[^0-9]/g, '');
form.barcode = input.value;
}🤖 Prompt for AI Agents
In frontend/components/Item/CreateModal.vue around lines 72 to 81, the barcode
input field lacks validation to restrict input to numeric characters and uses a
hardcoded label. Fix this by adding an input validation method that filters out
non-numeric characters on input events and bind this method to the barcode input
field. Also, replace the hardcoded label with an i18n translation key to support
localization.
| v-if="d?.imageBase64" | ||
| :src="d.imageBase64" | ||
| class="w-16 rounded object-fill shadow-sm" | ||
| alt="Product's photo" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Security and accessibility improvements needed for image display
- Security: Validate that
imageBase64contains a proper data URI before rendering - Accessibility: Use a more descriptive alt text, e.g.,
${d.name} product image
- <img
- v-if="d?.imageBase64"
- :src="d.imageBase64"
- class="w-16 rounded object-fill shadow-sm"
- alt="Product's photo"
- />
+ <img
+ v-if="d?.imageBase64 && isValidDataUri(d.imageBase64)"
+ :src="d.imageBase64"
+ class="w-16 rounded object-fill shadow-sm"
+ :alt="`${extractValue(d, h.value)} product image`"
+ />Also add this validation function:
function isValidDataUri(uri: string): boolean {
return /^data:image\/(jpeg|png);base64,/.test(uri);
}🤖 Prompt for AI Agents
In frontend/components/Item/View/Table.vue around lines 99 to 103, the image
rendering lacks security validation and accessibility improvements. Add a
validation function isValidDataUri to check that d.imageBase64 is a proper data
URI before rendering the image. Also, update the alt attribute to use a more
descriptive text like `${d.name} product image` to improve accessibility.
| <NuxtLink class="underline" :to="extractValue(d, h.url)" target="_blank">{{ | ||
| extractValue(d, h.value) | ||
| }}</NuxtLink> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add security attributes to external links
External links with target="_blank" should include rel="noopener noreferrer" to prevent window.opener attacks.
- <NuxtLink class="underline" :to="extractValue(d, h.url)" target="_blank">{{
+ <NuxtLink class="underline" :to="extractValue(d, h.url)" target="_blank" rel="noopener noreferrer">{{🤖 Prompt for AI Agents
In frontend/components/Item/View/Table.vue around lines 126 to 128, the NuxtLink
component uses target="_blank" for external links but lacks the rel="noopener
noreferrer" attribute. To fix this, add rel="noopener noreferrer" to the
NuxtLink element to enhance security and prevent window.opener attacks when
opening links in a new tab.
| "fuzzysort": "^3.1.0", | ||
| "h3": "^1.15.1", | ||
| "http-proxy": "^1.18.1", | ||
| "jsbarcode": "^3.12.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify package security and consider version pinning.
The jsbarcode dependency addition looks good for the barcode rendering functionality. However, consider these security recommendations:
- Verify this version is free from known vulnerabilities
- Consider using exact version pinning instead of caret ranges for production dependencies
- Ensure the package comes from a trusted source
🏁 Script executed:
#!/bin/bash
# Check for security advisories and verify package authenticity
npm audit --audit-level moderate
npm view jsbarcode@3.12.1 --json | jq '.maintainers, .repository, .license'
# Check for any known vulnerabilities in this specific version
curl -s "https://registry.npmjs.org/jsbarcode" | jq '.versions["3.12.1"].deprecated // "No deprecation notice"'Length of output: 715
Confirm jsbarcode security & lock versions
Verified jsbarcode@3.12.1:
- Maintained by lindell (johan@lindell.me), MIT-licensed
- No deprecation notice or known vulnerabilities in npm registry
Recommendations:
- Pin the version exactly in frontend/package.json to prevent unintended minor/patch upgrades:
- "jsbarcode": "^3.12.1", + "jsbarcode": "3.12.1",
- Generate and commit a package-lock.json (or yarn.lock) so future
npm auditruns can detect new advisories and ensure reproducible installs.
📝 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.
| "jsbarcode": "^3.12.1", | |
| "jsbarcode": "3.12.1", |
🤖 Prompt for AI Agents
In frontend/package.json at line 67, the jsbarcode dependency version is
specified with a caret (^3.12.1), allowing minor and patch upgrades. Change this
to an exact version "3.12.1" to prevent unintended updates. Additionally,
generate and commit a package-lock.json or yarn.lock file to lock all
dependencies and enable consistent installs and effective vulnerability
auditing.
|
is this still in development? |
634139d to
7d8617b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/components/Item/View/Table.vue (4)
60-64: Fix reversed header content alignment mappingLeft/right mappings are flipped.
:class="{ 'justify-center': h.align === 'center', - 'justify-start': h.align === 'right', - 'justify-end': h.align === 'left', + 'justify-start': h.align === 'left', + 'justify-end': h.align === 'right', }"
135-139: Prevent broken overlay link for BarcodeProduct rows
d.iddoesn’t exist on BarcodeProduct. Only render the overlay for ItemSummary rows.- <TableCell v-if="!props.selectionMode" class="absolute inset-0"> + <TableCell v-if="!props.selectionMode && props.itemType === 'itemsummary'" class="absolute inset-0"> <NuxtLink :to="`/item/${d.id}`" class="absolute inset-0"> <span class="sr-only">{{ $t("components.item.view.table.view_item") }}</span> </NuxtLink> </TableCell>
344-361: Sorting fails for nested properties (e.g., 'item.name')
extractSortableindexes only top-level keys, so BarcodeProduct sorting byitem.*never works. Use the same path logic asextractValueand relax the types accordingly.- function extractSortable<T extends Record<string, any>>(item: T, property: keyof T): string | number | boolean { - const value = item[property]; + function extractSortable(item: TableData, property: TableProperties): string | number | boolean { + const value = extractValue(item, property as string); if (typeof value === "string") { // Try to parse number const parsed = Number(value); if (!isNaN(parsed)) { return parsed; } return value.toLowerCase(); } if (typeof value !== "number" && typeof value !== "boolean") { return ""; } return value; }
363-382: Simplify comparator typing to avoid TS mismatchWith nested paths,
keyof Tno longer applies. UseTableDatato avoid type errors and support both shapes.- function itemSort<T extends Record<string, any>>(a: T, b: T) { + function itemSort(a: TableData, b: TableData) { if (!sortByProperty.value) { return 0; } const aVal = extractSortable(a, sortByProperty.value); const bVal = extractSortable(b, sortByProperty.value); if (typeof aVal === "string" && typeof bVal === "string") { return aVal.localeCompare(bVal, undefined, { numeric: true, sensitivity: "base" }); } if (aVal < bVal) { return -1; } if (aVal > bVal) { return 1; } return 0; }backend/app/api/demo.go (1)
14-21: CSV column shift bug after adding HB.barcodeYou inserted HB.barcode in the header but didn’t add the corresponding empty field before HB.purchase_from in each row. “Amazon” is currently landing in HB.notes, and prices/dates are shifted, which will break imports.
Apply this diff to align data rows with the new header (add an extra empty field for HB.notes):
- ,Garage,IOT;Home Assistant; Z-Wave,1,Zooz Universal Relay ZEN17,"Zooz 700 Series Z-Wave Universal Relay ZEN17 for Awnings, Garage Doors, Sprinklers, and More | 2 NO-C-NC Relays (20A, 10A) | Signal Repeater | Hub Required (Compatible with SmartThings and Hubitat)",,,ZEN17,Zooz,,Amazon,39.95,10/13/2021,,,,,,, + ,Garage,IOT;Home Assistant; Z-Wave,1,Zooz Universal Relay ZEN17,"Zooz 700 Series Z-Wave Universal Relay ZEN17 for Awnings, Garage Doors, Sprinklers, and More | 2 NO-C-NC Relays (20A, 10A) | Signal Repeater | Hub Required (Compatible with SmartThings and Hubitat)",,,ZEN17,Zooz,,,Amazon,39.95,10/13/2021,,,,,,, - ,Living Room,IOT;Home Assistant; Z-Wave,1,Zooz Motion Sensor,"Zooz Z-Wave Plus S2 Motion Sensor ZSE18 with Magnetic Mount, Works with Vera and SmartThings",,,ZSE18,Zooz,,Amazon,29.95,10/15/2021,,,,,,, + ,Living Room,IOT;Home Assistant; Z-Wave,1,Zooz Motion Sensor,"Zooz Z-Wave Plus S2 Motion Sensor ZSE18 with Magnetic Mount, Works with Vera and SmartThings",,,ZSE18,Zooz,,,Amazon,29.95,10/15/2021,,,,,,, - ,Office,IOT;Home Assistant; Z-Wave,1,Zooz 110v Power Switch,"Zooz Z-Wave Plus Power Switch ZEN15 for 110V AC Units, Sump Pumps, Humidifiers, and More",,,ZEN15,Zooz,,Amazon,39.95,10/13/2021,,,,,,, + ,Office,IOT;Home Assistant; Z-Wave,1,Zooz 110v Power Switch,"Zooz Z-Wave Plus Power Switch ZEN15 for 110V AC Units, Sump Pumps, Humidifiers, and More",,,ZEN15,Zooz,,,Amazon,39.95,10/13/2021,,,,,,, - ,Downstairs,IOT;Home Assistant; Z-Wave,1,Ecolink Z-Wave PIR Motion Sensor,"Ecolink Z-Wave PIR Motion Detector Pet Immune, White (PIRZWAVE2.5-ECO)",,,PIRZWAVE2.5-ECO,Ecolink,,Amazon,35.58,10/21/2020,,,,,,, + ,Downstairs,IOT;Home Assistant; Z-Wave,1,Ecolink Z-Wave PIR Motion Sensor,"Ecolink Z-Wave PIR Motion Detector Pet Immune, White (PIRZWAVE2.5-ECO)",,,PIRZWAVE2.5-ECO,Ecolink,,,Amazon,35.58,10/21/2020,,,,,,, - ,Entry,IOT;Home Assistant; Z-Wave,1,Yale Security Touchscreen Deadbolt,"Yale Security YRD226-ZW2-619 YRD226ZW2619 Touchscreen Deadbolt, Satin Nickel",,,YRD226ZW2619,Yale,,Amazon,120.39,10/14/2020,,,,,,, + ,Entry,IOT;Home Assistant; Z-Wave,1,Yale Security Touchscreen Deadbolt,"Yale Security YRD226-ZW2-619 YRD226ZW2619 Touchscreen Deadbolt, Satin Nickel",,,YRD226ZW2619,Yale,,,Amazon,120.39,10/14/2020,,,,,,, - ,Kitchen,IOT;Home Assistant; Z-Wave,1,Smart Rocker Light Dimmer,"UltraPro Z-Wave Smart Rocker Light Dimmer with QuickFit and SimpleWire, 3-Way Ready, Compatible with Alexa, Google Assistant, ZWave Hub Required, Repeater/Range Extender, White Paddle Only, 39351",,,39351,Honeywell,,Amazon,65.98,09/30/0202,,,,,,, + ,Kitchen,IOT;Home Assistant; Z-Wave,1,Smart Rocker Light Dimmer,"UltraPro Z-Wave Smart Rocker Light Dimmer with QuickFit and SimpleWire, 3-Way Ready, Compatible with Alexa, Google Assistant, ZWave Hub Required, Repeater/Range Extender, White Paddle Only, 39351",,,39351,Honeywell,,,Amazon,65.98,09/30/0202,,,,,,,Security: Avoid importing malformed demo data into user DBs; this can cause confusing state or failed imports.
🧹 Nitpick comments (16)
frontend/composables/use-preferences.ts (1)
21-23: Default missing for new barcode header preferencesWithout adding
tableHeadersBarcodeto the defaults, existing users’ stored prefs will keep this field undefined, which can break reactivity when components expect an array. Please seed it in theuseLocalStoragedefaults.itemsPerTablePage: 10, + tableHeadersBarcode: [], displayLegacyHeader: false,Security recommendation: ensure barcode header labels are sanitized before persisting to localStorage to prevent downstream injection risks.
docs/en/user-guide/tips-tricks.md (1)
24-29: Document the barcode API token as a secretSince this token enables authenticated lookups against an external service, the docs should warn operators to treat it as sensitive (e.g., keep it server-side, store via secrets manager, avoid embedding in client bundles). Adding that callout here helps prevent accidental leakage.
frontend/components/Form/TextField.vue (1)
22-23: LGTM: Forwarding inputmode/patternPassing through inputmode and pattern to the underlying input is correct and matches the new Input.vue props.
Consider adding
ref="input"to the inline branch too sotriggerFocusworks consistently in both paths.Also applies to: 48-49
frontend/components/Item/View/Selectable.vue (1)
82-93: Prefer stable keys for barcode cardsUsing
indexas key can cause selection drift if the list changes. If available, use a stable unique field (e.g.,item.barcode).Example:
<ItemBarcodeCard v-for="it in items" :key="(it as BarcodeProduct).barcode" :item="it as BarcodeProduct" :model-value="selectedCard === items.indexOf(it)" @update:model-value="val => (selectedCard = val ? items.indexOf(it) : -1)" />frontend/components/Item/View/Table.types.ts (1)
7-8: Type url as a valid table propertyTighten
urlto useTablePropertiesso only valid paths can be referenced.export type TableHeaderType = { text: string; - value: TableProperties; - url?: string; + value: TableProperties; + url?: TableProperties; sortable?: boolean; align?: "left" | "center" | "right"; enabled: boolean; type?: "price" | "boolean" | "name" | "location" | "date"; };frontend/components/Item/View/Table.vue (1)
97-107: Alt text should be localized; optional data-URI validation
- Hardcoded alt breaks i18n policy. Use a translatable string with a name placeholder.
- Optional: Validate data URI if you want a belt-and-braces check (backend already enforces MIME/size; optional). Based on learnings
- <img - v-if="d?.imageBase64" - :src="d.imageBase64" - class="w-16 rounded object-fill shadow-sm" - alt="Product's photo" - /> + <img + v-if="d?.imageBase64" + :src="d.imageBase64" + class="w-16 rounded object-fill shadow-sm" + :alt="$t('components.item.view.table.product_image_alt', { name: extractValue(d, h.value) })" + />If you opt-in to validate:
function isValidDataUri(uri: string): boolean { return /^data:image\/(jpeg|png);base64,/.test(uri); }and add
&& isValidDataUri(d.imageBase64)to thev-if. Based on learningsfrontend/components/Item/BarcodeModal.vue (2)
26-39: Button label says “Cancel” but no cancellation is implementedAvoid misleading UX. Either implement request cancellation or disable the button and show a “Searching…” label while in-flight.
Minimal UX fix:
- <Button - :variant="searching ? 'destructive' : 'default'" - class="mt-auto h-10" - @click="retrieveProductInfo(barcode)" - > - <MdiLoading v-if="searching" class="animate-spin" /> + <Button + :disabled="searching" + class="mt-auto h-10" + @click="retrieveProductInfo(barcode)" + > + <MdiLoading v-if="searching" class="animate-spin mr-2" /> <div v-if="!searching" class="relative mx-2"> <div class="absolute inset-0 flex items-center justify-center"> <MdiBarcode class="size-5 group-hover:hidden" /> </div> </div> - {{ searching ? $t("global.cancel") : $t("components.item.product_import.search_item") }} + {{ searching ? $t("global.searching") : $t("components.item.product_import.search_item") }} </Button>If you prefer true cancel, I can wire an AbortController into
useUserApi().products.searchFromBarcode.
51-53: Localize “Import selected”UI strings must be translatable.
- <Button type="import" :disabled="selectedItem === null" @click="createItem"> Import selected </Button> + <Button type="import" :disabled="selectedItem === null" @click="createItem"> + {{ $t("components.item.product_import.import_selected") }} + </Button>backend/internal/data/repo/repo_items_search_test.go (1)
163-178: Minimal assertionUsing NotEmpty is fine, but consider a negative case to ensure normalization actually changes matching behavior (optional).
backend/app/api/handlers/v1/v1_ctrl_barcodes.go (1)
11-19: Fix swagger annotations (name/required) and comment headerAlign swagger with actual query schema and handler purpose.
Apply this diff:
-// HandleGenerateQRCode godoc +// HandleProductSearchFromBarcode godoc // -// @Summary Search EAN from Barcode +// @Summary Search products by barcode (EAN/UPC/GTIN) // @Tags Items // @Produce json -// @Param data query string false "barcode to be searched" +// @Param productEAN query string true "barcode to be searched" // @Success 200 {object} []repo.BarcodeProduct // @Router /v1/products/search-from-barcode [GET] // @Security BearerNote: Verify the doc generator resolves repo.BarcodeProduct without explicit import in this file.
backend/internal/data/repo/repo_items.go (4)
76-79: Add validation for Barcode in DTOsAdd length and digits-only validation (and trim) to ItemCreate/ItemUpdate/ItemOut. Keeps API consistent with “numeric-only” policy.
Security: Input validation prevents polluted data reaching persistence and external APIs.
Also applies to: 100-105, 165-169
613-625: Trim/sanitize barcode before persisting (Create)Avoid storing leading/trailing spaces.
Apply this diff:
- SetLocationID(data.LocationID). - SetAssetID(int(data.AssetID)). - SetManufacturer(data.Manufacturer). - SetModelNumber(data.ModelNumber). - SetBarcode(data.Barcode) + SetLocationID(data.LocationID). + SetAssetID(int(data.AssetID)). + SetManufacturer(data.Manufacturer). + SetModelNumber(data.ModelNumber). + SetBarcode(strings.TrimSpace(data.Barcode))Add this import:
import "strings"Security: Reduces risk of mismatches when querying by barcode.
676-677: Trim/sanitize barcode on updateMirror Create behavior on Update.
Apply this diff:
- SetManufacturer(data.Manufacturer). - SetBarcode(data.Barcode). + SetManufacturer(data.Manufacturer). + SetBarcode(strings.TrimSpace(data.Barcode)).
361-399: Consider indexing items.barcode for searchYou now search on Barcode; add a DB index on items.barcode to keep queries fast at scale.
Security: Indexing also helps mitigate accidental DoS from heavy scans.
backend/internal/data/repo/repo_barcodes_test.go (2)
61-66: Close mock servers to avoid leaksRegister cleanup to close httptest servers.
Apply this diff:
p.mock_server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { handler(w, r, p.products) })) + t.Cleanup(func() { + p.mock_server.Close() + })
100-129: Strengthen assertions and ensure empty.json fixtures exist
- Add minimal assertions (non-nil/shape) so failures are visible.
- Verify testdata contains empty.json in both folders or handlers should return a valid empty payload when not found.
Apply this diff:
for _, bc := range gtins { products, err := api.barcode_handler(config.BarcodeAPIConf{}, bc, api.mock_server.URL) checkError(t, err) - for _, p := range products { - log.Info().Msg("Found: " + p.Item.Name) - } + // Minimal assertions + if len(products) > 0 { + if products[0].Item.Name == "" { + t.Errorf("%s: expected product name for barcode %s", api.api_name, bc) + } + } }Security: Tests should not hit real services or leak tokens (you’re good here).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
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/item.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item/item.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/item/where.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_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/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/item.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/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
backend/app/api/demo.go(1 hunks)backend/app/api/handlers/v1/v1_ctrl_barcodes.go(1 hunks)backend/app/api/handlers/v1/v1_ctrl_product_search.go(0 hunks)backend/app/api/routes.go(1 hunks)backend/internal/core/services/reporting/io_row.go(1 hunks)backend/internal/data/migrations/sqlite3/20250723163847_add_barcode.sql(1 hunks)backend/internal/data/repo/repo_barcodes.go(1 hunks)backend/internal/data/repo/repo_barcodes_test.go(1 hunks)backend/internal/data/repo/repo_items.go(8 hunks)backend/internal/data/repo/repo_items_search_test.go(2 hunks)backend/internal/data/repo/repo_product_search.go(0 hunks)backend/internal/data/repo/repos_all.go(2 hunks)backend/internal/data/repo/testdata/barcodespider/855800001203.json(1 hunks)backend/internal/data/repo/testdata/barcodespider/empty.json(1 hunks)backend/internal/data/repo/testdata/upcitemdb/855800001869.json(1 hunks)backend/internal/data/repo/testdata/upcitemdb/885911209809.json(1 hunks)backend/internal/data/repo/testdata/upcitemdb/empty.json(1 hunks)backend/pkgs/textutils/normalize.go(1 hunks)docs/en/configure/index.md(1 hunks)docs/en/user-guide/tips-tricks.md(1 hunks)frontend/assets/css/main.css(1 hunks)frontend/components/App/CreateModal.vue(2 hunks)frontend/components/App/ScannerModal.vue(1 hunks)frontend/components/Form/TextField.vue(3 hunks)frontend/components/Item/BarcodeCard.vue(1 hunks)frontend/components/Item/BarcodeModal.vue(4 hunks)frontend/components/Item/CreateModal.vue(5 hunks)frontend/components/Item/View/Selectable.vue(3 hunks)frontend/components/Item/View/Table.types.ts(1 hunks)frontend/components/Item/View/Table.vue(10 hunks)frontend/components/global/Barcode.vue(1 hunks)frontend/components/global/DetailsSection/DetailsSection.vue(1 hunks)frontend/components/global/DetailsSection/types.ts(2 hunks)frontend/components/ui/dialog/Dialog.vue(2 hunks)frontend/components/ui/input/Input.vue(2 hunks)frontend/composables/use-preferences.ts(1 hunks)frontend/lib/api/__test__/user/items.test.ts(7 hunks)frontend/lib/api/types/data-contracts.ts(5 hunks)frontend/locales/en.json(1 hunks)frontend/package.json(1 hunks)frontend/pages/home/index.vue(1 hunks)frontend/pages/item/[id]/index.vue(2 hunks)frontend/pages/item/[id]/index/edit.vue(1 hunks)frontend/pages/label/[id].vue(1 hunks)frontend/pages/location/[id].vue(1 hunks)
💤 Files with no reviewable changes (2)
- backend/app/api/handlers/v1/v1_ctrl_product_search.go
- backend/internal/data/repo/repo_product_search.go
✅ Files skipped from review due to trivial changes (4)
- backend/internal/data/repo/testdata/upcitemdb/885911209809.json
- backend/internal/data/repo/testdata/upcitemdb/empty.json
- backend/internal/data/repo/testdata/barcodespider/empty.json
- frontend/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (17)
- docs/en/configure/index.md
- frontend/assets/css/main.css
- frontend/lib/api/types/data-contracts.ts
- backend/app/api/routes.go
- backend/pkgs/textutils/normalize.go
- frontend/lib/api/test/user/items.test.ts
- frontend/components/global/Barcode.vue
- frontend/pages/location/[id].vue
- frontend/components/Item/CreateModal.vue
- frontend/pages/home/index.vue
- frontend/components/global/DetailsSection/DetailsSection.vue
- frontend/pages/label/[id].vue
- frontend/components/global/DetailsSection/types.ts
- backend/internal/data/repo/repos_all.go
- frontend/components/App/CreateModal.vue
- backend/internal/data/migrations/sqlite3/20250723163847_add_barcode.sql
- frontend/package.json
🧰 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/Item/View/Table.types.tsfrontend/components/Item/BarcodeCard.vuefrontend/components/ui/dialog/Dialog.vuefrontend/components/App/ScannerModal.vuefrontend/composables/use-preferences.tsfrontend/pages/item/[id]/index.vuefrontend/pages/item/[id]/index/edit.vuefrontend/components/ui/input/Input.vuefrontend/components/Form/TextField.vuefrontend/components/Item/View/Table.vuefrontend/components/Item/View/Selectable.vuefrontend/components/Item/BarcodeModal.vue
🧠 Learnings (4)
📚 Learning: 2025-07-07T19:05:42.824Z
Learnt from: crumbowl
PR: sysadminsmedia/homebox#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/BarcodeCard.vuefrontend/pages/item/[id]/index.vuefrontend/pages/item/[id]/index/edit.vuefrontend/components/Item/View/Table.vuefrontend/components/Item/BarcodeModal.vue
📚 Learning: 2025-07-07T19:07:38.949Z
Learnt from: crumbowl
PR: sysadminsmedia/homebox#818
File: frontend/components/Item/BarcodeModal.vue:0-0
Timestamp: 2025-07-07T19:07:38.949Z
Learning: In the barcode product import feature for frontend/components/Item/BarcodeModal.vue, user crumbowl confirmed that comprehensive image validation is performed in the backend (HandleProductSearchFromBarcode) including HTTPS enforcement, MIME type detection, format whitelisting (JPEG/PNG only), size limits, and proper data URI construction, making frontend sanitization unnecessary.
Applied to files:
frontend/components/Item/BarcodeCard.vuefrontend/components/Item/View/Table.vuefrontend/components/Item/BarcodeModal.vue
📚 Learning: 2025-09-23T17:43:10.715Z
Learnt from: tonyaellie
PR: sysadminsmedia/homebox#998
File: frontend/components/ModalConfirm.vue:6-13
Timestamp: 2025-09-23T17:43:10.715Z
Learning: In frontend/components/ModalConfirm.vue, the team considers direct href rendering acceptable since URLs are developer-controlled rather than user input. They've assessed the XSS risk as non-issue given their threat model.
Applied to files:
frontend/components/Item/BarcodeCard.vuefrontend/components/Item/View/Table.vue
📚 Learning: 2025-08-24T21:12:46.584Z
Learnt from: crumbowl
PR: sysadminsmedia/homebox#920
File: backend/app/api/handlers/v1/v1_ctrl_barcodes.go:32-42
Timestamp: 2025-08-24T21:12:46.584Z
Learning: In the Homebox barcode API handler (backend/app/api/handlers/v1/v1_ctrl_barcodes.go), the user prefers to return HTTP 500 for all external barcode API call failures rather than distinguishing between different error types like network failures or authentication errors. Their reasoning is that from the application's perspective, these are internal API call failures, and returning codes like 401 or 503 would be misleading to clients about where the actual problem lies.
Applied to files:
backend/app/api/handlers/v1/v1_ctrl_barcodes.go
🧬 Code graph analysis (6)
frontend/components/Item/View/Table.types.ts (1)
frontend/lib/api/types/data-contracts.ts (3)
ItemSummary(586-605)BarcodeProduct(456-465)ItemCreate(507-523)
frontend/composables/use-preferences.ts (1)
frontend/components/Item/View/Table.types.ts (1)
TableHeaderType(5-13)
backend/internal/data/repo/repo_barcodes.go (2)
frontend/lib/api/types/data-contracts.ts (2)
ItemCreate(507-523)BarcodeProduct(456-465)backend/internal/sys/config/conf.go (1)
BarcodeAPIConf(76-78)
backend/internal/data/repo/repo_items.go (1)
backend/internal/data/repo/asset_id_type.go (1)
AssetID(9-9)
backend/app/api/handlers/v1/v1_ctrl_barcodes.go (2)
backend/app/api/handlers/v1/controller.go (1)
V1Controller(67-77)backend/internal/web/adapters/decoders.go (1)
DecodeQuery(17-30)
backend/internal/data/repo/repo_barcodes_test.go (2)
backend/internal/data/repo/repo_barcodes.go (1)
ProductDatabaseFunc(36-36)backend/internal/sys/config/conf.go (1)
BarcodeAPIConf(76-78)
🔇 Additional comments (25)
frontend/components/App/ScannerModal.vue (1)
79-82: Do not surface raw error objects to end usersConcatenating the raw
errorintoerrorMessagecan leak stack traces or device details through the UI. Log the full error for diagnostics and only show a sanitized, user-safe message.const handleError = (error: unknown) => { console.error("Scanner error:", error); - errorMessage.value = t("scanner.error") + ": " + error; + const userMessage = error instanceof Error ? error.message : String(error ?? ""); + errorMessage.value = userMessage ? `${t("scanner.error")}: ${userMessage}` : t("scanner.error"); };Security recommendation: ensure user-facing alerts never expose raw error objects so sensitive data stays internal.
frontend/components/Form/TextField.vue (1)
85-94: Prop typings look good once PropType is importedNo further changes needed here beyond importing
PropTypeas noted.frontend/components/ui/input/Input.vue (1)
9-11: LGTM: New props and bindings are correct
inputmodeandpatternare appropriately typed and bound to the native input.Also applies to: 33-34
frontend/components/Item/BarcodeCard.vue (2)
4-10: Security & a11y: External image rendering; add meaningful alt textA previous review already flagged sanitization for base64 image sources; given the team’s accepted risk and backend validation, at least set a descriptive alt for accessibility.
- alt="" + :alt="item.item.name || item.item.modelNumber || 'product image'"Based on learnings
51-51: Security: Markdown from external sourcesPrior review noted XSS risk when rendering external markdown without sanitization. If the threat model accepts it, acknowledge and proceed; otherwise sanitize or show plain text.
Based on learnings
frontend/pages/item/[id]/index.vue (1)
783-784: LGTM: Explicit item-type wiringPassing item-type as 'itemsummary' clarifies rendering and matches Selectable expectations.
frontend/components/ui/dialog/Dialog.vue (2)
4-4: LGTM: Narrowed prop forwardingExcluding
classanddialogIdfrom forwarded props avoids leaking internal control props to DialogRoot. Good use of computed wrapper.
16-23: LGTM: Delegated forwarding via useForwardPropsEmitsThis keeps reactivity and prevents prop pollution. No issues spotted.
frontend/components/Item/View/Selectable.vue (2)
77-79: LGTM: Prop/emit forwarding to tableForwarding via
useForwardPropsEmitssimplifies maintenance and keeps API parity.
33-39: LGTM: Selection propagation
Emittingupdate:selectedItemon selection changes is correct and is properly declared in TableEmits. Validate and sanitize item data before rendering to prevent XSS.frontend/components/Item/View/Table.types.ts (2)
15-17: Event name casing consistency across componentsYou emit
update:selectedItemhere. Downstream templates often listen with kebab-case (@update:selected-item). Confirm that wrappers (e.g., ItemViewSelectable) normalize/forward consistently; otherwise selection won’t propagate.If needed, we can expose both variants in typing:
export type TableEmits = { (event: "update:selectedItem", value: ItemSummary | BarcodeProduct | null): void; (event: "update:selected-item", value: ItemSummary | BarcodeProduct | null): void; };
3-3: LGTM on the unified TablePropertiesCombining keys across ItemSummary, BarcodeProduct, and nested
item.*gives the table a clean, single sorting/rendering surface.frontend/components/Item/View/Table.vue (2)
80-86: Row selection UX addition looks goodClick-to-select with highlight and emit is clean and well-guarded by selectionMode.
125-128: Add rel="noopener noreferrer" to external linksSecurity hardening for target="_blank" to prevent window.opener attacks.
[security]
- <NuxtLink class="underline" :to="extractValue(d, h.url)" target="_blank">{{ + <NuxtLink class="underline" :to="extractValue(d, h.url)" target="_blank" rel="noopener noreferrer">{{ extractValue(d, h.value) }}</NuxtLink>frontend/components/Item/BarcodeModal.vue (3)
43-49: Verify event name matches emitterYou listen to
@update:selected-item. Confirm thatItemViewSelectableemits kebab-case, or selection won’t propagate.
61-62: Verify BaseModal import path
BaseModalimported fromApp/CreateModal.vuelooks suspicious. Ensure this component actually renders a generic modal for ProductImport (and not the create-item modal).
16-25: Barcode input validation looks goodGood use of
inputmode="numeric"andpattern="[0-9]*"to enforce numeric input.backend/internal/core/services/reporting/io_row.go (1)
36-36: CSV export: Barcode field addition looks goodStruct tag matches HB.barcode. Ensure importer maps by header name, not position, to avoid order issues.
backend/internal/data/repo/testdata/upcitemdb/855800001869.json (1)
1-1: Fixture OKSample looks valid and safe for tests. No secrets included.
backend/internal/data/repo/testdata/barcodespider/855800001203.json (1)
1-1: Fixture OKWell-formed JSON aligned with expected shape. No secrets included.
backend/internal/data/repo/repo_items_search_test.go (1)
7-8: Import organization change is fineNo functional impact.
backend/internal/data/repo/repo_items.go (1)
1066-1071: Duplicating barcode: confirm uniqueness expectationsIf barcode is intended unique per item, duplication could violate constraints. If not unique, fine. Please confirm.
backend/internal/data/repo/repo_barcodes_test.go (1)
76-79: Not-found case depends on empty.json fixtureHandlers fall back to productMap["empty.json"]; ensure this file exists in both testdata folders or return a valid empty JSON response inline when missing.
Also applies to: 92-95
backend/internal/data/repo/repo_barcodes.go (2)
129-130: URL-encode the UPC query parameter before calling UPCItemDB
iBarcodecomes straight from the request, so concatenating it directly into the UPCItemDB URL lets callers inject extra query parameters or reserved characters. Encode it before issuing the request.- resp, err := client.Get(apiUrl + "/prod/trial/lookup?upc=" + iBarcode) + encodedBarcode := url.QueryEscape(iBarcode) + resp, err := client.Get(apiUrl + "/prod/trial/lookup?upc=" + encodedBarcode)
153-155: Propagate JSON unmarshal errors to the callerIf UPCItemDB returns malformed JSON, we silently swallow the error and fall through with empty results. That hides the real failure mode from callers, making retries or diagnostics impossible.
- if err := json.Unmarshal(body, &result); err != nil { // Parse []byte to go struct pointer - log.Error().Msg("Can not unmarshal JSON") - } + if err := json.Unmarshal(body, &result); err != nil { // Parse []byte to go struct pointer + log.Error().Msg("Can not unmarshal JSON") + return nil, fmt.Errorf("failed to decode UPCItemDB payload: %w", err) + }
| type query struct { | ||
| // 80 characters is the longest non-2D barcode length (GS1-128) | ||
| EAN string `schema:"productEAN" validate:"required,max=80"` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce numeric-only barcode server-side
Frontend restriction is good; add backend validation to reject non-digits to prevent unnecessary external calls.
Apply this diff:
type query struct {
// 80 characters is the longest non-2D barcode length (GS1-128)
EAN string `schema:"productEAN" validate:"required,max=80"`
}
return func(w http.ResponseWriter, r *http.Request) error {
q, err := adapters.DecodeQuery[query](r)
if err != nil {
return err
}
+ // Backend guard: digits only
+ for _, ch := range q.EAN {
+ if ch < '0' || ch > '9' {
+ return server.JSON(w, http.StatusBadRequest, "invalid barcode")
+ }
+ }Security: Input validation should be enforced server-side as well.
Also applies to: 26-31
🤖 Prompt for AI Agents
In backend/app/api/handlers/v1/v1_ctrl_barcodes.go around lines 21-24 (and
similarly lines 26-31), the barcode field currently only checks required and max
length; add a server-side numeric-only validation so non-digit EANs are rejected
before external calls. Update the query struct validation to include a
digits-only rule (e.g. a regex like ^\d+$) or wire in the validator's
numeric/digits validator, adjust any parsing logic to rely on that validation,
and ensure the handler returns a 400 Bad Request with a clear validation error
when the EAN contains non-digits.
| products, err := ctrl.repo.Barcode.RetrieveProductsFromBarcode(ctrl.config.Barcode, q.EAN) | ||
|
|
||
| if err != nil { | ||
| return server.JSON(w, http.StatusInternalServerError, err.Error()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t leak internal errors; log and return generic 500
Return a generic 500 body and log the detailed error instead of sending err.Error() to clients.
Apply these diffs:
@@
-import (
+import (
"net/http"
"github.com/hay-kot/httpkit/errchain"
"github.com/hay-kot/httpkit/server"
"github.com/sysadminsmedia/homebox/backend/internal/web/adapters"
+ "github.com/rs/zerolog/log"
)
@@
- if err != nil {
- return server.JSON(w, http.StatusInternalServerError, err.Error())
- }
+ if err != nil {
+ log.Error().Err(err).Str("ean", q.EAN).Msg("barcode product search failed")
+ return server.JSON(w, http.StatusInternalServerError, nil)
+ }Security: Avoid exposing upstream provider errors or config hints in responses. Based on learnings.
📝 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.
| products, err := ctrl.repo.Barcode.RetrieveProductsFromBarcode(ctrl.config.Barcode, q.EAN) | |
| if err != nil { | |
| return server.JSON(w, http.StatusInternalServerError, err.Error()) | |
| } | |
| import ( | |
| "net/http" | |
| "github.com/hay-kot/httpkit/errchain" | |
| "github.com/hay-kot/httpkit/server" | |
| "github.com/sysadminsmedia/homebox/backend/internal/web/adapters" | |
| "github.com/rs/zerolog/log" | |
| ) | |
| // ... | |
| products, err := ctrl.repo.Barcode.RetrieveProductsFromBarcode(ctrl.config.Barcode, q.EAN) | |
| if err != nil { | |
| log.Error().Err(err).Str("ean", q.EAN).Msg("barcode product search failed") | |
| return server.JSON(w, http.StatusInternalServerError, nil) | |
| } |
🤖 Prompt for AI Agents
In backend/app/api/handlers/v1/v1_ctrl_barcodes.go around lines 32 to 36, the
handler currently returns err.Error() to the client exposing internal details;
instead, log the detailed error server-side and return a generic 500 response
body. Update the error branch to send a non-revealing message (e.g. "internal
server error") to the client and use the controller's logger (or standard
logger) to record the full err with context (including barcode and any other
useful request identifiers) before returning the generic response.
| res, err := client.Get(p.ImageURL) | ||
| if err != nil { | ||
| log.Warn().Msg("Cannot fetch image for URL: " + p.ImageURL + ": " + err.Error()) | ||
| } | ||
|
|
||
| defer func() { | ||
| err = errors.Join(err, res.Body.Close()) | ||
| }() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return immediately when the image fetch fails
When client.Get fails, res is nil. We log the warning but still defer res.Body.Close() and later dereference res, which will panic at runtime. Bail out as soon as the fetch fails.
- res, err := client.Get(p.ImageURL)
- if err != nil {
- log.Warn().Msg("Cannot fetch image for URL: " + p.ImageURL + ": " + err.Error())
- }
-
- defer func() {
- err = errors.Join(err, res.Body.Close())
- }()
+ res, err := client.Get(p.ImageURL)
+ if err != nil {
+ log.Warn().Msg("Cannot fetch image for URL: " + p.ImageURL + ": " + err.Error())
+ return
+ }
+
+ defer func() {
+ err = errors.Join(err, res.Body.Close())
+ }()📝 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.
| res, err := client.Get(p.ImageURL) | |
| if err != nil { | |
| log.Warn().Msg("Cannot fetch image for URL: " + p.ImageURL + ": " + err.Error()) | |
| } | |
| defer func() { | |
| err = errors.Join(err, res.Body.Close()) | |
| }() | |
| res, err := client.Get(p.ImageURL) | |
| if err != nil { | |
| log.Warn().Msg("Cannot fetch image for URL: " + p.ImageURL + ": " + err.Error()) | |
| return | |
| } | |
| defer func() { | |
| err = errors.Join(err, res.Body.Close()) | |
| }() |
🤖 Prompt for AI Agents
In backend/internal/data/repo/repo_barcodes.go around lines 272-279, the code
defers res.Body.Close() even when client.Get returned an error and res is nil;
this will cause a panic. Change the flow to return (or handle) immediately when
err != nil after client.Get — log the warning and then return the error (or skip
processing) so res is never used; only set up the deferred res.Body.Close()
after confirming err == nil and res != nil, and preserve error joining logic by
joining any close error into the existing err variable when deferring.
| </template> | ||
|
|
||
| <script lang="ts" setup> | ||
| import type { HTMLAttributes } from "vue"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix: Missing PropType import breaks TS compile
PropType is used but not imported. This will fail type-checking.
Apply this diff:
- import type { HTMLAttributes } from "vue";
+ import type { HTMLAttributes, PropType } from "vue";📝 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.
| import type { HTMLAttributes } from "vue"; | |
| import type { HTMLAttributes, PropType } from "vue"; |
🤖 Prompt for AI Agents
In frontend/components/Form/TextField.vue around line 56, the code uses the
PropType type but the import statement only includes HTMLAttributes; add
PropType to the named imports from "vue" (i.e., import type { HTMLAttributes,
PropType } from "vue") so TypeScript can resolve the PropType usage and the file
compiles.
| @@ -0,0 +1,94 @@ | |||
| <template> | |||
| <Card class="selectable-card cursor-pointer overflow-hidden" :class="{ selected }" @click="toggleSelect"> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessibility: Make the card keyboard-accessible
Clickable card lacks keyboard support and ARIA state. Add role, tabindex, key handlers, and aria-pressed.
- <Card class="selectable-card cursor-pointer overflow-hidden" :class="{ selected }" @click="toggleSelect">
+ <Card
+ class="selectable-card cursor-pointer overflow-hidden"
+ :class="{ selected }"
+ role="button"
+ tabindex="0"
+ :aria-pressed="selected"
+ @click="toggleSelect"
+ @keydown.enter.prevent="toggleSelect"
+ @keydown.space.prevent="toggleSelect"
+ >📝 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.
| <Card class="selectable-card cursor-pointer overflow-hidden" :class="{ selected }" @click="toggleSelect"> | |
| <Card | |
| class="selectable-card cursor-pointer overflow-hidden" | |
| :class="{ selected }" | |
| role="button" | |
| tabindex="0" | |
| :aria-pressed="selected" | |
| @click="toggleSelect" | |
| @keydown.enter.prevent="toggleSelect" | |
| @keydown.space.prevent="toggleSelect" | |
| > |
🤖 Prompt for AI Agents
In frontend/components/Item/BarcodeCard.vue around line 2, the clickable Card
lacks keyboard accessibility and ARIA state; update the template to add
role="button", tabindex="0", and bind aria-pressed to the selected prop
(aria-pressed="selected"), and attach key handlers that invoke the same
toggleSelect method for Enter and Space (handle Space with preventDefault to
avoid page scroll). Ensure the corresponding methods exist (or add a small
keyDown handler that calls toggleSelect) and keep the existing @click behavior;
also confirm focus-visible/focus styles are present so keyboard users can see
focus.
| locationFlatTree: { | ||
| type: Array as () => FlatTreeItem[], | ||
| required: false, | ||
| default: () => [], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused prop or import missing type
locationFlatTree references FlatTreeItem but the type isn’t imported, and the prop isn’t used. This will fail type-checking and adds dead API surface.
Apply this diff to remove the unused prop:
- locationFlatTree: {
- type: Array as () => FlatTreeItem[],
- required: false,
- default: () => [],
- },If you actually need it, import the type and use it; otherwise, removing is cleaner.
📝 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.
| locationFlatTree: { | |
| type: Array as () => FlatTreeItem[], | |
| required: false, | |
| default: () => [], | |
| }, |
🤖 Prompt for AI Agents
In frontend/components/Item/BarcodeCard.vue around lines 73-77, the prop
definition for locationFlatTree references an undeclared type FlatTreeItem and
the prop is unused; either remove this unused prop block entirely from the
component props to eliminate dead API and fix type errors, or if it is needed,
add an import for the FlatTreeItem type at the top of the script (e.g. import
type { FlatTreeItem } from '...') and ensure the prop is actually consumed in
the template or script so the type is justified.
| function createItem() { | ||
| if ( | ||
| products.value !== null && | ||
| products.value.length > 0 && | ||
| selectedRow.value >= 0 && | ||
| selectedRow.value < products.value.length | ||
| ) { | ||
| const p = products.value![selectedRow.value]; | ||
| openDialog(DialogID.CreateItem, { | ||
| if (selectedItem.value === null) return; | ||
| openDialog(DialogID.CreateItem, { | ||
| params: { product: p }, | ||
| }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Undefined variable 'p' in createItem()
p is not defined; should pass the selected product.
function createItem() {
if (selectedItem.value === null) return;
- openDialog(DialogID.CreateItem, {
- params: { product: p },
- });
+ openDialog(DialogID.CreateItem, {
+ params: { product: selectedItem.value },
+ });
}📝 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.
| function createItem() { | |
| if ( | |
| products.value !== null && | |
| products.value.length > 0 && | |
| selectedRow.value >= 0 && | |
| selectedRow.value < products.value.length | |
| ) { | |
| const p = products.value![selectedRow.value]; | |
| openDialog(DialogID.CreateItem, { | |
| if (selectedItem.value === null) return; | |
| openDialog(DialogID.CreateItem, { | |
| params: { product: p }, | |
| }); | |
| } | |
| }); | |
| } | |
| function createItem() { | |
| if (selectedItem.value === null) return; | |
| openDialog(DialogID.CreateItem, { | |
| params: { product: selectedItem.value }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In frontend/components/Item/BarcodeModal.vue around lines 108 to 114, the
createItem() function references an undefined variable `p`; change the code to
pass the selected product instead (use selectedItem.value) when opening the
dialog. Keep the existing null guard, and call openDialog(DialogID.CreateItem, {
params: { product: selectedItem.value } }); so the dialog receives the actual
selected product.
| { | ||
| name: "items.barcode", | ||
| text: item.value?.barcode, | ||
| type: "barcode", | ||
| copyable: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
LGTM: Barcode detail added; verify renderer supports type 'barcode'
Looks correct. Please verify DetailsSection supports a type: "barcode" detail.
🏁 Script executed:
#!/bin/bash
# Verify DetailsSection supports a barcode detail type
set -euo pipefail
echo "Searching for DetailsSection type handling of 'barcode'..."
fd -a types.ts frontend/components/global/DetailsSection 2>/dev/null | while read -r f; do
echo "==> $f"
rg -n -C2 -i 'barcode|DetailType|type.*barcode' "$f" || true
doneLength of output: 411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for barcode rendering logic in DetailsSection Vue component..."
fd -e vue frontend/components/global/DetailsSection 2>/dev/null | while read -r file; do
echo "==> $file"
rg -n -C3 -i 'barcode' "$file" || true
doneLength of output: 72
Implement rendering for type: "barcode" in DetailsSection.vue. DetailsSection defines a Barcode detail but has no template or switch‐case handling it; add a rendering branch (e.g. <Barcode> component) and sanitize barcode text to prevent XSS.
🤖 Prompt for AI Agents
In frontend/pages/item/[id]/index.vue around lines 215 to 220, the
DetailsSection consumer is passing a detail with type: "barcode" but
DetailsSection.vue has no template/switch-case handling for "barcode"; add a
branch in the DetailsSection render logic (the switch or template v-if chain) to
render the Barcode component (import it at the top), pass the sanitized barcode
string as a plain text prop (do not use v-html), and preserve the copyable
behavior; sanitize/escape the barcode value before passing (or use a trusted
sanitizer library like DOMPurify) to prevent XSS and ensure the component
displays only text content.
| { | ||
| type: "text", | ||
| label: "items.barcode", | ||
| ref: "barcode", | ||
| maxLength: 255, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce numeric-only barcode input (aligns with PR goal)
The new field is plain text; it doesn’t restrict input to digits. Add input hints and a pattern to enforce numeric-only on mobile and desktop.
Apply this diff to the field object:
{
type: "text",
label: "items.barcode",
ref: "barcode",
maxLength: 255,
+ // Hints and client-side enforcement
+ inputmode: "numeric",
+ pattern: "^[0-9]*$",
},Additionally, update types and pass-through so these props reach FormTextField:
- Extend TextFormField and import HTMLAttributes (outside this hunk):
// near other type defs
import type { HTMLAttributes } from "vue";
type TextFormField = {
type: "text" | "textarea";
label: string;
ref: NonNullableStringKeys<ItemOut>;
maxLength?: number;
minLength?: number;
inputmode?: HTMLAttributes["inputmode"];
pattern?: string;
};- Pass to FormTextField in the template (outside this hunk):
<FormTextField
v-else-if="field.type === 'text'"
v-model="item[field.ref]"
:label="$t(field.label)"
inline
type="text"
:max-length="field.maxLength"
:min-length="field.minLength"
:inputmode="field.inputmode"
:pattern="field.pattern"
/>🤖 Prompt for AI Agents
In frontend/pages/item/[id]/index/edit.vue around lines 203-208, the barcode
field is currently a plain text field and needs to enforce numeric-only input;
update the field object to include inputmode: "numeric" and pattern: "^[0-9]*$"
(or a pattern that matches your barcode rules) so mobile keyboards and desktop
validation favor digits, and then follow the additional changes: extend
TextFormField (import HTMLAttributes from "vue" and add inputmode and pattern
properties) and pass :inputmode and :pattern through to FormTextField in the
template so those props reach the component.
|
@tonyaellie sorry to ping you again on this, if you have some time to check the conflict with the latest table modification. Thanks! |
|
Yes it's on my to-do list I've just been really busy with uni starting again, will have a look as soon as I can! |
tonyaellie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about using this for things that arent create modals, i think if we want to do that we should make a more universal abstract component for all dialogs
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import JsBarcode from "jsbarcode"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if the cost of jsbarcode (a relatively big package) is really worth it, seems like if we want to just display the barcode we would be better off with just the number, or alternatively generate the barcode in go and expose it as an api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively https://nuxt.com/docs/4.x/guide/directory-structure/app/components#delayed-or-lazy-hydration might help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it mean the JS code of jsbarcode will be loaded only when the barcode have to be displayed, instead of being "loaded at first" page load (e.g: index)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few different hydration options, so like you can load on click, or on visible or others, though I haven't used it before so not sure how well it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont really think using this for both items and barcodes makes sense, espeically with the changes to how the item table works, i think we would be better to have a separate lot of components for barcode, a simpler version of the new item table should be realtively easy to build and be simpler, and then i think instead of making it selectable we just use card view on sm/md displays and table on bigger.
| <Card class="selectable-card cursor-pointer overflow-hidden" :class="{ selected }" @click="toggleSelect"> | ||
| <div class="relative h-[200px]"> | ||
| <img | ||
| v-if="props.item?.imageBase64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image doesnt seem to be loading for me, we should prob use the placeholder if it doesnt load (or just dont have the h-[200px] if theres no image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometime images are not available for a given product. Could you check if an image is available when you go on UPCDBItem website for those specific, yummy, Krispies Cereal :) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It did show an image on UPCDBItem website

What type of PR is this?
What this PR does / why we need it:
Improve the barcode feature brought by #818
Documentation, better ui on smartphone and card support are the main new features of this PR.
Which issue(s) this PR fixes:
EDIT: postgre migration is not included in this PR. Anyone is welcomed to add it to this PR!
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores