diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c6f78be8..737ec87a 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -16,4 +16,6 @@ ## Bug Fixes +* Deal correctly with cancelled orders with no price or quantity + diff --git a/src/frequenz/client/electricity_trading/_types.py b/src/frequenz/client/electricity_trading/_types.py index 409dcdb2..a01745df 100644 --- a/src/frequenz/client/electricity_trading/_types.py +++ b/src/frequenz/client/electricity_trading/_types.py @@ -1010,8 +1010,16 @@ def from_pb(cls, order: electricity_trading_pb2.Order) -> Self: delivery_period=DeliveryPeriod.from_pb(order.delivery_period), type=OrderType.from_pb(order.type), side=MarketSide.from_pb(order.side), - price=Price.from_pb(order.price), - quantity=Power.from_pb(order.quantity), + price=( + Price.from_pb(order.price) + if order.HasField("price") + else Price(Decimal("NaN"), Currency.UNSPECIFIED) + ), + quantity=( + Power.from_pb(order.quantity) + if order.HasField("quantity") + else Power(Decimal("NaN")) + ), stop_price=( Price.from_pb(order.stop_price) if order.HasField("stop_price") @@ -1308,8 +1316,11 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self: Returns: OrderDetail object corresponding to the protobuf message. + + Raises: + ValueError: If the order price and quantity are not specified for a non-canceled order. """ - return cls( + od = cls( order_id=order_detail.order_id, order=Order.from_pb(order_detail.order), state_detail=StateDetail.from_pb(order_detail.state_detail), @@ -1321,6 +1332,19 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self: ), ) + # Only cancelled orders are allowed to have missing price or quantity + missing_price_or_quantity = ( + od.order.price.amount.is_nan() + or od.order.price.currency == Currency.UNSPECIFIED + or od.order.quantity.mw.is_nan() + ) + if missing_price_or_quantity and od.state_detail.state != OrderState.CANCELED: + raise ValueError( + f"Price and quantity must be specified for a non-canceled order (`{od}`)." + ) + + return od + def to_pb(self) -> electricity_trading_pb2.OrderDetail: """Convert an OrderDetail object to protobuf OrderDetail. diff --git a/src/frequenz/client/electricity_trading/cli/etrading.py b/src/frequenz/client/electricity_trading/cli/etrading.py index df1adf7f..f6369633 100644 --- a/src/frequenz/client/electricity_trading/cli/etrading.py +++ b/src/frequenz/client/electricity_trading/cli/etrading.py @@ -97,7 +97,7 @@ async def list_orders( """ client = Client(server_url=url, auth_key=key) - # print_header() + print_order_header() delivery_period = None # If delivery period is selected, list historical orders also @@ -200,18 +200,18 @@ async def cancel_order( def print_trade_header() -> None: """Print trade header in CSV format.""" header = ( - "public_trade_id, " - "execution_time, " - "delivery_period_start, " - "delivery_period_duration, " - "buy_delivery_area_code, " - "sell_delivery_area_code, " - "buy_delivery_area_code_type, " - "sell_delivery_area_code_type" - "quantity_mw, " - "price, " - "currency, " - "state, " + "public_trade_id," + "execution_time," + "delivery_period_start," + "delivery_period_duration," + "buy_delivery_area_code," + "sell_delivery_area_code," + "buy_delivery_area_code_type," + "sell_delivery_area_code_type," + "quantity_mw," + "currency," + "price," + "state " ) print(header) @@ -238,20 +238,20 @@ def print_trade(trade: PublicTrade) -> None: def print_order_header() -> None: """Print order header in CSV format.""" header = ( - "order_id, " - "create_time, " - "modification_time, " - "delivery_period_start, " - "delivery_period_duration" - "delivery_area_code, " - "delivery_area_code_type, " - "order_type, " - "quantity_mw, " - "open_quantity_mw, " - "side, " - "currency, " - "price, " - "state, " + "order_id," + "create_time," + "modification_time," + "delivery_period_start," + "delivery_period_duration," + "delivery_area_code," + "delivery_area_code_type," + "order_type," + "quantity_mw," + "open_quantity_mw," + "side," + "currency," + "price," + "state" ) print(header) diff --git a/tests/test_types.py b/tests/test_types.py index e30d6c6f..f216e78b 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -498,6 +498,29 @@ def test_order_detail_from_pb() -> None: ) +def test_order_detail_from_pb_missing_fields() -> None: + """Test the client order detail type conversion from protobuf with missing fields.""" + # Missing price + od_pb1 = electricity_trading_pb2.OrderDetail() + od_pb1.CopyFrom(ORDER_DETAIL_PB) + od_pb1.order.ClearField("price") + # Not allowed for active orders + with pytest.raises(ValueError): + OrderDetail.from_pb(od_pb1) + # But allowed for canceled orders + od_pb1.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED + OrderDetail.from_pb(od_pb1) + + # Missing quantity (same logic as above) + od_pb2 = electricity_trading_pb2.OrderDetail() + od_pb2.CopyFrom(ORDER_DETAIL_PB) + od_pb2.order.ClearField("quantity") + with pytest.raises(ValueError): + OrderDetail.from_pb(od_pb2) + od_pb2.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED + OrderDetail.from_pb(od_pb2) + + def test_order_detail_no_timezone_error() -> None: """Test that an order detail with inputs with no timezone raises a ValueError.""" with pytest.raises(ValueError):