Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2225 +/- ##
==========================================
+ Coverage 36.89% 37.08% +0.18%
==========================================
Files 959 905 -54
Lines 62209 60332 -1877
Branches 3076 2958 -118
==========================================
- Hits 22953 22372 -581
+ Misses 38887 37620 -1267
+ Partials 369 340 -29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Draft app-side updates to support Saleor’s POC for skipping/deferring calculateTaxes delivery, and to adjust SMTP app fallback sender email derivation to be tenant-based rather than fully configured.
Changes:
- SMTP app: replace fallback sender email env var with a sender domain and derive the sender email from
saleorApiUrl. - AvaTax app: update
CalculateTaxessubscription and webhook handlers to the newcalculateTaxes(deferIf: ...)shape. - Add tests + changeset entry reflecting the SMTP fallback behavior change.
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/smtp/turbo.json | Updates Turbo env var list to use FALLBACK_SMTP_SENDER_DOMAIN. |
| apps/smtp/src/modules/smtp/configuration/smtp-config-schema.ts | Changes fallback schema from senderEmail to senderDomain. |
| apps/smtp/src/modules/saleor-fallback-behavior/fallback-sender-email.ts | Introduces derived fallback sender email generator from saleorApiUrl. |
| apps/smtp/src/modules/saleor-fallback-behavior/fallback-sender-email.test.ts | Adds unit tests for fallback sender email derivation and error cases. |
| apps/smtp/src/modules/event-handlers/use-case/send-event-messages.use-case.ts | Uses derived fallback sender email when sending via fallback config. |
| apps/smtp/src/modules/event-handlers/use-case/send-event-messages.use-case.test.ts | Updates fallback tests for senderDomain and derived senderEmail; adds invalid-URL test. |
| apps/smtp/src/env.ts | Renames env var to FALLBACK_SMTP_SENDER_DOMAIN. |
| apps/smtp/.env.example | Updates example env var name and layout. |
| apps/avatax/src/modules/webhooks/definitions/order-calculate-taxes.ts | Adjusts webhook payload typing for new calculateTaxes subscription structure. |
| apps/avatax/src/modules/webhooks/definitions/checkout-calculate-taxes.ts | Adjusts webhook payload typing for new calculateTaxes subscription structure. |
| apps/avatax/src/app/api/webhooks/order-calculate-taxes/route.ts | Reads calculateTaxes payload from the updated webhook payload structure. |
| apps/avatax/src/app/api/webhooks/checkout-calculate-taxes/route.ts | Reads calculateTaxes payload from the updated webhook payload structure. |
| apps/avatax/graphql/subscriptions/CalculateTaxes.graphql | Switches from event { ... } to calculateTaxes(deferIf: ADDRESS_MISSING) { ... }. |
| apps/avatax/graphql/fragments/CalculateTaxesEvent.graphql | Updates fragment type condition to CalculateTaxes and includes additional metadata fields. |
| apps/avatax/graphql.config.ts | Adds scalar mappings for Hour and PositiveInt. |
| .changeset/calm-spoons-judge.md | Adds changeset for SMTP fallback sender email behavior change. |
Comments suppressed due to low confidence (1)
apps/avatax/src/app/api/webhooks/order-calculate-taxes/route.ts:131
SubscriptionPayloadErrorCheckertreats GraphQL errors on pathevent.taxBase.sourceObject.useras handled. With the new subscription root field (calculateTaxes(...)), GraphQL error paths will likely start withcalculateTaxesinstead ofevent, so these previously-handled errors may be misclassified and captured as unhandled. Update the checker (or its handled path logic) to account for the new root field name.
subscriptionErrorChecker.checkPayload(payload);
loggerContext.set("channelSlug", channelSlug);
loggerContext.set("orderId", orderId);
| subscriptionErrorChecker.checkPayload(payload); | ||
|
|
||
| loggerContext.set(ObservabilityAttributes.CHANNEL_SLUG, ctx.payload.taxBase.channel.slug); | ||
| loggerContext.set(ObservabilityAttributes.CHECKOUT_ID, ctx.payload.taxBase.sourceObject.id); | ||
| loggerContext.set(ObservabilityAttributes.CHANNEL_SLUG, payload.taxBase.channel.slug); |
There was a problem hiding this comment.
SubscriptionPayloadErrorChecker treats GraphQL errors on path event.taxBase.sourceObject.user as handled. With the new subscription root field (calculateTaxes(...)), GraphQL error paths will likely start with calculateTaxes instead of event, so these previously-handled errors may be misclassified and captured as unhandled. Update the checker (or its handled path logic) to account for the new root field name.
| subscription CalculateTaxes { | ||
| event { | ||
| ...CalculateTaxesEvent | ||
| calculateTaxes(deferIf: ADDRESS_MISSING) { | ||
| taxBase { | ||
| ...TaxBase | ||
| } | ||
| recipient { | ||
| privateMetadata { | ||
| key | ||
| value | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The subscription no longer selects __typename/version/issuedAt, but the webhook handlers and SubscriptionPayloadErrorChecker rely on these fields for observability and error reporting. Either add these fields back to the selection set or reuse ...CalculateTaxesEvent so the runtime payload matches the expected CalculateTaxesPayload shape.
draft impl for saleor/saleor#18784 on app side