[segunmailerlite]

Why?

• The Java SDK only had e-commerce models for automation triggers, so it couldn't reach shops, orders, variants or the revenue reports from the API - Ecommerce & Revenue project.

Changes

• New ml.ecommerce() with shops, products, variants, categories, customers, orders, carts, cart items, imports and settings, using the SDK's retriever/builder pattern.
• ecommerceReport() on campaigns, subscribers, automations and shops for revenue stats and attributed orders.

[mailerlite-kritika]

Confidence Score: 4/5

[Medium risk] The review ran only grep, never compiled the code or ran EcommerceTest, and its claim that the import, change-status, categories/multiple and variant endpoints match mailerlite-php rests on an unpinned repository tree whose files it does not show reading. The new tests assert the same paths and field names the code uses, so they cannot confirm the API contract, and the many new endpoints and JSON field names are unverified against the API. The risk is medium: the change adds a large new public …

Findings

Earlier findings (4 dismissed)

• src/main/java/com/mailerlite/sdk/automations/Automations.java:62 ecommerceEmailReport exposes orders() that throws NullPointerException · dismissed: thread resolved by segunmailerlite
• src/main/java/com/mailerlite/sdk/ecommerce/EcommerceReportRetriever.java:86 orders() throws NullPointerException when no orders endpoint was given · dismissed: thread resolved by segunmailerlite
• src/test/java/com/mailerlite/sdk/tests/EcommerceTest.java:67 Capturing client is left installed in the global factory after each test · dismissed: thread resolved by segunmailerlite
• src/main/java/com/mailerlite/sdk/susbcribers/Subscribers.java:83 New public method lacks Javadoc unlike its neighbours · dismissed: thread resolved by segunmailerlite

Summary

Adds e-commerce resources and revenue reports to the Java SDK

Adds ml.ecommerce(), which exposes shops, products, variants, categories, customers, orders, carts, cart items, imports and settings. It uses the existing retriever/builder pattern, a shared EcommerceApi helper and an EcommerceRequest base class that URL-encodes query strings. ecommerceReport() on campaigns, subscribers, automations and shops returns a shared EcommerceReportRetriever with stats() and orders(). Merging adds new public API; the existing e-commerce model classes gain fields and now extend ApiResource. The import, change-status, categories/multiple and variant sync endpoints match the official mailerlite-php SDK's Import.php, Product.php and ProductVariant.php.

Flow

%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["Caller uses ml.ecommerce()"] --> B["Pick resource: shops, products, orders, carts, etc."]
B --> C["Retriever or builder creates request"]
C --> D["EcommerceApi sends request with URL-encoded query"]
D --> E["MailerLite e-commerce API"]
F["ecommerceReport() on campaigns, subscribers, automations, shops"] --> G["EcommerceReportRetriever"]
G -->|"stats() or orders()"| D

Loading

What's good

• Consistent retriever/builder structure across all new resources, with the README documenting each one.
• EcommerceTest checks the HTTP method, URL and JSON body of every call without live traffic.
• CartBuilder.update and OrderBuilder.update drop nested fields (items, customer, cart) from update bodies so they aren't resent.

Review context · 1 source · 1/5 commands

• Sources:
  • https://redirect.github.com/mailerlite/mailerlite-php/tree/HEAD
• Commands run: grep

Reviewed in 2 parts, each by an agent of its own.

_{Reviews (5) · Last reviewed commit: "ECO-600" · claude-sonnet-5-5}

[johnkelesidis]

@segunmailerlite fix the conflicts please

[johnkelesidis]

Thanks for this! I reviewed the request side against the API source: paths, field names and import shapes (root arrays for products, categories and customers; {"orders": [...]} for orders) all match, and the full suite passes (120 tests, 0 failures).

The main blocker is the conversion_rate type in EcommerceStats, which makes both stats() calls throw on every real response. The tests don't catch it because every stubbed body is {}. The other comments are lower-severity bugs, duplication with existing models, and test gaps. Details inline.

[johnkelesidis]

Bug (blocking): stats() crashes on real responses. The campaign and automation e-commerce stats endpoints return conversion_rate as a percent string, not a number. CampaignEcommerceReportController appends '%', and MonthlyAutomationStatsRepository returns calculate_percentage(...) . '%' (or '0%'). Gson can't parse "2.5%" into a Double, so ml.campaigns().ecommerceReport(...).stats() and ml.automations().ecommerceReport(...).stats() throw JsonSyntaxException on every real response.

The existing automations/reports/EcommerceStats already types this field as String. Suggest the same here, plus a test that stubs a real body (for example "conversion_rate": "2.5%").

[johnkelesidis]

Bug: removeQueryParameter() does nothing on e-commerce retrievers. This class keeps its own queryParameters map and overrides add/get, but not ApiRequest.removeQueryParameter. So retriever().limit(10) followed by removeQueryParameter("limit") still sends ?limit=10.

