src/app/cancel.rs (1)

104-135: Refactor to eliminate code duplication in cancellation handling

The blocks of code handling order cancellation for creators (lines 104–135) and takers (lines 136–184) contain similar logic, such as updating the order status, canceling hold invoices, and sending notifications. Refactoring these blocks to extract common functionality into helper functions can improve maintainability and reduce repetition.

Also applies to: 136-184

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/app/cancel.rs (1)

136-186: ⚠️ Potential issue

Fix logical error: Notifications may not be sent due to premature status update.

When the taker cancels the order, the status is updated to Pending at line 173. The subsequent status checks at lines 145-147 and 159-161 will never be true because the status has already been changed, causing the creator to miss cancellation notifications.

Store the original status before updating it:

+        let original_status = order.status.clone();
         if let Ok(order_updated) = update_order_event(my_keys, Status::Pending, &order).await {
             let _ = order_updated.update(pool).await;
         }

         if order.kind == OrderKind::Buy.to_string()
-            && order.status == Status::WaitingBuyerInvoice.to_string()
+            && original_status == Status::WaitingBuyerInvoice.to_string()
         {
             // ... notification code ...
         }

         if order.kind == OrderKind::Sell.to_string()
-            && order.status == Status::WaitingPayment.to_string()
+            && original_status == Status::WaitingPayment.to_string()
         {
             // ... notification code ...
         }

src/app/cancel.rs (1)

91-102: Consider simplifying taker identification logic.

While the current implementation is correct, it can be more concise by using a ternary operation.

-        let taker_pubkey: String;
-        if seller_pubkey == &order.creator_pubkey {
-            taker_pubkey = buyer_pubkey.to_string();
-        } else {
-            taker_pubkey = seller_pubkey.to_string();
-        }
+        let taker_pubkey = if seller_pubkey == &order.creator_pubkey {
+            buyer_pubkey.to_string()
+        } else {
+            seller_pubkey.to_string()
+        };

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/app/cancel.rs (2)

91-102: Consider simplifying taker identification logic.

While the current implementation is correct, it could be more concise.

Consider this more idiomatic approach:

-        let taker_pubkey: String;
-        if seller_pubkey == &order.creator_pubkey {
-            taker_pubkey = buyer_pubkey.to_string();
-        } else {
-            taker_pubkey = seller_pubkey.to_string();
-        }
+        let taker_pubkey = if seller_pubkey == &order.creator_pubkey {
+            buyer_pubkey.to_string()
+        } else {
+            seller_pubkey.to_string()
+        };

---

144-168: Consider reducing code duplication in taker cancellation logic.

The price reset and order update logic is duplicated between Buy and Sell order handling.

Consider extracting the common logic:

+            fn reset_order_state(
+                order: &mut Order,
+                pool: &Pool<Sqlite>,
+                my_keys: &Keys,
+            ) -> Result<()> {
+                if order.price_from_api {
+                    order.amount = 0;
+                    order.fee = 0;
+                }
+                update_order_to_initial_state(pool, order.id, order.amount, order.fee).await?;
+                update_order_event(my_keys, Status::Pending, order).await?;
+                Ok(())
+            }
+
             if order.kind == OrderKind::Buy.to_string() {
                 if order.status == Status::WaitingBuyerInvoice.to_string() {
                     send_new_order_msg(
                         request_id,
                         Some(order.id),
                         Action::Canceled,
                         None,
                         &creator_pubkey,
                         None,
                     )
                     .await;
                 }
-                if order.price_from_api {
-                    order.amount = 0;
-                    order.fee = 0;
-                }
                 edit_seller_pubkey_order(pool, order.id, None).await?;
                 edit_master_seller_pubkey_order(pool, order.id, None).await?;
-                update_order_to_initial_state(pool, order.id, order.amount, order.fee).await?;
-                update_order_event(my_keys, Status::Pending, &order).await?;
+                reset_order_state(&mut order, pool, my_keys)?;
                 info!(
                     "{}: Canceled order Id {} republishing order",
                     buyer_pubkey, order.id
                 );
             }

             if order.kind == OrderKind::Sell.to_string() {
                 if order.status == Status::WaitingPayment.to_string() {
                     send_new_order_msg(
                         request_id,
                         Some(order.id),
                         Action::Canceled,
                         None,
                         &creator_pubkey,
                         None,
                     )
                     .await;
                 }
-                if order.price_from_api {
-                    order.amount = 0;
-                    order.fee = 0;
-                }
                 edit_buyer_pubkey_order(pool, order.id, None).await?;
                 edit_master_buyer_pubkey_order(pool, order.id, None).await?;
-                update_order_to_initial_state(pool, order.id, order.amount, order.fee).await?;
-                update_order_event(my_keys, Status::Pending, &order).await?;
+                reset_order_state(&mut order, pool, my_keys)?;
                 info!(
                     "{}: Canceled order Id {} republishing order",
                     buyer_pubkey, order.id
                 );
             }

Also applies to: 170-194

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro