Skip to content

Make the rating optional so a seller reply does not leave one - #43

Open
zlexdev wants to merge 1 commit into
funpayhub:devfrom
zlexdev:feat/review-reply
Open

Make the rating optional so a seller reply does not leave one#43
zlexdev wants to merge 1 commit into
funpayhub:devfrom
zlexdev:feat/review-reply

Conversation

@zlexdev

@zlexdev zlexdev commented Aug 30, 2026

Copy link
Copy Markdown

orders/review serves two different acts and the endpoint does not tell them apart: as the buyer you leave or edit a review with a rating, as the seller you reply and the site posts rating= empty.

Review models only the first. rating is a required Literal[0, 1, 2, 3, 4, 5], and rating or '' in make_data makes 0 the only way to express "none".

So a seller reply has to be built as rating=0. The site accepts it and records it as the sender's rating on that order. Nothing raises, and it is visible only on the order page afterwards.

Repro

On dev:

await bot.review(order_id="...", text="thanks", rating=0)

posts rating= empty — indistinguishable at the call site from meaning to rate, because 0 is also the value you would pass if you did.

What this changes

  • rating becomes optional and narrows to 1..5;
  • reply_review: bool = False states that this is a reply;
  • omitting the rating without saying so raises ReviewRatingRequiredError at construction, before a request goes out.

The request body is unchanged: a reply still posts rating= empty, a review still posts its number. Verified field by field against a request captured from the site.

tests/ is new — the Makefile already points TESTS at it. Four tests; test_omitting_the_rating_without_saying_it_is_a_reply_is_refused is the one that cannot pass on the parent commit.

Named ReviewRatingRequiredError rather than ReviewRatingRequired to match RefundError / RaiseOffersError and ruff's N818.

Not included

ruff check reports 13 pre-existing findings elsewhere in the tree. They are left alone so this diff stays about one thing.

`orders/review` serves two different acts and the endpoint does not tell them
apart: as the buyer you leave or edit a review with a rating, as the seller you
reply and the site posts `rating=` empty. `Review` modelled only the first —
`rating` was a required `Literal[0, 1, 2, 3, 4, 5]`, and `rating or ''` in
`make_data` made `0` the only way to express "none".

So a seller reply had to be built as `rating=0`. The site accepts it and records
it as the sender's rating on that order. Nothing raises, and it shows up only on
the order page afterwards.

Repro, against the current dev:

    await bot.review(order_id="...", text="thanks", rating=0)

posts `rating=` empty — indistinguishable at the call site from meaning to rate.

What changes: `rating` becomes optional and narrows to `1..5`; `reply_review`
states that this is a reply. Omitting the rating without saying so raises
`ReviewRatingRequiredError` at construction, before a request goes out, so the
two acts cannot collapse into one by accident. The reply body is unchanged and
matches a request captured from the site.

`tests/` is new — the Makefile already points `TESTS` at it. Four tests, and the
third fails on the parent commit for the reason above.
@zlexdev
zlexdev requested a review from qvvonk as a code owner August 30, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant