Repository navigation
next-campaigns-create 1.3.0: multi-currency campaigns at fixed prices, post-purchase packages - #90
Merged
Merged
Conversation
…, post-purchase packages recommend could create a campaign in one currency only: it hard-coded additional_currencies to [], apply sent only the campaign-currency price, and verify priced carts in that currency alone. A merchant with a fixed local price per market had no way to carry those prices into a campaign through the engine. recommend --additional-currencies CODE,... offers a campaign in other currencies the store has enabled (never the campaign currency). --prices-file sets a fixed price for every package (variants.<id>, or packages.<key> for a package priced apart from its variant) and every shipping method in each one; without it the store's forex prices stand. The plan prints each currency's prices and its landed rows, priced by the same rule as the campaign currency's, and the plan hash covers them. apply PATCHes each package and shipping method with prices[] and recalculate_prices false right after its create, journalled so --resume sends it again. verify reads every currency's price back, reads the campaign as a funnel does with ?currency= for each one, and prices every landed row as a cart in every currency, failing a cart the Cart API priced in another currency. The update path treats each currency's price as its own field: adopt records the live ones, diff merges them three ways and sends changed ones as prices[] rows, adds or removes currencies with the campaign PATCH first, warns that a removed currency's prices are deleted everywhere (hand-set ones included), and gives a package or shipping method created in the same change set its fixed prices with a dependent PATCH. Resume and settle understand all of it. --post-purchase <variant>:<price> makes a full-price package sold after the order, with no voucher, labelled post-purchase rather than "Checkout bump"; verify prices it as an upsell cart in every currency. --upsell v:p:0 points at it. The gateway-group check now requires the group to list every campaign currency. A group whose currency list is empty has no active gateway (an active gateway with no currency list contributes every currency to that list), so it is refused with that reason rather than treated as accepting any currency. discover records is_default on each group.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous Review Summaries (3 snapshots, latest commit 9952669)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9952669)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 76e3f42)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 0df5bf5)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
Reviewed by minimax-m3 · Input: 124.5K · Output: 4.2K · Cached: 1.2M |
- merge_prices skips the three-way merge only for a currency the base plan did not fix. A price the base fixed and the store lost is kept lost (the merged plan stops fixing it) when the candidate left it alone, and a conflict when the candidate changed it. - The "price op leaves these currencies" warning names only currencies the campaign keeps, never one the same change set removes. - --post-purchase refuses a package an upsell voucher discounts, which would carry two prices on one upsell page; reusing a hero package now says the exit voucher also works there. - A prices file must price a package free in every currency or in none. - Additional-currency prices[] rows carry currency and price only. The endpoint already ignores a null price_recurring, but leaving the field out is safe either way. - apply's read-back names a missing row as "no EUR price row".
…rning (#90) - The fake store now leaves a prices[] field the row omits as it is, which is what the endpoint's per-currency upsert does (it writes a recurring price only from a non-null price_recurring). A new test seeds a subscription package with a EUR recurring price and proves an update of its EUR price keeps it. - The exit-voucher handoff note for a post-purchase hero package is written only when an exit voucher is planned, after it is built, and names its code. - merge_prices' docstring describes its three cases. A price the base fixed and the store lost, left alone by the candidate, now gets its own warning instead of "the store's null is kept".
… point (#90) - A currency price the store has lost (in a currency the campaign offers) is sent back at the candidate's value with a warning naming it, instead of being dropped from the merged plan. Dropping it made diff refuse a created run's merged plan as "partial" with a message that never mentioned the lost price. There is no store value to keep or to conflict with, and an unpriced package drops out of that currency's checkout. New test runs diff on a created run. - The fake store's comment says where it differs from the platform source (it writes an explicit null price_recurring; the source does not), and admin-api-contract.md lists clearing a recurring price as an open point that predates 1.3.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
next-campaigns-createcould build a campaign in one currency only.recommendhard-codedadditional_currencies: [],applysent each package and shipping method its campaign-currency price alone, andverifypriced carts in that one currency. A merchant who already sells at a fixed local price in each market had no way to carry those prices into a campaign through the engine, and the Admin token may only be used through it.Multi-currency, create path.
recommend --additional-currencies EUR,GBP,...offers the campaign in currencies the store has enabled (refused: the campaign currency itself, a repeat, a code no Market has made public).--prices-file <json>gives a fixed price per currency for every package (variants.<variant id>, orpackages.<package key>for a package priced apart from its variant, such as an upsell-page price) and every shipping method (shipping.<store code>). A gap, a stray package key, a campaign-currency row that disagrees with the command line, a fractional price in a zero-decimal currency, or a field the engine does not read is refused rather than skipped. Without a prices file the store's forex prices stand and the plan says so.planprints each currency's prices and its landed rows, priced by the same rule as the campaign currency's (relandon a copy priced in that currency, so rounding and one-price-per-row are checked in every currency); the plan hash covers the prices and so the rows.applysendsPATCH .../packages/{id}/(and.../shipping-methods/{id}/) withprices[]andrecalculate_prices: falseright after each create, journalled asprices_statusso--resumesends it again; a 200 that did not set the prices stops the run.verifychecks each object's price in each currency, readsGET /api/v1/campaigns/?currency=for every currency (the Cart API silently leaves out anything unpriced), and prices every landed row as a cart withcurrency, failing a response priced in another currency.Multi-currency, update path.
adoptrecords every live additional-currency price as the plan'sprices.<code>.diffmerges each currency three ways like any other field and sends changed ones asprices[]rows; adding or removing a currency is the campaign PATCH, sent first; a package or shipping method the change set creates gets its fixed prices from a dependent PATCH (depends_onits POST).diffwarns which objects keep the forex price an added currency gets, and that removing a currency deletes its prices on every package and shipping method, hand-set ones included. Resume and settle understand the new fields, including a currency removal that already landed.Post-purchase packages.
--post-purchase <variant_id>:<price>makes a full-price package sold after the order, with no voucher, labelled post-purchase instead of the "Checkout bump" that--bumpgave it.verifyprices it as an upsell cart (?upsell=true, no shipping, no code) in every currency.--upsell v:p:0is refused with a pointer to it.Gateway groups. The platform's gateway-group read lists every currency an active gateway in the group accepts, and a gateway with no currency list counts as accepting all of them, so an empty
available_currenciesmeans the group has no active gateway (the card method is listed for every group regardless).recommendnow refuses such a group with that reason, and requires the group to list the campaign currency and every additional currency: the Campaigns API checks only the campaign currency, but checkout charges through a gateway that accepts the order's currency.discoverrecords and printsis_default.scripts/campaign_admin.py:--additional-currencies,--prices-file,--post-purchase;load_prices_file,currency_coverage,plan_in_currency,currency_landed,prices_patch_body; the per-currency gate print, apply PATCH, verify rows, listing and carts; per-currency fields in the normalised snapshot, adopt, diff, resume and settle; the gateway-group rule.SKILL.md: write inventory row 29 (the apply PATCH) and rows 14, 17, 25; Phase 3 Markets and post-purchase; Phases 4 to 7; 15 Failure modes rows; invariants.references/admin-api-contract.mdgains an "Additional currencies" section and schema updates;references/update-path.mdgains the currency procedure;README.mda plain-language section.tests/test_multi_currency.py: 43 tests on a currency-aware fake store (forex fill on create, currency add and remove on a campaign PATCH, a currency-aware cart and listing). Synthetic catalogue and prices only.priceson plan objects,prices_statuson manifest entries).Not covered here: no multi-currency write has been sent to a live store by this engine (the shapes come from the published contract and the platform source, proven against the offline fake), checkout bumps still get no
verifycart, andedithas no per-currency operation (per-currency prices change through the update path).Checks run locally:
python3 -m unittest discover -s next-campaigns-create/tests(726 tests, OK),python3 -m unittest discover -s scripts/tests(44, OK),check_skill_versions.py --base origin/main,skill_catalog.py validate --check-readme,check_public_safety.py(no hits).