Tested/Working Wishlist Population by giannacastr · Pull Request #305 · GTBitsOfGood/hope-for-haiti

giannacastr · 2026-03-09T07:06:33Z

Description

Resolves ticket number: #296

Explain what your code changes:
When bulkUpdateRequests is called to set final quantities on partner requests, any request where finalQuantity < quantity now automatically creates a wishlist entry for that partner with the unfulfilled amount and the comment "Automatically generated due to unfulfilled request". The logic lives in a new createUnfulfilledWishlistEntries method in WishlistService, which is called after the bulk update transaction completes. If a partner already has a wishlist entry with the same item name, the unfulfilled quantity is added to it rather than creating a duplicate.

List the steps you took to test your code:
The seed file was updated with a three-partner test scenario mirroring the ticket exactly — 500 units of Cough Syrup, with Partner 1 (LOW, 200 requested, 0 fulfilled), Partner 2 (HIGH, 300 requested, 300 fulfilled), and Partner 3 (MEDIUM, 300 requested, 200 fulfilled). Console assertions confirmed wishlist entries were created only for Partners 1 and 3 with quantities 200 and 100 respectively, and Partner 2 had no entry. A second test case on a separate donor offer verified that a partner's existing wishlist entry for the same item name has its quantity merged rather than duplicated.

Checklist

The ticket is mentioned above
The changes fulfill the success criteria of the ticket
The changes were self-reviewed
A review was requested

netlify · 2026-03-09T07:06:37Z

✅ Deploy Preview for hope-for-haiti ready!

Name	Link
🔨 Latest commit	`f07ac3c`
🔍 Latest deploy log	https://app.netlify.com/projects/hope-for-haiti/deploys/69b3b4c63cdb220009a2ca82
😎 Deploy Preview	https://deploy-preview-305--hope-for-haiti.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

greptile-apps · 2026-03-09T07:09:08Z

Greptile Summary

This PR implements automatic wishlist population for unfulfilled partner requests. When a partner's request is not fully fulfilled — either because an admin sets a finalQuantity < quantity via bulkUpdateRequests, or because a distribution is approved with fewer allocated units than requested — a wishlist entry is auto-created (or updated) for the partner with the unfulfilled quantity. The core logic lives in the new WishlistService.createUnfulfilledWishlistEntries method, which now has two independent callers.

Key concerns:

Dual trigger paths conflict with each other (distributionService.ts and generalItemRequestService.ts both call createUnfulfilledWishlistEntries independently). If both workflows are triggered for the same generalItemRequest, the wishlist is updated twice, potentially with conflicting values.
populateUnfulfilledWishlists silently overwrites finalQuantity — it always sets finalQuantity = allocated, unconditionally overwriting any value previously set by bulkUpdateRequests, which can corrupt admin-set data.
Replace vs. add semantics — previously flagged in the JSDoc, and now also visible in the seed test comments which assert an additive 50 + 125 = 175 result that the code does not actually produce (it would produce 125).
Several other issues were flagged in prior review rounds (N+1 queries, idempotency, generalItemId: null filter matching manual entries, missing newline at EOF, console.error/console.warn) and carry over as outstanding concerns.

Backend checklist areas needing improvement: operations are not fully optimized (sequential DB calls per iteration remain), edge cases around dual-trigger and finalQuantity overwrite are unhandled, and there is a console.error in production code.

Score: 62/100

Confidence Score: 3/5

Not safe to merge — two P1 data-integrity issues remain: admin-set finalQuantity can be silently overwritten on distribution approval, and the dual trigger paths can leave wishlist entries in an inconsistent state.
Two confirmed P1 bugs exist in the new code: (1) populateUnfulfilledWishlists overwrites finalQuantity without checking whether an admin already set a value, and (2) both bulkUpdateRequests and populateUnfulfilledWishlists independently call createUnfulfilledWishlistEntries, producing double-updates with potentially different quantities. Several prior concerns (N+1 queries, idempotency, replace-vs-add semantics, EOF newlines, console statements) also remain open. The feature works in isolation for the happy path but is brittle when both admin and distribution workflows touch the same requests.
src/services/distributionService.ts (overwrite bug, dual-trigger) and src/services/wishlistService.ts (core logic, outstanding prior review concerns) need the most attention before merging.

Important Files Changed

Filename	Overview
src/services/wishlistService.ts	Adds `createUnfulfilledWishlistEntries` — the core new method. Pre-fetches existing wishlist items in one query (good), but the update path replaces quantity instead of adding it (contradicting JSDoc, PR description, and seed comments), the `generalItemId: null` filter incorrectly treats manually-created entries as auto-generated, and there's no in-memory cache update after creating a new entry (could produce duplicates within one batch).
src/services/distributionService.ts	Adds `populateUnfulfilledWishlists` triggered on distribution approval. Two significant issues: (1) it unconditionally overwrites `finalQuantity` regardless of any admin-set value from `bulkUpdateRequests`, and (2) together with `generalItemRequestService`, it creates two independent code paths that can both call `createUnfulfilledWishlistEntries` for the same requests, producing conflicting wishlist state. Also has a missing indentation on the new method.
src/services/generalItemRequestService.ts	Adds wishlist auto-population after `bulkUpdateRequests` and fixes a `findFirst`→`findUnique` correctness improvement. The new wishlist trigger is logically sound in isolation but interacts badly with the distribution-approval trigger in `distributionService.ts`.
scripts/seed.ts	Adds a manual test scenario for wishlist merging. The scenario setup is clear, but the expected outcome comments describe additive behavior (`50 + 125 = 175`) that does not match the code's actual replace semantics (result would be `125`).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Admin calls bulkUpdateRequests] -->|sets finalQuantity per request| B[DB: generalItemRequest.finalQuantity = X]
    B --> C{unfulfilledUpdates\nfinalQuantity < quantity?}
    C -->|Yes| D[createUnfulfilledWishlistEntries\ngeneralItemRequestService path]
    D --> E[Wishlist entry created/updated]

    F[Admin approves Distribution] --> G[distributionService.updateDistribution\npending: false]
    G --> H[createShipmentsForDistribution]
    G --> I[populateUnfulfilledWishlists]
    I --> J[Compute allocated qty\nper generalItemId]
    J --> K{allocated <\nrequest.quantity?}
    K -->|Yes| L[⚠️ Overwrites finalQuantity\nwith allocated — ignores\nadmin-set value]
    L --> M[createUnfulfilledWishlistEntries\ndistributionService path]
    M --> N[Wishlist entry created/updated AGAIN]

    E -.->|Same wishlist entry| N
    style L fill:#f99,stroke:#c00
    style N fill:#f99,stroke:#c00

