From ccb2cb8500494b4bcf75e9afd5987aa53ea605d1 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 15 Apr 2021 20:43:33 +0200 Subject: [PATCH] lingo: check real payment status of remote_item when shown or paid (#53186) Currently the remote_item.paid attribute comes from what the remote regie is providing through a web service. But a remote_item can be already paid if a transaction with a paid status (eopayment.PAID or eopayment.ACCEPTED) already exist. This patch add a RemoteItem.update_paid(regie, remote_items) which update bunch of remote_item(s) by querying the correponding paid transactions. The methods Regie.get_invoices() and Regie.get_invoice() get a new parameter update_paid=False, which is set to true in cells and views which display or allow paying a remote item. --- combo/apps/lingo/models.py | 53 +++++++++++++++++++++++++++--- combo/apps/lingo/views.py | 10 ++++-- tests/test_lingo_remote_regie.py | 55 ++++++++++++++++++++++---------- 3 files changed, 93 insertions(+), 25 deletions(-) diff --git a/combo/apps/lingo/models.py b/combo/apps/lingo/models.py index ee065b5f..1f3b4a81 100644 --- a/combo/apps/lingo/models.py +++ b/combo/apps/lingo/models.py @@ -21,6 +21,7 @@ import json import logging import re from decimal import Decimal +from functools import reduce import eopayment from dateutil import parser @@ -228,7 +229,7 @@ class Regie(models.Model): return self.text_on_success return _('Your payment has been succesfully registered.') - def get_invoices(self, user, history=False): + def get_invoices(self, user, history=False, update_paid=False): if not self.is_remote(): return [] if user: @@ -254,11 +255,15 @@ class Regie(models.Model): if items.get('data'): if not isinstance(items['data'], list): raise RegieException(regie_exc_msg) - return [build_remote_item(item, self) for item in items['data']] + remote_items = [build_remote_item(item, self) for item in items['data']] + if not history and update_paid: + # update paid status using known transactions + RemoteItem.update_paid(self, remote_items) + return remote_items return [] return [] - def get_invoice(self, user, invoice_id, log_errors=True, raise_4xx=False): + def get_invoice(self, user, invoice_id, log_errors=True, raise_4xx=False, update_paid=False): if not self.is_remote(): return self.basketitem_set.get(pk=invoice_id) url = self.webservice_url + '/invoice/%s/' % invoice_id @@ -274,7 +279,11 @@ class Regie(models.Model): raise RemoteInvoiceException() if response.json().get('data') is None: raise ObjectDoesNotExist() - return build_remote_item(response.json().get('data'), self) + remote_item = build_remote_item(response.json().get('data'), self) + if update_paid: + # update paid status using known transactions + RemoteItem.update_paid(self, [remote_item]) + return remote_item def get_invoice_pdf(self, user, invoice_id): """ @@ -603,6 +612,38 @@ class RemoteItem(object): def crypto_id(self): return aes_hex_encrypt(settings.SECRET_KEY, force_bytes(str(self.id))) + @classmethod + def update_paid(cls, regie, remote_items): + remote_item_ids = [remote_item.id for remote_item in remote_items if not remote_item.paid] + if not remote_item_ids: + return + + paid_items = {} + # filter transactions by regie, status and contained remote_item id + transaction_qs = Transaction.objects.filter( + regie=regie, status__in=[eopayment.PAID, eopayment.ACCEPTED] + ) + query = reduce( + models.Q.__or__, + (models.Q(remote_items__contains=remote_item_id) for remote_item_id in remote_item_ids), + ) + + # accumulate in paid_items each remote_item earliest payment_date + for transaction in transaction_qs.filter(query): + for remote_item in transaction.remote_items.split(','): + if remote_item not in paid_items: + paid_items[remote_item] = transaction.end_date + else: + paid_items[remote_item] = min(transaction.end_date, paid_items[remote_item]) + + # update remote_item.paid using paid_items + for remote_item in remote_items: + if remote_item.paid: + continue + if remote_item.id in paid_items: + remote_item.paid = True + remote_item.payment_date = paid_items[remote_item.id] + class Transaction(models.Model): regie = models.ForeignKey(Regie, on_delete=models.CASCADE, null=True) @@ -915,7 +956,9 @@ class ActiveItems(Items): errors = [] for r in self.get_regies(): try: - items.extend(r.get_invoices(user)) + for remote_item in r.get_invoices(user, update_paid=True): + if not remote_item.paid: + items.append(remote_item) except RegieException as e: errors.append(e) return items, errors diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py index e4a155f5..b2b12033 100644 --- a/combo/apps/lingo/views.py +++ b/combo/apps/lingo/views.py @@ -390,6 +390,10 @@ class PayMixin(object): messages.error(request, _('This regie allows to pay only one item.')) return HttpResponseRedirect(next_url) + if any(item.paid for item in remote_items): + messages.error(request, _('Some items are already paid.')) + return HttpResponseRedirect(next_url) + total_amount = sum([x.amount for x in remote_items or items]) if total_amount < regie.payment_min_amount: @@ -497,7 +501,7 @@ class PayView(PayMixin, View): regie = Regie.objects.get(pk=regie_id) # get all items data from regie webservice for item_id in request.POST.getlist('item'): - remote_items.append(regie.get_invoice(user, item_id)) + remote_items.append(regie.get_invoice(user, item_id, update_paid=True)) except (requests.exceptions.RequestException, RemoteInvoiceException): messages.error(request, _(u'Technical error: impossible to retrieve invoices.')) return HttpResponseRedirect(next_url) @@ -878,7 +882,7 @@ class ItemView(TemplateView): raise Http404() try: - item = regie.get_invoice(self.request.user, item_id) + item = regie.get_invoice(self.request.user, item_id, update_paid=True) if self.request.GET.get('page'): try: ret['page'] = Page.objects.get(pk=self.request.GET['page']) @@ -948,7 +952,7 @@ class SelfInvoiceView(View): else: for regie in obj.get_regies(): try: - invoice = regie.get_invoice(None, invoice_id, log_errors=False) + invoice = regie.get_invoice(None, invoice_id, log_errors=False, update_paid=True) except ObjectDoesNotExist: continue if invoice.total_amount != invoice_amount: diff --git a/tests/test_lingo_remote_regie.py b/tests/test_lingo_remote_regie.py index 4b927e22..46a3ff63 100644 --- a/tests/test_lingo_remote_regie.py +++ b/tests/test_lingo_remote_regie.py @@ -4,6 +4,7 @@ import copy import json from decimal import Decimal +import eopayment import mock import pytest from django.apps import apps @@ -47,7 +48,7 @@ INVOICES = [ 'has_pdf': True, 'online_payment': True, 'paid': False, - 'payment_date': '1970-01-01', + 'payment_date': None, 'no_online_payment_reason': '', 'reference_id': 'order-id-1', }, @@ -63,7 +64,7 @@ INVOICES = [ 'has_pdf': True, 'online_payment': True, 'paid': False, - 'payment_date': '1970-01-01', + 'payment_date': None, 'no_online_payment_reason': '', 'reference_id': 'order-id-2', }, @@ -140,6 +141,16 @@ def test_remote_regie_active_invoices_cell(mock_send, remote_regie): assert 'F-2016-Two' in content assert '543.21' in content + # set the second one as paid + Transaction.objects.create( + regie=remote_regie, remote_items=INVOICES[1]['id'], status=eopayment.PAID, end_date=now() + ) + content = cell.render(context) + assert 'F-2016-One' in content + assert '123.45' in content + assert 'F-2016-Two' not in content + assert '543.21' not in content + assert '?page=%s' % page.pk in content # check if regie webservice has been correctly called assert mock_send.call_args[0][0].method == 'GET' @@ -367,6 +378,7 @@ def test_anonymous_successful_item_payment(mock_get, mock_pay_invoice, app, remo assert 'Total amount: 123.45€' in resp.text assert 'Amount to pay: 123.45€' in resp.text assert 'Amount already paid>' not in resp.text + assert '"buttons"' in resp form = resp.form @@ -424,6 +436,14 @@ def test_anonymous_successful_item_payment(mock_get, mock_pay_invoice, app, remo assert resp.status_code == 200 + # check invoice cannot be paid a second time + resp = app.get('/lingo/item/%s/%s/' % (remote_regie.id, encrypt_id)) + assert '"buttons"' not in resp + + resp = form.submit() + assert resp.location == '/' + assert 'Some items are already paid' in app.session['_messages'] + @mock.patch('combo.apps.lingo.models.requests.get') def test_remote_item_failure(mock_get, app, remote_regie): @@ -626,11 +646,14 @@ def test_remote_item_payment_failure(mock_post, mock_get, mock_pay_invoice, app, appconfig.update_transactions() +@pytest.mark.parametrize('can_pay_only_one_basket_item', [False, True]) @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): +def test_remote_invoice_successfull_payment_redirect( + mock_get, mock_pay_invoice, can_pay_only_one_basket_item, app, remote_regie +): assert remote_regie.is_remote() - assert remote_regie.can_pay_only_one_basket_item is False + remote_regie.can_pay_only_one_basket_item = can_pay_only_one_basket_item remote_regie.save() page = Page(title='xxx', slug='active-remote-invoices-page', template_name='standard') @@ -641,6 +664,7 @@ def test_remote_invoice_successfull_payment_redirect(mock_get, mock_pay_invoice, mock_get.return_value = mock_json mock_pay_invoice.return_value = mock.Mock(status_code=200) resp = app.get('/lingo/item/%s/%s/?page=%s' % (remote_regie.id, encrypt_id, page.pk)) + assert '"paid"' not in resp form = resp.form assert form['next_url'].value == '/active-remote-invoices-page/' form['email'] = 'test@example.net' @@ -652,8 +676,12 @@ 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 + if can_pay_only_one_basket_item: + assert qs['orderid'] == ['order-id-1'] + assert qs['subject'] == ['invoice-one'] + else: + 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) # redirect to payment status @@ -665,17 +693,10 @@ def test_remote_invoice_successfull_payment_redirect(mock_get, mock_pay_invoice, == '/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'] + # check true payment status is visible, even if the remote regie web-service still report the invoice as unpaid + resp = app.get('/lingo/item/%s/%s/?page=%s' % (remote_regie.id, encrypt_id, page.pk)) + assert not INVOICES[0]['paid'] + assert '"paid"' in resp @mock.patch('combo.apps.lingo.models.UserSAMLIdentifier') -- 2.31.0