From dd6563f9943dd336c9fe2b046cb7b0c96174cfb9 Mon Sep 17 00:00:00 2001 From: Asmin963 Date: Sun, 30 Aug 2026 15:35:40 +0300 Subject: [PATCH] Replying to a review no longer leaves a rating of your own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- funpaybotengine/client/bot.py | 24 +++++++- .../exceptions/action_exceptions.py | 25 +++++++- funpaybotengine/methods/review.py | 53 ++++++++++++++--- tests/test_review.py | 59 +++++++++++++++++++ 4 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 tests/test_review.py diff --git a/funpaybotengine/client/bot.py b/funpaybotengine/client/bot.py index c8d902a..7fd912d 100644 --- a/funpaybotengine/client/bot.py +++ b/funpaybotengine/client/bot.py @@ -444,9 +444,29 @@ async def send_message( async def refund(self, order_id: str) -> bool: return (await Refund(order_id=order_id).execute(self)).response_obj - async def review(self, order_id: str, text: str, rating: Literal[0, 1, 2, 3, 4, 5]) -> bool: + async def review( + self, + order_id: str, + text: str, + rating: Literal[1, 2, 3, 4, 5] | None = None, + reply_review: bool = False, + ) -> bool: + """Leave or edit a review as the buyer, or reply to one as the seller. + + One endpoint serves both, and who you are on the order is what + separates them: the buyer rates, the seller replies and the site posts + ``rating=`` empty. ``reply_review`` states which act this is, because + a reply built with no rating would otherwise post ``0`` -- accepted by + the site, and recorded as the sender's rating on that order. + + Raises ``ReviewRatingRequired`` when neither is stated. Sending + where a review already exists **overwrites** it; there is no separate + edit call. + """ return ( - await Review(order_id=order_id, text=text, rating=rating).execute(self) + await Review( + order_id=order_id, text=text, rating=rating, reply_review=reply_review + ).execute(self) ).response_obj async def delete_review(self, order_id: str) -> bool: diff --git a/funpaybotengine/exceptions/action_exceptions.py b/funpaybotengine/exceptions/action_exceptions.py index 419312f..f471985 100644 --- a/funpaybotengine/exceptions/action_exceptions.py +++ b/funpaybotengine/exceptions/action_exceptions.py @@ -1,7 +1,7 @@ from __future__ import annotations -__all__ = ['RefundError', 'RaiseOffersError'] +__all__ = ['RefundError', 'RaiseOffersError', 'ReviewRatingRequiredError'] import re from .base import FunPayBotEngineError @@ -17,6 +17,29 @@ def __str__(self) -> str: return self.message +class ReviewRatingRequiredError(FunPayBotEngineError): + """Leaving a review needs a rating; replying to one does not. + + orders/review serves both, and what separates them is not the endpoint + but who you are on the order: as the buyer you rate, as the seller you + reply, and the site posts rating= empty. + + Raised at construction, before any request. Without it the two collapse + into one: a reply built with no rating would post rating=0, which the + site accepts and records as the sender's rating on that order. + """ + + def __init__(self, order_id: str) -> None: + super().__init__() + self.order_id = order_id + + def __str__(self) -> str: + return ( + f'{self.order_id}: rating is required to leave a review; ' + f'to reply to one as the seller pass reply_review=True' + ) + + class RaiseOffersError(FunPayBotEngineError): def __init__( self, diff --git a/funpaybotengine/methods/review.py b/funpaybotengine/methods/review.py index 941fcd2..cd439e4 100644 --- a/funpaybotengine/methods/review.py +++ b/funpaybotengine/methods/review.py @@ -6,11 +6,12 @@ from typing import TYPE_CHECKING, Any, Literal -from pydantic import BaseModel +from pydantic import BaseModel, model_validator from funpaybotengine.types.enums import Language from funpaybotengine.methods.base import FunPayMethod from funpaybotengine.client.session.http_methods import HTTPMethod +from funpaybotengine.exceptions.action_exceptions import ReviewRatingRequiredError if TYPE_CHECKING: @@ -20,9 +21,26 @@ class Review(FunPayMethod[bool], BaseModel): """ - Leave / Edit a review / reply to review (``https://funpay.com/orders/review``). - - Returns ``True``. + Leave / edit a review, or reply to one (``https://funpay.com/orders/review``). + + One endpoint, two different acts, and what separates them is who you are on + the order -- not the URL. As the **buyer** you leave or edit a review, and a + rating is part of it. As the **seller** you reply, and the site posts + ``rating=`` empty; the rating on that order is the buyer's and is not yours + to set. + + ``reply_review`` says which act this is. It exists because the two are + otherwise indistinguishable at the call site: a reply built with no rating + would fall back to ``0``, which the site accepts and records as YOUR rating. + Omitting the rating without saying it is a reply raises + :class:`~funpaybotengine.exceptions.ReviewRatingRequiredError` at construction, + before any request goes out. + + Sending a review where one already exists **overwrites** it. There is no + separate edit call, and none for replying twice. + + Returns ``True``. **The site's answer is not parsed** -- this is not + evidence of success, only of the absence of an exception. """ order_id: str @@ -31,15 +49,25 @@ class Review(FunPayMethod[bool], BaseModel): text: str """Review text.""" - rating: Literal[0, 1, 2, 3, 4, 5] - """Review rating.""" + rating: Literal[1, 2, 3, 4, 5] | None = None + """Review rating. ``None`` for a seller's reply. + + ``0`` is gone from the accepted values on purpose: it used to be the way to + say "no rating", and it was indistinguishable from a real one at every call + site. Absence is now spelled ``None``, and it is only legal together with + ``reply_review=True``. + """ + + reply_review: bool = False + """This is a seller's REPLY, not a review. Sends ``rating=`` empty.""" def __init__( self, order_id: str, text: str, - rating: Literal[0, 1, 2, 3, 4, 5], + rating: Literal[1, 2, 3, 4, 5] | None = None, locale: Language | None = None, + reply_review: bool = False, ): super().__init__( method=HTTPMethod.POST, @@ -50,8 +78,15 @@ def __init__( order_id=order_id, text=text, rating=rating, + reply_review=reply_review, ) + @model_validator(mode='after') + def _rating_matches_the_act(self) -> Review: + if not self.reply_review and self.rating is None: + raise ReviewRatingRequiredError(self.order_id) + return self + async def parse_result(self, response: RawResponse[Any]) -> bool: return True @@ -62,7 +97,9 @@ async def transform_result(self, parsing_result: Any, response: RawResponse[Any] async def make_data(method: Review, bot: Bot) -> dict[str, Any]: return { 'orderId': method.order_id, - 'rating': method.rating or '', + # Empty string, not ``0``: this is the field as the site sends it for a + # seller's reply, and ``0`` would be a rating. + 'rating': '' if method.rating is None else method.rating, 'text': method.text, 'authorId': bot.userid, } diff --git a/tests/test_review.py b/tests/test_review.py new file mode 100644 index 0000000..b0d7d42 --- /dev/null +++ b/tests/test_review.py @@ -0,0 +1,59 @@ +"""`orders/review` serves two different acts; the tests keep them apart. + +As the buyer you leave or edit a review and a rating is part of it. As the +seller you reply, and the site posts ``rating=`` empty -- the rating on that +order is the buyer's. + +The expected reply body is not invented: it is the form captured from the +site's own request when a seller replies. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pydantic import ValidationError +from funpaybotengine.exceptions import ReviewRatingRequiredError +from funpaybotengine.methods.review import Review + + +class _Bot: + userid = 8331834 + + +async def body(method: Review) -> dict[str, Any]: + return await method.data(method, _Bot()) # type: ignore[arg-type,no-any-return] + + +@pytest.mark.asyncio +async def test_a_seller_reply_sends_an_empty_rating() -> None: + assert await body(Review(order_id='JDPEUYUS', text='thanks', reply_review=True)) == { + 'orderId': 'JDPEUYUS', + 'rating': '', + 'text': 'thanks', + 'authorId': 8331834, + } + + +@pytest.mark.asyncio +async def test_a_review_still_carries_its_rating() -> None: + assert (await body(Review(order_id='X', text='t', rating=5)))['rating'] == 5 + + +def test_omitting_the_rating_without_saying_it_is_a_reply_is_refused() -> None: + """The whole point of the flag. + + Without it a reply is built with no rating, falls back to ``0``, and the + site records that as the sender's rating on the order. Nothing raises, and + it is visible only on the order page afterwards. + """ + with pytest.raises(ReviewRatingRequiredError) as caught: + Review(order_id='X', text='t') + assert caught.value.order_id == 'X' + + +def test_zero_is_no_longer_a_way_to_say_no_rating() -> None: + """It used to be, and at a call site it read as a real rating.""" + with pytest.raises(ValidationError): + Review(order_id='X', text='t', rating=0) # type: ignore[arg-type]