From 7cea2f291d6c5b73352dd362c79dffb08d6b1efe Mon Sep 17 00:00:00 2001
From: Thomas NOEL <tnoel@entrouvert.com>
Date: Tue, 12 Jan 2021 12:23:50 +0100
Subject: [PATCH] lingo: support can_pay_only_one on remote regie (#50067)

---
 combo/apps/lingo/forms.py        |  6 -----
 combo/apps/lingo/views.py        | 39 +++++++++++++++--------------
 tests/test_lingo_remote_regie.py | 42 +++++++++++++++++++++++++++++++-
 3 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/combo/apps/lingo/forms.py b/combo/apps/lingo/forms.py
index 13d94b35..683d2cdd 100644
--- a/combo/apps/lingo/forms.py
+++ b/combo/apps/lingo/forms.py
@@ -109,12 +109,6 @@ class RegieForm(forms.ModelForm):
         instance.save()
         return instance
 
-    def clean(self):
-        cleaned_data = super().clean()
-        if cleaned_data['webservice_url'] and cleaned_data['can_pay_only_one_basket_item']:
-            raise ValidationError('Individual item payment is incompatible with remote regie.')
-        return cleaned_data
-
 
 class PaymentBackendForm(forms.ModelForm):
 
diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py
index 6957c1b2..95ce62f9 100644
--- a/combo/apps/lingo/views.py
+++ b/combo/apps/lingo/views.py
@@ -360,13 +360,12 @@ class PayMixin(object):
             self, request, regie, items, remote_items, next_url='/', email='', firstname='',
             lastname=''):
         # check contract
-        assert bool(len(items)) != bool(len(remote_items)), (
-            'there should be at least one item or remote item to pay and not both items'
-        )
-        assert not regie.can_pay_only_one_basket_item or len(items) == 1, (
-            'regie can only pay one basket item, but handle_payment() '
-            'did not receive one item: len(items) = %d' % len(items)
-        )
+        if bool(len(items)) == bool(len(remote_items)):
+            messages.error(request, _('Items to pay are missing or are not of the same type (local/remote).'))
+            return HttpResponseRedirect(next_url)
+        if regie.can_pay_only_one_basket_item and (len(items) > 1 or len(remote_items) > 1):
+            messages.error(request, _('This regie allows to pay only one item.'))
+            return HttpResponseRedirect(next_url)
 
         total_amount = sum([x.amount for x in remote_items or items])
 
@@ -418,14 +417,14 @@ class PayMixin(object):
             # copy command reference / invoice number
             if item.reference_id:
                 kwargs['orderid'] = item.reference_id
-            # PayFiP/TIPI specific
-            if regie.payment_backend.service in ('payfip_ws', 'tipi'):
-                if item.request_data and item.request_data.get('exer') and item.request_data.get('refdet'):
-                    kwargs['exer'] = item.request_data['exer']
-                    kwargs['refdet'] = item.request_data['refdet']
-            # allow easy testing/use of backend specific keyword arguments
-            EOPAYMENT_REQUEST_KWARGS_PREFIX = 'eopayment_request_kwargs_'
-            if hasattr(item, 'request_data'):
+            if getattr(item, 'request_data', None):
+                # PayFiP/TIPI specific
+                if regie.payment_backend.service in ('payfip_ws', 'tipi'):
+                    if item.request_data.get('exer') and item.request_data.get('refdet'):
+                        kwargs['exer'] = item.request_data['exer']
+                        kwargs['refdet'] = item.request_data['refdet']
+                # allow easy testing/use of backend specific keyword arguments
+                EOPAYMENT_REQUEST_KWARGS_PREFIX = 'eopayment_request_kwargs_'
                 for key in item.request_data:
                     if key.startswith(EOPAYMENT_REQUEST_KWARGS_PREFIX):
                         arg_name = key[len(EOPAYMENT_REQUEST_KWARGS_PREFIX):]
@@ -495,11 +494,11 @@ class PayView(PayMixin, View):
             regie.compute_extra_fees(user=user)
             items = BasketItem.get_items_to_be_paid(user=user).filter(regie=regie)
 
-            if regie.can_pay_only_one_basket_item and len(items) > 1:
-                messages.error(request, _('Grouping basket items is not allowed.'))
-                logger.error('lingo: regie can only pay one basket item, but handle_payment() received',
-                             extra={'regie': str(regie), 'items': items, 'remote_items': remote_items})
-                return HttpResponseRedirect(next_url)
+        if regie.can_pay_only_one_basket_item and (len(items) > 1 or len(remote_items) > 1):
+            messages.error(request, _('Grouping basket items is not allowed.'))
+            logger.error('lingo: regie can only pay one basket item, but handle_payment() received',
+                         extra={'regie': str(regie), 'items': items, 'remote_items': remote_items})
+            return HttpResponseRedirect(next_url)
 
         if items:
             capture_date = items[0].capture_date
