Skip to content

fix: apply post-merge review follow-up#104

Open
GitAddRemote wants to merge 1 commit intomainfrom
fix/ISSUE-55-review-followup
Open

fix: apply post-merge review follow-up#104
GitAddRemote wants to merge 1 commit intomainfrom
fix/ISSUE-55-review-followup

Conversation

@GitAddRemote
Copy link
Copy Markdown
Owner

  • enforce non-negative org inventory quantity filters in controller parsing

  • add regression coverage for negative minQuantity and maxQuantity inputs

  • stabilize fallback inline drafts so memoized inventory rows do not rerender unnecessarily

- enforce non-negative org inventory quantity filters in controller parsing

- add regression coverage for negative minQuantity and maxQuantity inputs

- stabilize fallback inline drafts so memoized inventory rows do not rerender unnecessarily
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Follow-up fixes to org inventory filtering validation and frontend inventory row rendering stability.

Changes:

  • Enforce minQuantity/maxQuantity query filters to be non-negative in the org-inventory controller.
  • Add regression tests asserting negative quantity filters return 400 BadRequest.
  • Stabilize inline draft fallback objects in the Inventory page to reduce unnecessary row re-renders.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
frontend/src/pages/Inventory.tsx Adds memoized fallback draft caching for inline edit rows and cleans up stale cache entries when items change.
backend/src/modules/org-inventory/org-inventory.controller.ts Applies min: 0 validation to min_quantity and max_quantity query parsing.
backend/src/modules/org-inventory/org-inventory.controller.spec.ts Adds tests ensuring negative quantity filters throw the expected BadRequestException.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1258 to +1274
const rowKey = item.id.toString();
const nextFallback: InlineDraft = {
locationId: Number(item.locationId) || '',
quantity: Number(item.quantity) || 0,
};
const cachedFallback = inlineDraftFallbacks.current.get(rowKey);

if (
cachedFallback &&
cachedFallback.locationId === nextFallback.locationId &&
cachedFallback.quantity === nextFallback.quantity
) {
return cachedFallback;
}

inlineDraftFallbacks.current.set(rowKey, nextFallback);
return nextFallback;
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getInlineDraft mutates inlineDraftFallbacks.current during render (via set), since it’s invoked from renderInlineRow. Render-phase mutations are side effects and can behave unpredictably with React 18 concurrent rendering / StrictMode (e.g., aborted renders populating the cache). Consider making getInlineDraft a pure read and moving cache updates to an effect keyed on items (or building/refreshing the fallback map in the existing [items] effect).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants