Fix retransmission on repeated Tx confirmation

The previous implementation could trigger an unnecessary retransmission
if a repeated Tx confirmation (with the same region confirmed before)
was sent.

server/order_helpers.py

@@ -122,8 +122,7 @@ def synthesize_presumed_rx_confirmations(order):
     # 2- If those regions are part of the requested regions by the order
     for region in [Regions.t11n_afr, Regions.t11n_eu]:
         if region.value in order_region_numbers:
-            add_confirmation_if_not_present(RxConfirmation, order, region,
+            add_rx_confirmation_if_not_present(order, region, presumed)
-                                            presumed)
 
 
 def sent_criteria_met(order):
@@ -212,15 +211,29 @@ def confirmation_exists(confirmations_table, order_id, region_id):
     return False
 
 
-def add_confirmation_if_not_present(confirmations_table,
+def add_tx_confirmation_if_not_present(order, region_number, presumed=False):
-                                    order,
-                                    region_number,
-                                    presumed=False):
     region_id = region_number_to_id(region_number)
-    if not confirmation_exists(confirmations_table, order.id, region_id):
+    if not confirmation_exists(TxConfirmation, order.id, region_id):
-        new_confirmation = confirmations_table(order_id=order.id,
+        # A Tx confirmation indicates that at least one Tx host finished
-                                               region_id=region_id,
+        # transmitting the order. At this point, the expectation is that the
-                                               presumed=presumed)
+        # other Tx hosts complete their transmissions soon. In the meantime,
+        # change the order state from transmitting to confirming so that other
+        # pending orders can be unblocked.
+        if order.status == OrderStatus.transmitting.value:
+            order.status = OrderStatus.confirming.value
+        new_confirmation = TxConfirmation(order_id=order.id,
+                                          region_id=region_id,
+                                          presumed=presumed)
+        db.session.add(new_confirmation)
+        db.session.commit()
+
+
+def add_rx_confirmation_if_not_present(order, region_number, presumed=False):
+    region_id = region_number_to_id(region_number)
+    if not confirmation_exists(RxConfirmation, order.id, region_id):
+        new_confirmation = RxConfirmation(order_id=order.id,
+                                          region_id=region_id,
+                                          presumed=presumed)
         db.session.add(new_confirmation)
         db.session.commit()
 

server/orders.py

@@ -14,7 +14,7 @@ from constants import CHANNEL_INFO, ORDER_FETCH_STATES, OrderStatus
 from database import db
 from error import get_http_error_resp
 from invoice_helpers import new_invoice, pay_invoice
-from models import Order, RxConfirmation, TxConfirmation, TxRetry
+from models import Order, TxRetry
 from regions import region_number_list_to_code
 from schemas import admin_order_schema, order_schema, orders_schema,\
     order_upload_req_schema, order_bump_schema,\
@@ -380,20 +380,11 @@ class TxConfirmationResource(Resource):
         if not order:
             return get_http_error_resp('SEQUENCE_NUMBER_NOT_FOUND', tx_seq_num)
 
-        # A Tx confirmation indicates that at least one Tx host finished
-        # transmitting the order. At this point, the other Tx hosts should
-        # complete the order soon. In the meantime, change the order state
-        # from transmitting to confirming so that other pending orders
-        # can be unblocked.
         last_status = order.status
-        if order.status == OrderStatus.transmitting.value:
-            order.status = OrderStatus.confirming.value
-            db.session.commit()
-
         regions_in_request = json.loads(args['regions'])
         for region_number in regions_in_request:
-            order_helpers.add_confirmation_if_not_present(
+            order_helpers.add_tx_confirmation_if_not_present(
-                TxConfirmation, order, region_number)
+                order, region_number)
 
         # Check whether the order is in "sent" or "received" state already. In
         # the positive case, end the current transmission to start a new one.
@@ -434,8 +425,8 @@ class RxConfirmationResource(Resource):
             return get_http_error_resp('SEQUENCE_NUMBER_NOT_FOUND', tx_seq_num)
 
         region_in_request = int(args['region'])
-        order_helpers.add_confirmation_if_not_present(RxConfirmation, order,
+        order_helpers.add_rx_confirmation_if_not_present(
-                                                      region_in_request)
+            order, region_in_request)
 
         # Check whether the order is in "sent" or "received" state already. In
         # the positive case, end the current transmission to start a new one.

server/tests/test_transmitter.py

@@ -390,6 +390,7 @@ def test_retransmission(mock_new_invoice, client, mockredis):
     db.session.commit()
 
     transmitter.tx_start()
+    assert_order_state(first_order_uuid, 'transmitting')
     retry_order = TxRetry.query.all()
     assert len(retry_order) == 1
     assert retry_order[0].order_id == first_order.id
@@ -398,6 +399,12 @@ def test_retransmission(mock_new_invoice, client, mockredis):
     assert second_order.retransmission is None
     assert third_order.retransmission is None
 
+    # A repeated Tx confirmation should not put the order back into confirming
+    # state. Otherwise, another retransmission would be triggered.
+    confirm_tx(first_order.tx_seq_num, all_region_numbers[1:3], client)
+    assert_order_state(first_order_uuid, 'transmitting')
+    assert first_order.retransmission.retry_count == 3
+
     # The retransmission information should be returned by the
     # /admin/order/:uuid endpoint or the /admin/orders/:state endpoint
     get_rv1_admin = client.get(