Conversation
CherryCIC
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080b098d97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (value.deliveryMethod === 'pickup_point') { | ||
| if (!value.courier) { | ||
| return helpers.error('any.custom', { | ||
| message: | ||
| '"courier" is required when "deliveryMethod" is "pickup_point"', |
There was a problem hiding this comment.
This branch now accepts deliveryMethod: "pickup_point" as long as courier and pickupPointId are present, but createShipment still rejects any order without order.shipping and immediately dereferences order.shipping.address when it builds the Sendcloud payload in src/modules/shipping/controllers/shippingController.ts. In other words, a client that follows the new checkout contract can create pickup-point orders that can never be fulfilled later via /api/shipping/shipment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We'll want to fix this one
| async getParcel(parcelId: number): Promise<SendcloudParcel> { | ||
| return createMockParcel(parcelId, 'Mock Carrier'); |
There was a problem hiding this comment.
In the new non-production default (src/shared/config/sendcloudConfig.ts resolves to mock outside production), getShipmentStatus still overwrites the stored shipment with shippingProvider.getParcel() on every read. Because this mock implementation always returns an Announced parcel, any shipment that was cancelled locally or advanced by handleSendcloudWebhook will be reverted back to announced the next time /api/shipping/shipment/:orderId is fetched.
Useful? React with 👍 / 👎.
MattGoodwin0
left a comment
MattGoodwin0
left a comment
There was a problem hiding this comment.
I'd pick up the codex change and just review the firebase config - if that project is setup then all good.
| if (value.deliveryMethod === 'pickup_point') { | ||
| if (!value.courier) { | ||
| return helpers.error('any.custom', { | ||
| message: | ||
| '"courier" is required when "deliveryMethod" is "pickup_point"', |
There was a problem hiding this comment.
We'll want to fix this one
There was a problem hiding this comment.
This will be an issue if cherry-test-project is not configured, I'd probably pull this out if it's not
3 participants
Strengthens cherry’s backend checkout foundations by tightening Stripe and Sendcloud safety, making non-production shipping mock-first, and enforcing clearer order and fulfilment contracts.
This PR fixes Stripe webhook verification, adds raw-body handling, introduces mock versus live shipping providers, supports Sendcloud service-point flows, verifies Sendcloud webhook signatures, rejects stale shipment updates, improves order lookup and validation, and adds focused tests for the new runtime safeguards.