Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cancel_status param to the cancel order action #1326

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion api/logics.py
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,17 @@ def is_penalized(user):
return False, None

@classmethod
def cancel_order(cls, order, user, state=None):
def cancel_order(cls, order, user, cancel_status=None):
aftermath2 marked this conversation as resolved.
Show resolved Hide resolved
# If cancel status is specified, do no cancel the order
# if it is not the correct one.
# This prevents the client from cancelling an order that
# recently changed status.
if cancel_status is not None:
if order.status != cancel_status:
return False, {
"bad_request": f"Current order status is {order.status}, not {cancel_status}."
}

# Do not change order status if an is in order
# any of these status
do_not_cancel = [
Expand Down
4 changes: 4 additions & 0 deletions api/oas_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ class OrderViewSchema:
- `17` - Maker lost dispute
- `18` - Taker lost dispute

The client can use `cancel_status` to cancel the order only
if it is in the specified status. The server will
return an error without cancelling the trade otherwise.

Note that there are penalties involved for cancelling a order
mid-trade so use this action carefully:

Expand Down
7 changes: 7 additions & 0 deletions api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,13 @@ class UpdateOrderSerializer(serializers.Serializer):
mining_fee_rate = serializers.DecimalField(
max_digits=6, decimal_places=3, allow_null=True, required=False, default=None
)
cancel_status = serializers.ChoiceField(
choices=Order.Status.choices,
allow_null=True,
allow_blank=True,
default=None,
help_text="Status the order should have for it to be cancelled.",
)


class ClaimRewardSerializer(serializers.Serializer):
Expand Down
3 changes: 2 additions & 1 deletion api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ def take_update_confirm_dispute_cancel(self, request, format=None):
mining_fee_rate = serializer.data.get("mining_fee_rate")
statement = serializer.data.get("statement")
rating = serializer.data.get("rating")
cancel_status = serializer.data.get("cancel_status")

# 1) If action is take, it is a taker request!
if action == "take":
Expand Down Expand Up @@ -593,7 +594,7 @@ def take_update_confirm_dispute_cancel(self, request, format=None):

# 3) If action is cancel
elif action == "cancel":
valid, context = Logics.cancel_order(order, request.user)
valid, context = Logics.cancel_order(order, request.user, cancel_status)
if not valid:
return Response(context, status.HTTP_400_BAD_REQUEST)

Expand Down
3 changes: 3 additions & 0 deletions docs/assets/schemas/api-latest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1996,6 +1996,9 @@ components:
format: decimal
pattern: ^-?\d{0,3}(?:\.\d{0,3})?$
nullable: true
cancel_status:
allOf:
- $ref: '#/components/schemas/StatusEnum'
required:
- action
Version:
Expand Down
11 changes: 10 additions & 1 deletion frontend/src/components/TradeBox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element =>
mining_fee_rate?: number;
statement?: string;
rating?: number;
cancel_status?: number;
}

const renewOrder = function (): void {
Expand Down Expand Up @@ -188,6 +189,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element =>
mining_fee_rate,
statement,
rating,
cancel_status
}: SubmitActionProps): void {
const slot = garage.getSlot();

Expand All @@ -201,6 +203,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element =>
mining_fee_rate,
statement,
rating,
cancel_status
})
.then((data: Order) => {
setOpen(closeAll);
Expand All @@ -222,8 +225,14 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element =>
};

const cancel = function (): void {
const order = garage.getSlot()?.order;
const noConfirmation = Boolean(order?.is_maker && [0, 1, 2].includes(order?.status));

setLoadingButtons({ ...noLoadingButtons, cancel: true });
submitAction({ action: 'cancel' });
submitAction({
action: 'cancel',
cancel_status: noConfirmation ? order?.status : undefined
});
};

const openDispute = function (): void {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/models/Order.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface SubmitActionProps {
statement?: string;
rating?: number;
amount?: number;
cancel_status?: number;
}

export interface TradeRobotSummary {
Expand Down
3 changes: 3 additions & 0 deletions robosats/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@
}
},
"REDOC_DIST": "SIDECAR",
"ENUM_NAME_OVERRIDES": {
"StatusEnum": "api.models.order.Order.Status",
}
}


Expand Down
52 changes: 52 additions & 0 deletions tests/test_trade_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,58 @@ def test_cancel_public_order(self):
f"❌ Hey {maker_nick}, you have cancelled your public order with ID {trade.order_id}.",
)

def test_cancel_order_cancel_status(self):
"""
Tests the cancellation of a public order using cancel_status.
"""
trade = Trade(self.client)
trade.publish_order()
data = trade.response.json()

self.assertEqual(trade.response.status_code, 200)
self.assertResponse(trade.response)

self.assertEqual(data["status_message"], Order.Status(Order.Status.WFB).label)

# Cancel order if the order status is waiting for maker bond
trade.cancel_order(cancel_status=Order.Status.WFB)
data = trade.response.json()

self.assertEqual(trade.response.status_code, 400)
self.assertResponse(trade.response)

self.assertEqual(
data["bad_request"], "This order has been cancelled by the maker"
)

def test_cancel_order_different_cancel_status(self):
"""
Tests the cancellation of a paused order with a different cancel_status.
"""
trade = Trade(self.client)
trade.get_order()
data = trade.response.json()

self.assertEqual(trade.response.status_code, 200)
self.assertResponse(trade.response)

self.assertEqual(data["status_message"], Order.Status(Order.Status.WFB).label)

# Try to cancel order if it is public
trade.cancel_order(cancel_status=Order.Status.PUB)
data = trade.response.json()

self.assertEqual(trade.response.status_code, 400)
self.assertResponse(trade.response)

self.assertEqual(
data["bad_request"],
f"Current order status is {Order.Status.WFB}, not {Order.Status.PUB}."
)

# Cancel order to avoid leaving pending HTLCs after a successful test
trade.cancel_order()

def test_collaborative_cancel_order_in_chat(self):
"""
Tests the collaborative cancellation of an order in the chat state
Expand Down
4 changes: 3 additions & 1 deletion tests/utils/trade.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ def get_order(self, robot_index=1, first_encounter=False):
self.response = self.client.get(path + params, **headers)

@patch("api.tasks.send_notification.delay", send_notification)
def cancel_order(self, robot_index=1):
def cancel_order(self, robot_index=1, cancel_status=None):
path = reverse("order")
params = f"?order_id={self.order_id}"
headers = self.get_robot_auth(robot_index)
body = {"action": "cancel"}
if cancel_status is not None:
body.update({"cancel_status": cancel_status})
aftermath2 marked this conversation as resolved.
Show resolved Hide resolved
self.response = self.client.post(path + params, body, **headers)

@patch("api.tasks.send_notification.delay", send_notification)
Expand Down