diff --git a/tests/test_lingo_remote_regie.py b/tests/test_lingo_remote_regie.py
index e710d55b..95410d04 100644
--- a/tests/test_lingo_remote_regie.py
+++ b/tests/test_lingo_remote_regie.py
@@ -42,6 +42,23 @@ INVOICES = [
         'paid': False,
         'payment_date': '1970-01-01',
         'no_online_payment_reason': '',
+        'reference_id': 'order-id-1',
+    },
+    {
+        'id': 'F201602',
+        'display_id': 'F-2016-Two',
+        'label': 'invoice-two',
+        'regie': 'remote',
+        'created': '2016-02-02',
+        'pay_limit_date': '2999-12-31',
+        'total_amount': '543.21',
+        'amount': '543.21',
+        'has_pdf': True,
+        'online_payment': True,
+        'paid': False,
+        'payment_date': '1970-01-01',
+        'no_online_payment_reason': '',
+        'reference_id': 'order-id-2',
     },
 ]
 
@@ -87,6 +104,7 @@ class MockUser(object):
 @mock.patch('combo.utils.requests_wrapper.RequestsSession.send')
 def test_remote_regie_active_invoices_cell(mock_send, remote_regie):
     assert remote_regie.is_remote() == True
+    assert remote_regie.can_pay_only_one_basket_item is False
 
     page = Page(title='xxx', slug='test_basket_cell', template_name='standard')
     page.save()
@@ -108,6 +126,8 @@ def test_remote_regie_active_invoices_cell(mock_send, remote_regie):
     content = cell.render(context)
     assert 'F-2016-One' in content
     assert '123.45' in content
+    assert 'F-2016-Two' in content
+    assert '543.21' in content
 
     assert '?page=%s' % page.pk in content
     # check if regie webservice has been correctly called
@@ -199,11 +219,14 @@ def test_remote_regie_past_invoices_cell(mock_send, remote_regie):
     content = cell.render(context)
     assert 'F-2016-One' in content
     assert '123.45' in content
+    assert 'F-2016-Two' in content
+    assert '543.21' in content
     assert 'class="invoice-payment-limit-date"' in content
 
     # invoice without limit date
     invoices = copy.deepcopy(INVOICES)
     invoices[0]['pay_limit_date'] = ''
+    invoices[1]['pay_limit_date'] = ''
     ws_invoices = {'err': 0, 'data': invoices}
     mock_response = mock.Mock(status_code=200, content=json.dumps(ws_invoices))
     mock_response.json.return_value = ws_invoices
@@ -592,9 +615,12 @@ def test_remote_item_payment_failure(mock_post, mock_get, mock_pay_invoice, app,
 @mock.patch('combo.apps.lingo.models.Regie.pay_invoice')
 @mock.patch('combo.apps.lingo.models.requests.get')
 def test_remote_invoice_successfull_payment_redirect(mock_get, mock_pay_invoice, app, remote_regie):
+    assert remote_regie.is_remote()
+    assert remote_regie.can_pay_only_one_basket_item is False
+    remote_regie.save()
+
     page = Page(title='xxx', slug='active-remote-invoices-page', template_name='standard')
     page.save()
-    assert remote_regie.is_remote()
     encrypt_id = aes_hex_encrypt(settings.SECRET_KEY, force_bytes('F201601'))
     mock_json = mock.Mock()
     mock_json.json.return_value = {'err': 0, 'data': INVOICES[0]}
@@ -612,6 +638,8 @@ def test_remote_invoice_successfull_payment_redirect(mock_get, mock_pay_invoice,
     parsed = urlparse.urlparse(location)
     # get return_url and transaction id from location
     qs = urlparse.parse_qs(parsed.query)
+    assert 'orderid' not in qs
+    assert 'subject' not in qs
     args = {'transaction_id': qs['transaction_id'][0], 'signed': True,
             'ok': True, 'reason': 'Paid'}
     resp = app.get(qs['return_url'][0], params=args)
@@ -622,6 +650,18 @@ def test_remote_invoice_successfull_payment_redirect(mock_get, mock_pay_invoice,
     assert urlparse.urlparse(resp.html.find('a', {'id': 'next-url'})['href']).path == \
         '/active-remote-invoices-page/'
 
+    # one item limitation: send orderid to eopayment
+    remote_regie.can_pay_only_one_basket_item = True
+    remote_regie.save()
+    resp = form.submit()
+    assert resp.status_code == 302
+    location = resp.location
+    assert 'dummy-payment' in location
+    parsed = urlparse.urlparse(location)
+    qs = urlparse.parse_qs(parsed.query)
+    assert qs['orderid'] == ['order-id-1']
+    assert qs['subject'] == ['invoice-one']
+
 
 @mock.patch('combo.apps.lingo.models.UserSAMLIdentifier')
 @mock.patch('combo.apps.lingo.models.requests.get')
-- 
2.29.2

