fix(workflow-engine-redis,core-flows): support top-level redis options and allow adding addresses to orders without existing country code - #16736
Conversation
…s and allow adding addresses to orders without existing country code (fixes medusajs#16697, medusajs#16636)
🦋 Changeset detectedLatest commit: c79fe7e The changes in this PR will be included in the next version bump. This PR includes changesets to release 83 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the contribution! We need more information before reviewing this further. The PR addresses two separate issues: (1) adding support for top-level redisUrl/options in the workflow-engine-redis loader so that configurations matching the declared types work at runtime without nesting under redis, and (2) fixing a false positive error in updateOrderValidationStep that prevented adding a shipping or billing address to an order that had no existing country code. The implementation extracts a reusable throwIfCountryCodeChanged helper and adjusts the guard to only reject changes when an existing country_code is present. Unit tests are included for both fixes. The PR body does not follow the required template — the What, Why, How, and Testing sections are all absent. Please ask the author to resubmit using the standard PR template before the code is reviewed further. Heads up: PR #16698 (opened 2026-09-02) and PR #16699 (opened 2026-09-02) both reference issue #16697 and were opened earlier than this PR; if either is merged first, this PR may be closed as a duplicate for that fix. Similarly, PR #16637 (opened 2026-08-27), PR #16659 (opened 2026-08-30), and PR #16689 (opened 2026-09-02) reference issue #16636 and were opened earlier. Triggered by: new PR opened |
|
Thanks for the contribution! A few items need to be addressed before this can move forward: The author filled in all four PR template sections (What, Why, How, Testing) since the previous review, so that earlier issue is resolved. The logic in the new throwIfCountryCodeChanged helper is correct and properly fixes the false-positive validation error. The Redis loader fallback is also correct. However, the patch for update-order.ts has a structural error: the line } from "../steps" is marked as a removed line (-) in the diff rather than being kept as context. This closing brace belongs to the import { block at line 23 that imports five named exports from "../steps". After the patch is applied that block is unclosed, producing a TypeScript syntax error that would prevent the file from compiling. The fix is straightforward: keep } from "../steps" as a context line in the patch and only remove the import { throwIfOrderIsCancelled } from "../utils/order-validation" line.
Triggered by: new commit pushed |
|
Thanks for the contribution! A few items need to be addressed before this can move forward: The author filled in all four PR template sections after the first review, and both fixes are logically sound: the throwIfCountryCodeChanged helper correctly permits adding an address when no country code previously existed, and the Redis loader fallback correctly handles top-level options via options?.redis ?? options. Unit tests are included for both. However, the patch for update-order.ts still contains the same structural error that was flagged in the previous bot review: the } from "../steps" closing brace (line 29 in the base file) is marked as a removed line (-) in the diff rather than kept as context. After the patch is applied the multi-export import block for ../steps is left unclosed, producing a TypeScript syntax error that prevents the file from compiling. The author needs to regenerate the patch so that only the single-line import { throwIfOrderIsCancelled } from "../utils/order-validation" is removed, while } from "../steps" is left as a context (unchanged) line.
Triggered by: PR description updated |
|
Thanks for the contribution! Initial automated review looks good. The author has resolved the blocking issue flagged in the two previous bot reviews: the } from "../steps" closing brace in update-order.ts is now correctly kept as a context line in the diff rather than being removed. Both fixes are logically sound. The throwIfCountryCodeChanged helper guards on existingAddress?.country_code before comparing values, so it correctly skips the check when no prior country code exists, resolving the false-positive error when adding an address to an order that has none. The Redis loader fallback (options?.redis ?? options) correctly handles top-level redisUrl/redisOptions without the nested redis key. The PR template is fully filled, a changeset with the correct patch bump and fix(...) message is included, and unit tests cover the new helper and the validation step. Notes (non-blocking): - packages/core/core-flows/src/order/workflows/tests/update-order.spec.ts (first it block, line 5): const step = (updateOrderValidationStep as any).step is declared but never used — this is dead code that can be removed. Triggered by: new commit pushed |
|
Checked this again. The latest bot review only has a non-blocking note; the syntax issue is already fixed and the changeset/template/tests are in place. Since #16761 covers the same areas plus the fulfillment fix, I’ll leave this one as-is unless maintainers prefer the smaller two-fix PR. |
What
redisUrlandredisOptionsin@medusajs/workflow-engine-redisservice loader.updateOrderValidationStep(@medusajs/core-flows) to allow adding shipping/billing addresses when an order does not already have an existing country code.Why
redisUrlandredisOptions, but the service loader only checkedoptions.redis.optionsandoptions.redis.url, failing runtime initialization when passed top-level options.How
packages/workflow-engine-redis/src/loaders/index.tsto check both top-level and nested Redis options.packages/core-flows/src/order/steps/update-order-validation.tsto allow new address assignment when prior country code is absent.packages/workflow-engine-redis/src/loaders/__tests__/redis-loader.spec.tsandpackages/core-flows/src/order/steps/__tests__/update-order-validation.spec.ts.Testing
.changeset/eighty-sheep-burn.md.