More broadly, this forks query-string building (insertion order, key encoding, no bare ?) away from ApiRequest, which every other retriever still uses. Fixing ApiRequest.getQueryParameters() once (LinkedHashMap, encode keys, return "" when empty) would fix all callers and remove the need for this subclass.

[segunmailerlite]

Fixed: EcommerceRequest now overrides removeQueryParameter, with a test.

I tried the broader fix in ApiRequest.getQueryParameters() (LinkedHashMap, encoded keys, no bare ?), but it broke 33 existing tests. The VCR fixtures are keyed on the exact request URL, so the trailing ? and the param order change every cassette hash, and we can’t re-record them without a live account. I’d rather keep the fork in EcommerceRequest for now and move it into ApiRequest in a separate PR when the fixtures can be re-recorded.

[johnkelesidis]

Bug: query params leak between stats() and orders(). Both methods write their shop param (shop_id / ecommerce_shop_id) into the same persistent query map. Reusing a retriever sends the other endpoint's params:

r = ml.automations().ecommerceReport("8").shopId("1").timeFrame("all_time");
r.stats();
r.orders(); // ?time_frame=all_time&shop_id=1&ecommerce_shop_id=1

stats() also sends time_frame to /monthly-totals/ecommerce, which ignores it (that endpoint uses start_date/end_date). Suggest building the shop param per call instead of mutating the shared map.

[johnkelesidis]

Bug: update with the builder's usual setters returns 422. create() and update() send the same body, but EcommerceUpdateOrderRequest marks customer and cart as prohibited. So builder().customer(c).item(i).status("complete").update("9") fails. CartBuilder.update() has the same problem with items (EcommerceUpdateCartRequest). Either strip those fields on update or document that update only accepts a subset.

[segunmailerlite]

Fixed: order update() now strips customer and cart, and cart update() strips items, so the body matches the update request rules. I added tests for both that set those fields and check they aren’t sent.

[johnkelesidis]

Bug (minor): group dates are never parsed. parseDates() / postDeserialize() is never called on this nested Group, so shop.group.createdAt is always null.

[segunmailerlite]

Fixed: EcommerceShop now extends ApiResource and overrides parseDates() to also parse group. SingleShop and ShopsList call it from postDeserialize(). The new test checks that shops[0].group.createdAt gets parsed.

[johnkelesidis]

This duplicates the existing OrderActivity model for the same EcommerceReportOrderResource payload, but with subscriber/email as raw JsonElement and updated_at as an unparsed String. Reusing OrderActivity / OrderActivityList would give callers typed fields and parsed dates.

[johnkelesidis]

Dates (created_at, updated_at, completed_at) are raw Strings here and across the new models. The rest of the SDK uses xxxStr + @ParseDate Date (via ApiResource/postDeserialize), as in Group, Automation and OrderActivity. Worth matching that so callers get java.util.Date consistently.

[segunmailerlite]

Done: shop, category, product and order dates now use xxxStr + @ParseDate Date. The single and list responses call parseDates() from postDeserialize(), and nested objects get parsed too (shop group, product categories, the order’s customer and cart items). There’s a test that checks the order, product and category dates come back non-null.

[johnkelesidis]

This builds a new GsonBuilder/Gson with the same exclusion strategies on every request, duplicating JsonBody.toJson / SdkGson.plain(). Suggest return JsonBody.toJson(body);.

[segunmailerlite]

Done: EcommerceApi.json() now returns JsonBody.toJson(body). The new jsonWithout() helper for the update bodies uses SdkGson.plain() too.

[johnkelesidis]

assignProduct / unassignProduct (and ProductVariants.delete) return a bare MailerLiteResponse, but the API returns the updated product (EcommerceProductResource). Callers need an extra getSingle round-trip to see the result. Changing the return type to SingleProduct later would be a breaking change, so it's worth deciding now.

[johnkelesidis]

Test coverage. Every stubbed response body is {}, so these tests check the outgoing request (method, URL, body) but never parse a real response. That's why the conversion_rate crash isn't caught.

JaCoCo on this branch: about 70% of the PR's new lines and 55% of its new branches are covered. Notable gaps:

• OrderBuilder.create() is never called (only update is tested).
• Imports.orders() is never called. It's the one import with a different body shape ({"orders": [...]}).
• No response model (EcommerceStats, EcommerceOrder, EcommerceCustomer, EcommerceCart, ...) is ever populated.
• Most filters/sorts (ProductRetriever, OrderRetriever, CustomerRetriever, report timeFrameStart/timeFrameEnd) and about 40 builder setters are unexercised.

Suggest adding an order create(), an Imports.orders() call, and a few stats/list tests with realistic response bodies.

[johnkelesidis]

please check comments