Enter owned quantity per product and auto-calculate the global owned amount - #40
Conversation
Before, the shopping list took one "owned" amount per ingredient in a single unit. A user could not say "I have 3 of pack A and 5 of pack B" when an ingredient has several products of different pack sizes. Now each product row has its own owned count input. The ingredient's global owned amount is summed from each product's count times its pack quantity, converted with the ingredient's existing grams/ml conversions (toGrams / fromGrams). That summed amount feeds the existing remaining-to-buy and multi-trip planning logic unchanged. OwnedStock gains a perProduct shape (a count per product) alongside the existing single amount+unit shape, and a shared amountInUnit resolver so the on-screen "Need" and the copied trip amounts still match exactly. Ingredients with no products keep the single header owned input. Updates ADR 0010 and ADR 0014, which documented owned as one amount per ingredient. Closes #24 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An ingredient that has products but where no product's unit matches any recipe unit (for example a pieces-only product used by a grams recipe, bridged via gramsPerPiece) rendered no product rows and also hid the single header owned input. The user could then enter no owned amount at all, so the shopping list always asked to buy the full amount. The header owned input was gated only on "the ingredient has no products". Gate both inputs on a new shared check, usesPerProductOwnedInputs: per-product inputs stay the primary path when at least one product unit matches a recipe unit; otherwise the header input shows as a fallback. ShoppingPage's owned-stock resolver uses the same check, so the fallback amount flows through the shared resolver and the remaining-to-buy stays correct. Also fix the fallback's default unit: packs cannot convert when no product matches the target unit, so defaultOwnedUnit now falls through to a concrete unit (pieces, then the first recipe unit, then grams). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ShoppingProductRow seeded its text controller once in initState and never updated it, and the rows were built in a loop with no Key. If a product's owned count were ever reset from outside (for example clearing all owned stock), the field would keep showing stale text, and without a Key Flutter could reuse the wrong row's state when the product list changes. Add didUpdateWidget so the field re-seeds when ownedCount changes from the parent, and give each row a ValueKey of the product's true index so its state stays bound to that product. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
guplem
left a comment
There was a problem hiding this comment.
Reviewed by an automated pass (deep review + a fix round). One real gap was found and fixed on the branch; the change is now correct and ready for a human check.
What is done well
- Clean conversion logic: the global owned amount sums each product's
count x pack quantityand converts through the ingredient's realtoGrams/fromGrams(bridging by density or grams-per-piece). No double-counting; per-product indices are stable and read back through bounds-guarded lookups. - Page and copy use one shared resolver (
OwnedStock.amountInUnit), so the on-screen Need and the copied amounts stay identical, including the multi-trip planner. Good tests on real summed grams (3x500 + 5x250 = 2750), pieces-via-grams-per-piece, and the planner buy amount. - No-products fallback preserved.
Gap found and fixed on this branch (was a silent loss of function)
- An ingredient that has products but where no product unit matches any recipe unit (e.g. a pieces-only product used by a grams recipe) rendered no product rows AND hid the header input, so the user could enter no owned amount at all. Fixed: a shared
usesPerProductOwnedInputs(...)helper now decides both the per-product rows and the header fallback from the same condition, so the header input returns whenever no per-product input would show. A widget test proves entering owned then reduces the on-screen Need. Also hardened the row controller withdidUpdateWidget+ aValueKey. Full suite green (635 tests).
Verdict: approved pending human review. CI analyze-and-test gate must be green before merge.
Note for the reviewer: this PR and #39 both change owned_amount.dart and multi_trip_planner.dart, so expect a merge conflict when landing the second one.
Bring the per-product owned-quantity branch up to date with main and reconcile it with the shopping features that landed there: the copy matches page (#34), the under-buy chip (#29), and the freeze-on-arrival note (#28). The hard part was owned-stock subtraction. This branch (#24) added per-product owned counts; main (#34) added OwnedStockConsumer, a single shared grams pool that subtracts owned stock only once across all units, so the on-screen list and the multi-trip planner never disagree. Wire per-product owned INTO that single pool instead of the old per-unit path: - owned_amount.dart: OwnedStockConsumer and computeRemainingQuantities now take an OwnedStock, not a raw amount + unit. Both stock shapes (single-form and per-product) resolve their grams total through OwnedStock.amountInUnit, so per-product counts feed the same pool. The per-unit fallback (owned pieces with no gramsPerPiece) also reads from amountInUnit, keeping both shapes correct. - multi_trip_planner.dart: build the consumer from the OwnedStock and consume each need through it. Drops the per-unit ownedRemainingByUnit map that would over-subtract across units. - shopping_page.dart: _remainingAmounts routes _ownedStockFor through computeRemainingQuantities, so per-product owned is subtracted once, across units, exactly as the planner does. Removes the now-unused _ownedInUnit helper. - shopping_product_row.dart: keep #24's per-product owned input as a StatefulWidget AND #29's under-buy chip. Fix the under-buy code's bare field references to use widget.* now that the row is stateful. Tests: adapt computeRemainingQuantities callers to the OwnedStock signature (assertions unchanged) and add a per-product test proving it subtracts once across two units via the single pool. ADR 0014: describe the combined behavior (both stock shapes flow through the shared OwnedStockConsumer) and keep #34's copy-matches consequence. All 661 tests pass; flutter analyze clean; format check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
guplem
left a comment
There was a problem hiding this comment.
Rebased onto the updated main (which now has #34, #29, #28) and reconciled. CI is green, the PR is mergeable, and a fresh correctness review of the reconciliation passed.
Reconciliation (verified, this was the hardest merge)
- Per-product owned now flows through #34's single shared owned-pool.
OwnedStockConsumer/computeRemainingQuantitieswere changed to take anOwnedStock; both the single-amount and per-product shapes resolve their grams total viaOwnedStock.amountInUnit(ingredient, grams), which seeds one pool that is drawn down once across units. No per-unit double subtraction was reintroduced (the #34 bug guarantee survives). - Page and planner consume owned through the same
OwnedStockConsumervia one_ownedStockForhelper, so they cannot disagree. The old per-unit subtraction loops were removed. - Per-product grams total = sum of
count x pack quantityconverted once viatoGrams/fromGrams. Product-index keying is bounds-guarded. - Card keeps the per-product owned inputs (#24), the under-buy chip (#29), and the freeze note (#28) together.
- No asserted test value was weakened; the
computeRemainingQuantitiessignature change was applied consistently to all call sites. A new test proves per-product owned is subtracted once across a pieces+grams need (pieces 0, grams 50). Full suite passes together: 661 tests. ADR 0014 reconciled.
Verdict: approved pending human review. CI green and mergeable.
Note: this and #34 both rewrote owned_amount.dart (#34 is already merged), so this PR carries a merge commit reconciling them; the diff vs current main is the net per-product-owned addition.
Bring the per-product owned quantity branch (#40, issue #24) up to date with main after PR #37 (issue #27, "buy one of each" for equivalent products) merged. The only conflict was the product-row section of shopping_ingredient.dart. Both features change how product rows render: - #40 adds a per-product "Owned" input on each row and hides the header owned input when per-product inputs show (usesPerProductOwnedInputs). - #37 groups equivalent products into a combined "buy one of each" block joined by "and", with cycled one-of-each pack shares. Resolved by keeping #37's _buildProductRows grouping and passing each member's owned count and onProductOwnedChanged callback into its ShoppingProductRow, keyed by the product's index in Ingredient.products. Combined equivalent members now each show their own owned input. The header-fallback owned input still shows only when no per-product input renders. Freeze note and under-buy chip preserved. shopping_page.dart, waste_optimizer.dart, and the new copy test merged cleanly: the copy path keeps #37's one-of-each equivalence distribution (buildIngredientCopyLines) and #40's owned subtraction (_ownedStockFor / computeRemainingQuantities feed the remaining amount the copy distributes). Test fix: #37's two auto-fill tests exercised the header owned input's auto-fill button. Under #40, matching-unit products replace that header with per-product inputs, so the button no longer renders in that setup. Reframed both tests to the header-fallback scenario (recipe unit the products do not use) so the auto-fill summing logic is still covered; the summed expectations (3.0 and 1.0) are unchanged. Also added #40's new required params to #37's _pumpProducts helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
guplem
left a comment
There was a problem hiding this comment.
Re-merged after #37 landed on main. CI is green and the PR is mergeable again.
Reconciliation with #37 (verified)
- Only
shopping_ingredient.dartconflicted (the product-row build). #37's_buildProductRowsgrouping (combined "buy one of each" for equivalent products, cycled shares, "and"/"or" dividers, zero-share skipping) is kept, and each rendered row now also carries #40's per-product owned input:ownedCount+onOwnedCountChangedkeyed by the product's true index, plus aValueKey. So each member of a combined equivalent group still shows its own "Owned" input. - The header-fallback owned input stays mutually exclusive with per-product inputs (
!usesPerProductOwnedInputs). - Copy needed no manual edit: it still computes owned-adjusted remaining via
computeRemainingQuantities(#40) and then distributes one-of-each across equivalent products viabuildIngredientCopyLines(#37). - Test change (justified, values unchanged): two auto-fill tests were reframed to the header-fallback scenario because per-product inputs now hide the header auto-fill button for grams-product + grams-recipe; the summing logic and expected values (3.0, 1.0) are unchanged. Full suite passes together: 683 tests.
Verdict: approved pending human review. CI green and mergeable.
Note: #42 is also conflicting now for the same reason (it shares the card/copy code with #37) - it needs the same reconciliation before it can merge.
Summary
Closes #24
Before, the shopping list took one "owned" amount per ingredient in a single unit. Now the user can say how many they own of each product, and the ingredient's global owned amount is computed automatically in grams/ml.
count x pack quantity, converted to the required unit through the ingredient's existing conversions (toGrams/fromGrams, bridging by density or grams-per-piece).OwnedStock.amountInUnit), so the on-screen "Need" and the copied amounts stay identical.Test plan
flutter analyzepassesflutter test test/passes (633 tests)🤖 Generated with Claude Code