_{Reviews (4): Last reviewed commit: "merge" | Re-trigger Greptile}

giannacastr · 2026-03-10T16:03:36Z

@greptile-ai

greptile-apps · 2026-03-10T16:07:08Z

    }
  }
-}
+}


Missing newline at end of file

Both wishlistService.ts and generalItemRequestService.ts now end without a trailing newline (the diff shows \ No newline at end of file). Most linters (e.g., ESLint's eol-last rule) treat this as a warning or error. Please restore the trailing newline to keep the linter happy and avoid noisy future diffs.

The same issue exists at the end of src/services/generalItemRequestService.ts.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

giannacastr · 2026-03-10T16:25:15Z

@greptileai

greptile-apps · 2026-03-10T16:28:51Z

+  /**
+   * For each request where finalQuantity < quantity, automatically creates or
+   * updates a wishlist entry for the partner with the unfulfilled quantity.
+   * If an entry with the same name already exists for the partner, the
+   * unfulfilled quantity is added to it.
+   */


JSDoc comment contradicts actual implementation

The doc says "the unfulfilled quantity is added to it", but line 83 does data: { quantity: unfulfilledQuantity } which replaces the existing quantity rather than incrementing it. The PR description also describes the additive behavior. The comment should be corrected to match the code's actual semantics:

* If an entry with the same name already exists for the partner, its * quantity is replaced with the new unfulfilled quantity.

netlify · 2026-03-30T22:16:06Z

✅ Deploy Preview for hope-for-haiti ready!

Name	Link
🔨 Latest commit	`d2f6aec`
🔍 Latest deploy log	https://app.netlify.com/projects/hope-for-haiti/deploys/69cb30e06e9d7000084c1ec2
😎 Deploy Preview	https://deploy-preview-305--hope-for-haiti.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

ND68 · 2026-03-30T22:25:25Z

@greptileai

…manual quantity adjustments in table

…generalItemRequestService is sufficient

ND68 · 2026-03-31T21:24:03Z

Added more robustness to you implementation so that manual entry changes also reflect in unfulfilled wishlist quantities. Otherwise, great work

updated logic pre-test

ff3fbcd

giannacastr requested a review from ND68 March 9, 2026 07:06

giannacastr self-assigned this Mar 9, 2026

greptile-apps Bot reviewed Mar 9, 2026

View reviewed changes

Comment thread src/services/generalItemRequestService.ts Outdated

Comment thread src/services/wishlistService.ts Outdated

Comment thread src/services/wishlistService.ts Outdated

giannacastr linked an issue Mar 9, 2026 that may be closed by this pull request

Populate Wishlist with Unfulfilled Requests #296

Closed

greptile-apps Bot reviewed Mar 10, 2026

View reviewed changes

greptile suggestions

915ea6b

greptile-apps Bot reviewed Mar 10, 2026

View reviewed changes

giannacastr added 2 commits March 13, 2026 02:49

linked distribution approval to wishlist auto-population

f2c8d50

removed seed test

f07ac3c

giannacastr force-pushed the gia/296-populate-wishlist-summary branch from 915ea6b to f07ac3c Compare March 13, 2026 06:55

merge

f3396b7

greptile-apps Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/services/distributionService.ts Outdated

Comment thread src/services/generalItemRequestService.ts Outdated

ND68 added 3 commits March 30, 2026 22:15

updates wishlist based on quantity deltas instead, added support for …

5a10fd3

…manual quantity adjustments in table

wishlist functionality in distributionService is redundant, logic in …

0c5047d

…generalItemRequestService is sufficient

lint

d2f6aec

ND68 merged commit 044e844 into main Mar 31, 2026
5 checks passed

ND68 deleted the gia/296-populate-wishlist-summary branch March 31, 2026 02:29

                   }
                 }
-              }
+              }

                
                    No newline at end of file

Conversation

giannacastr commented Mar 9, 2026

Description

Checklist

Uh oh!

netlify Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for hope-for-haiti ready!

Uh oh!

greptile-apps Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

giannacastr commented Mar 10, 2026

Uh oh!

Uh oh!

greptile-apps Bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

giannacastr commented Mar 10, 2026

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

netlify Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for hope-for-haiti ready!

Uh oh!

ND68 commented Mar 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ND68 commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify Bot commented Mar 9, 2026 •

edited

Loading

greptile-apps Bot commented Mar 9, 2026 •

edited

Loading

netlify Bot commented Mar 30, 2026 •

edited

Loading