Fix: Prevent negative item quantities in edit and create modals#1463
Open
Copilot wants to merge 4 commits into
Open
Fix: Prevent negative item quantities in edit and create modals#1463Copilot wants to merge 4 commits into
Copilot wants to merge 4 commits into
Conversation
6 tasks
Agent-Logs-Url: https://github.com/sysadminsmedia/homebox/sessions/ba907706-5c93-4634-8721-4197c9a8bd8f Co-authored-by: katosdev <7927609+katosdev@users.noreply.github.com>
Agent-Logs-Url: https://github.com/sysadminsmedia/homebox/sessions/ba907706-5c93-4634-8721-4197c9a8bd8f Co-authored-by: katosdev <7927609+katosdev@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix negative quantity issue in advanced editing
Fix: Prevent negative item quantities in edit and create modals
Apr 25, 2026
Contributor
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
WalkthroughAdds client-side guards to block creating or saving items with negative quantities; other edits are formatting, import consolidation, and a minor script-block relocation. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (UI)"
participant CreateModal as "CreateModal / Edit Page"
participant Toast as "Toast"
participant API as "Backend API"
rect rgba(135,206,250,0.5)
User->>CreateModal: submit form (includes quantity)
end
alt quantity < 0
CreateModal->>Toast: emit error "quantity_cannot_negative"
Toast-->>User: show error
else quantity >= 0
CreateModal->>CreateModal: set loading
CreateModal->>API: POST/PUT item payload
API-->>CreateModal: response
CreateModal-->>User: success / update UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Security Recommendations
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/pages/location/[id]/index.vue (1)
1-14: Consolidate back into a single<script setup>block — and this change looks out of scope for the PR. 🛡️A few concerns with this refactor:
- Empty
<script setup>+ a separate plain<script>is unnecessary noise.definePageMetais a compile-time macro that works perfectly inside<script setup lang="ts">. The canonical pattern in this repo isfrontend/pages/profile.vue, which keepsdefinePageMeta({ middleware: ["auth"] })inside the setup block. Splitting into two script blocks (one empty) doesn't change behavior, only readability.- Manual
BaseContainerimport violates the auto-import rule. Components undercomponents/are auto-imported by Nuxt; the explicitimport BaseContainer from "@/components/Base/Container.vue"is redundant. Note also that this import lives in a non-setup script that has no other consumers, so it's effectively dead code —<BaseContainer>in the template resolves via Nuxt auto-import regardless.- Scope creep. PR
#1463is titled "Prevent negative item quantities in edit and create modals" and the AI summary describes this hunk as a structural refactor unrelated to the validation fix. Mixing unrelated refactors with bug fixes makes git history and revert/cherry-pick harder. Consider extracting this into its own PR (or just dropping it).Security note: no auth/authorization regression here — the
authmiddleware is still applied either way, sincedefinePageMetais hoisted by the Nuxt compiler regardless of which<script>block hosts it. So this is a hygiene/scope concern, not a security one.♻️ Suggested simplification
-<script setup lang="ts"></script> - -<template> - <BaseContainer> - <NuxtPage /> - </BaseContainer> -</template> - -<script lang="ts"> - import BaseContainer from "@/components/Base/Container.vue"; - definePageMeta({ - middleware: ["auth"], - }); -</script> +<script setup lang="ts"> + definePageMeta({ + middleware: ["auth"], + }); +</script> + +<template> + <BaseContainer> + <NuxtPage /> + </BaseContainer> +</template>As per coding guidelines: "Use
<script setup lang="ts">syntax in all Vue components" and (learning) "Do not manually import components fromcomponents/directory — they are auto-imported based on file path".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/location/`[id]/index.vue around lines 1 - 14, Consolidate the two script blocks into a single <script setup lang="ts"> block: move the definePageMeta({ middleware: ["auth"] }) call into the existing <script setup> and remove the separate plain <script> block and the manual import of BaseContainer; rely on Nuxt's auto-imported BaseContainer component in the template (keep NuxtPage as-is). This reverts the out-of-scope refactor—keep only the middleware meta change in the setup block and drop the explicit import to match the project's canonical pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/Item/CreateModal.vue`:
- Around line 647-650: The negative-quantity guard for form.quantity is bypassed
for empty/NaN values and the input lacks a min constraint; update the validation
to explicitly check Number.isFinite(form.quantity) and that form.quantity >= 0
(e.g., use !Number.isFinite(form.quantity) || form.quantity < 0) in the submit
path (where form.quantity is used) and add min="0" to the FormTextField
component bound with v-model.number to prevent negative/empty entry client-side;
additionally ensure server-side handlers (api.items.create and
api.templates.createItem) validate and reject quantity values that are negative
or non-finite.
In `@frontend/components/Location/CreateModal.vue`:
- Line 70: The FormTextArea in CreateModal.vue is using a hardcoded label
"Notes"; replace it with a translatable string using the i18n helper (e.g.
change label="Notes" to label="$t('location.form.notes')" or another appropriate
i18n key) so the FormTextArea bound to form.notes is consistent with the rest of
the modal and translatable.
In `@frontend/pages/collection/index/entity-types.vue`:
- Line 155: Replace hardcoded label text passed to the FormTextField components
with the translation function; locate the two usages bound to createForm.name
(the FormTextField at the create dialog and the one at line ~173) and change
label="Name" to label=t('...') using the appropriate i18n key (e.g.
t('entityType.name') or existing key convention), ensuring you import or
reference the same translation function used elsewhere in this component so both
FormTextField instances use translated labels.
In `@frontend/pages/item/`[id]/index/edit.vue:
- Around line 97-100: The quantity check currently allows NaN/empty and is
bypassed by the sibling update path; change the guard around item.value.quantity
to reject non-finite values (e.g., use Number.isFinite) and negative values so
it reads like !Number.isFinite(item.value.quantity) || item.value.quantity < 0,
update the FormTextField number inputs to include min="0" (and/or add validation
in mainFields) to prevent clearing to NaN in the UI, and apply the same
validation before calling api.items.update in syncChildEntityLocations (or
factor the validation into a shared helper used by both the inline edit save and
syncChildEntityLocations) so negative/non-finite quantities never get sent to
the API; also ensure the backend enforces the rule server-side.
---
Nitpick comments:
In `@frontend/pages/location/`[id]/index.vue:
- Around line 1-14: Consolidate the two script blocks into a single <script
setup lang="ts"> block: move the definePageMeta({ middleware: ["auth"] }) call
into the existing <script setup> and remove the separate plain <script> block
and the manual import of BaseContainer; rely on Nuxt's auto-imported
BaseContainer component in the template (keep NuxtPage as-is). This reverts the
out-of-scope refactor—keep only the middleware meta change in the setup block
and drop the explicit import to match the project's canonical pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3078254f-f0e2-4736-b63e-30c5aad0c078
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
frontend/components/Item/CreateModal.vuefrontend/components/Location/CreateModal.vuefrontend/components/Scanner/ARControls.vuefrontend/lib/api/classes/items.tsfrontend/pages/collection/index/entity-types.vuefrontend/pages/item/[id]/index/edit.vuefrontend/pages/location/[id]/index.vuefrontend/pages/location/[id]/index/index.vue
| :max-length="255" | ||
| :min-length="1" | ||
| /> | ||
| <FormTextField v-model="createForm.name" :autofocus="true" label="Name" :max-length="255" :min-length="1" /> |
Contributor
There was a problem hiding this comment.
Use translated labels instead of hardcoded "Name" in form fields.
Both changed FormTextField usages still hardcode user-facing text. Switch label to t(...) so the dialogs are localizable.
Suggested change
- <FormTextField v-model="createForm.name" :autofocus="true" label="Name" :max-length="255" :min-length="1" />
+ <FormTextField v-model="createForm.name" :autofocus="true" :label="t('common.name')" :max-length="255" :min-length="1" />
...
- <FormTextField v-model="updateForm.name" :autofocus="true" label="Name" :max-length="255" :min-length="1" />
+ <FormTextField v-model="updateForm.name" :autofocus="true" :label="t('common.name')" :max-length="255" :min-length="1" />Security recommendation: keep server-side validation/authorization as the source of truth for entity-type create/update/delete, since UI constraints can be bypassed.
As per coding guidelines "Check for hardcoded strings in UI components that should be translatable."
Also applies to: 173-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/pages/collection/index/entity-types.vue` at line 155, Replace
hardcoded label text passed to the FormTextField components with the translation
function; locate the two usages bound to createForm.name (the FormTextField at
the create dialog and the one at line ~173) and change label="Name" to
label=t('...') using the appropriate i18n key (e.g. t('entityType.name') or
existing key convention), ensuring you import or reference the same translation
function used elsewhere in this component so both FormTextField instances use
translated labels.
Comment on lines
+97
to
+100
| if (item.value.quantity < 0) { | ||
| toast.error(t("items.toast.quantity_cannot_negative")); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Same hop, slightly different burrow — and there's a side-door update path. 🐰🚪
Mirror of the CreateModal feedback, plus one extra:
- NaN/empty bypass. The number form fields on this page (e.g., line 632-639) use
v-model.numberwithtype="number"and nomin. An empty/cleared quantity becomes""orNaN, andNaN < 0isfalse, so the guard lets it through. Consider!Number.isFinite(item.value.quantity) || item.value.quantity < 0. Addingmin="0"to thenumber-type FormTextField branch (and/or to the schema inmainFields) would also harden the UI layer. - Sibling update path bypasses the guard.
syncChildEntityLocations()(lines 475-501) also callsapi.items.update(itemId.value, payload)and spreads...item.valueinto the payload — including a possibly-negativequantityset just before flipping the sync switch. If you want the rule to be "no negative quantities ever leave this page," that path needs the same check (or factor the validation into a shared helper). - 🔐 Security recommendation: Treat this guard as UX, not enforcement. Please confirm the backend
items.updaterejects negative (and non-finite) quantities — otherwise a hand-crafted PATCH still wins.
🛠️ Proposed shared guard + sibling path coverage
+ function hasInvalidQuantity(): boolean {
+ return !Number.isFinite(item.value.quantity) || item.value.quantity < 0;
+ }
+
async function saveItem(redirect: boolean) {
if (!item.value.parent?.id && !parent.value?.id) {
toast.error(t("items.toast.failed_save_no_location"));
return;
}
- if (item.value.quantity < 0) {
+ if (hasInvalidQuantity()) {
toast.error(t("items.toast.quantity_cannot_negative"));
return;
} async function syncChildEntityLocations() {
if (!item.value.parent?.id && !parent.value?.id) {
toast.error(t("items.toast.failed_save_no_location"));
return;
}
+
+ if (hasInvalidQuantity()) {
+ toast.error(t("items.toast.quantity_cannot_negative"));
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/pages/item/`[id]/index/edit.vue around lines 97 - 100, The quantity
check currently allows NaN/empty and is bypassed by the sibling update path;
change the guard around item.value.quantity to reject non-finite values (e.g.,
use Number.isFinite) and negative values so it reads like
!Number.isFinite(item.value.quantity) || item.value.quantity < 0, update the
FormTextField number inputs to include min="0" (and/or add validation in
mainFields) to prevent clearing to NaN in the UI, and apply the same validation
before calling api.items.update in syncChildEntityLocations (or factor the
validation into a shared helper used by both the inline edit save and
syncChildEntityLocations) so negative/non-finite quantities never get sent to
the API; also ensure the backend enforces the rule server-side.
Contributor
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/Item/CreateModal.vue`:
- Around line 647-650: The toast uses the wrong i18n key so users will see the
raw key; in CreateModal.vue update the t(...) call used in the quantity guard
(the code referencing form.quantity and toast.error) to use the existing key
"items.toast.quantity_cannot_negative" instead of
"components.item.create_modal.toast.quantity_cannot_negative", and confirm
server-side validation for api.items.create / api.templates.createItem rejects
quantity < 0 (and non-finite values) so the UI-only guard can't be bypassed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2a6f31c4-fe60-4c33-8239-db25c9f0f7eb
📒 Files selected for processing (4)
frontend/components/Item/CreateModal.vuefrontend/components/Location/CreateModal.vuefrontend/pages/collection/index/entity-types.vuefrontend/pages/item/[id]/index/edit.vue
✅ Files skipped from review due to trivial changes (2)
- frontend/pages/collection/index/entity-types.vue
- frontend/components/Location/CreateModal.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/pages/item/[id]/index/edit.vue
Comment on lines
+647
to
+650
| if (form.quantity < 0) { | ||
| toast.error(t("components.item.create_modal.toast.quantity_cannot_negative")); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Wrong i18n key — toast will display the raw key string instead of a translated message. 🐰🌍
The guard calls t("components.item.create_modal.toast.quantity_cannot_negative"), but that path doesn't exist in frontend/locales/en.json. The actual key lives under items.toast.quantity_cannot_negative (see en.json line 620), and the sibling frontend/pages/item/[id]/index/edit.vue (lines 97-100) and the PR description both use that path. With vue-i18n's default missing-handler the user will see the literal string components.item.create_modal.toast.quantity_cannot_negative in the toast — in every language. Switch to the existing key.
🥕 Proposed fix
if (form.quantity < 0) {
- toast.error(t("components.item.create_modal.toast.quantity_cannot_negative"));
+ toast.error(t("items.toast.quantity_cannot_negative"));
return;
}🔐 Security recommendation (carried over): this remains a client-side check only. Please confirm the backend handlers behind api.items.create and api.templates.createItem reject quantity < 0 (and ideally non-finite values) so a crafted request can't bypass the UI guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Item/CreateModal.vue` around lines 647 - 650, The toast
uses the wrong i18n key so users will see the raw key; in CreateModal.vue update
the t(...) call used in the quantity guard (the code referencing form.quantity
and toast.error) to use the existing key "items.toast.quantity_cannot_negative"
instead of "components.item.create_modal.toast.quantity_cannot_negative", and
confirm server-side validation for api.items.create / api.templates.createItem
rejects quantity < 0 (and non-finite values) so the UI-only guard can't be
bypassed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The quantity field in both the item edit page and create modal lacked validation, allowing users to type or spin negative values — bypassing the guard that already existed on the item detail page's +/- buttons.
What type of PR is this?
What this PR does / why we need it:
Adds
quantity < 0guard before save/create in both frontend surfaces, reusing the existingitems.toast.quantity_cannot_negativetranslation key for consistency.pages/item/[id]/index/edit.vue— validateitem.value.quantity < 0insaveItem()before proceedingcomponents/Item/CreateModal.vue— validateform.quantity < 0increate()before proceedingBoth bail early with a toast error, matching the existing behavior on the item detail page's quantity adjuster.
Which issue(s) this PR fixes:
Special notes for your reviewer:
The
items.toast.quantity_cannot_negativekey is already translated across all locale files — no new i18n strings needed.Summary by CodeRabbit
Bug Fixes
Localization / UI