From 2250bfd8d6a3f2c96d1881a31b5faa21c0ae76d6 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 18 May 2020 18:31:03 +0200 Subject: [PATCH] lingo: use form instead of json input for backends (#6710) --- combo/apps/lingo/forms.py | 77 +++++++++++++++++++-- combo/apps/lingo/manager_views.py | 13 ++-- combo/apps/lingo/models.py | 10 +-- tests/test_lingo_manager.py | 108 ++++++++++++++++-------------- 4 files changed, 139 insertions(+), 69 deletions(-) diff --git a/combo/apps/lingo/forms.py b/combo/apps/lingo/forms.py index 224b9e28..34c479ba 100644 --- a/combo/apps/lingo/forms.py +++ b/combo/apps/lingo/forms.py @@ -18,22 +18,58 @@ import datetime from django import forms +from django.core.exceptions import ValidationError from django.utils.translation import ugettext_lazy as _ -from .models import Regie +from .models import Regie, PaymentBackend + +TYPE_FIELD_MAPPING = { + str: forms.CharField, + bool: forms.BooleanField, + int: forms.IntegerField, + float: forms.FloatField, +} + + +def get_validator(func, err_msg): + def validate(value): + if not func(value): + message = err_msg or _('Invalid value.') + raise ValidationError(message) + return validate def create_form_fields(parameters, json_field): fields, initial = {}, {} for param in parameters: field_name = param['name'] - if param['type'] is bool: - fields[field_name] = forms.BooleanField(label=param['caption'], required=False) - initial[field_name] = param['default'] - if field_name in json_field: - initial[field_name] = json_field[field_name] + if field_name in ('normal_return_url', 'automatic_return_url') or param.get('deprecated'): + continue + + field_params = { + 'label': param.get('caption') or field_name, + 'required': param.get('required', False) + } + if 'validation' in param: + field_params['validators'] = [ + get_validator(param['validation'], param.get('validation_err_msg')) + ] + + _type = param.get('type', str) + choices = param.get('choices') + if choices is not None: + field_class = forms.MultipleChoiceField if _type is list else forms.ChoiceField + if choices and not isinstance(choices[0], tuple): + choices = [(choice, choice) for choice in choices] + field_params['choices'] = choices else: - raise NotImplementedError() + field_class = TYPE_FIELD_MAPPING[_type] + + fields[field_name] = field_class(**field_params) + initial_value = json_field.get(field_name, param.get('default')) + if initial_value: + initial[field_name] = initial_value + return fields, initial @@ -72,6 +108,33 @@ class RegieForm(forms.ModelForm): return instance +class PaymentBackendForm(forms.ModelForm): + + class Meta: + model = PaymentBackend + fields = ['label', 'slug', 'service'] + + def __init__(self, *args, **kwargs): + super(PaymentBackendForm, self).__init__(*args, **kwargs) + fields, initial = create_form_fields( + self.instance.get_payment().get_parameters(scope='global'), + self.instance.service_options + ) + self.fields.update(fields) + self.initial.update(initial) + if self.fields['service']: + self.fields['service'].disabled = True + + def save(self): + instance = super(PaymentBackendForm, self).save() + instance.service_options = compute_json_field( + self.instance.get_payment().get_parameters(scope='global'), + self.cleaned_data + ) + instance.save() + return instance + + class TransactionExportForm(forms.Form): start_date = forms.DateField( label=_('Start date'), diff --git a/combo/apps/lingo/manager_views.py b/combo/apps/lingo/manager_views.py index 6227207e..c1f6f126 100644 --- a/combo/apps/lingo/manager_views.py +++ b/combo/apps/lingo/manager_views.py @@ -20,10 +20,12 @@ from dateutil import parser as date_parser from django.urls import reverse from django.urls import reverse_lazy +from django.contrib import messages from django.db.models import Q, Prefetch from django.db.models.expressions import RawSQL from django.utils import six from django.utils.timezone import make_aware, now +from django.utils.translation import ugettext_lazy as _ from django.views.generic import CreateView, UpdateView, ListView, DeleteView, View from django.http import HttpResponse from django.http import HttpResponseRedirect @@ -32,7 +34,7 @@ from django.template.response import TemplateResponse import eopayment -from .forms import RegieForm +from .forms import RegieForm, PaymentBackendForm from .forms import TransactionExportForm from .models import BasketItem, PaymentBackend, Regie, Transaction @@ -64,13 +66,16 @@ class PaymentBackendListView(ListView): class PaymentBackendCreateView(CreateView): model = PaymentBackend - fields = '__all__' - success_url = reverse_lazy('lingo-manager-paymentbackend-list') + fields = ['label', 'slug', 'service'] + + def get_success_url(self): + messages.info(self.request, _('Please fill additional backend parameters.')) + return reverse_lazy('lingo-manager-paymentbackend-edit', kwargs={'pk': self.object.pk}) class PaymentBackendUpdateView(UpdateView): model = PaymentBackend - fields = '__all__' + form_class = PaymentBackendForm success_url = reverse_lazy('lingo-manager-paymentbackend-list') diff --git a/combo/apps/lingo/models.py b/combo/apps/lingo/models.py index c503d3f5..220ec309 100644 --- a/combo/apps/lingo/models.py +++ b/combo/apps/lingo/models.py @@ -90,11 +90,6 @@ def build_remote_item(data, regie): no_online_payment_reason=data.get('no_online_payment_reason')) -def validate_dict(value): - if not isinstance(value, dict): - raise ValidationError(_('Value must be a JSON object')) - - @python_2_unicode_compatible class PaymentBackend(models.Model): label = models.CharField(verbose_name=_('Label'), max_length=64) @@ -103,10 +98,7 @@ class PaymentBackend(models.Model): help_text=_('The identifier is used in webservice calls.')) service = models.CharField( verbose_name=_('Payment Service'), max_length=64, choices=SERVICES) - service_options = JSONField( - blank=True, - verbose_name=_('Payment Service Options'), - validators=[validate_dict]) + service_options = JSONField(blank=True, verbose_name=_('Payment Service Options')) def __str__(self): return self.label diff --git a/tests/test_lingo_manager.py b/tests/test_lingo_manager.py index dd7a842d..4b0ee90f 100644 --- a/tests/test_lingo_manager.py +++ b/tests/test_lingo_manager.py @@ -604,76 +604,86 @@ def test_add_payment_backend(app, admin_user): resp = app.get('/manage/lingo/paymentbackends/add/', status=200) assert '/manage/lingo/paymentbackends/' in resp.text - resp.forms[0]['label'] = 'Test' - resp.forms[0]['slug'] = 'test-add' - resp.forms[0]['service'] = 'dummy' - resp.forms[0]['service_options'] = '{"siret": "1234"}' + resp.form['label'] = 'Test' + resp.form['slug'] = 'test-add' + resp.form['service'] = 'dummy' resp = resp.forms[0].submit() assert PaymentBackend.objects.count() == 1 payment_backend = PaymentBackend.objects.get(slug='test-add') assert payment_backend.label == 'Test' - assert payment_backend.service_options == {'siret': '1234'} + assert payment_backend.service_options == {} - assert resp.location.endswith('/manage/lingo/paymentbackends/') + backend = PaymentBackend.objects.first() + assert resp.location.endswith('/manage/lingo/paymentbackends/%s/edit' % backend.pk) + resp = resp.follow() + assert 'fill additional backend parameters' in resp.text -def test_add_payment_backend_validate_options(app, admin_user): +def test_edit_payment_backend(app, admin_user): + payment_backend = PaymentBackend.objects.create(label='label1', slug='slug1', service='dummy') app = login(app) - resp = app.get('/manage/lingo/paymentbackends/add/', status=200) + resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk, status=200) assert '/manage/lingo/paymentbackends/' in resp.text - resp.forms[0]['label'] = 'Test' - resp.forms[0]['slug'] = 'test-add' - resp.forms[0]['service'] = 'dummy' - resp.forms[0]['service_options'] = 'xx' - resp2 = resp.forms[0].submit() - assert not PaymentBackend.objects.count() + # service cannot be changed + assert 'disabled' in resp.form['service'].attrs - resp.forms[0]['service_options'] = '"xx"' - resp2 = resp.forms[0].submit() - assert not PaymentBackend.objects.count() + # deprecated parameters are not shown + assert not 'next_url' in resp.form.fields + assert not 'direct_notification_url' in resp.form.fields - resp.forms[0]['service_options'] = '[1]' - resp2 = resp.forms[0].submit() - assert not PaymentBackend.objects.count() + # default values become initial values + assert resp.form['siret']._value == '1234' - resp.forms[0]['service_options'] = '{"a": 1}' - resp2 = resp.forms[0].submit() - assert PaymentBackend.objects.count() - assert PaymentBackend.objects.get().service_options == {'a': 1} + resp.form['label'] = 'label1-modified' + resp.form['siret'] = '12345' + resp.form['consider_all_response_signed'] = True + resp.form['number'] = 12 + resp.form['choice'] = 'b' + resp.form['choices'] = ['a', 'b'] + resp = resp.form.submit() + assert resp.location.endswith('/manage/lingo/paymentbackends/') + payment_backend = PaymentBackend.objects.get(slug='slug1') + assert payment_backend.label == 'label1-modified' + assert payment_backend.service_options['siret'] == '12345' + assert payment_backend.service_options['consider_all_response_signed'] == True + assert payment_backend.service_options['number'] == 12 + assert payment_backend.service_options['choice'] == 'b' + assert payment_backend.service_options['choices'] == ['a', 'b'] -@pytest.mark.xfail -def test_jsonfield_null_bug(app, admin_user): - app = login(app) - resp = app.get('/manage/lingo/paymentbackends/add/', status=200) - assert '/manage/lingo/paymentbackends/' in resp.text - resp.forms[0]['label'] = 'Test' - resp.forms[0]['slug'] = 'test-add' - resp.forms[0]['service'] = 'dummy' - resp.forms[0]['service_options'] = 'null' - resp2 = resp.forms[0].submit() - assert PaymentBackend.objects.count() - assert PaymentBackend.objects.get().service_options == {'a': 1} +def test_edit_payment_backend_validation(app, admin_user): + payment_backend = PaymentBackend.objects.create(label='label1', slug='slug1', service='dummy') + app = login(app) + resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk) + # required field + resp.form['siret'] = '' + resp = resp.form.submit() + assert 'This field is required.' in resp.text + assert resp.form['siret']._value == None -def test_edit_payment_backend(app, admin_user): - payment_backend = PaymentBackend.objects.create(label='label1', slug='slug1') - app = login(app) - resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk, status=200) - assert '/manage/lingo/paymentbackends/' in resp.text + # validation function + resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk) + resp.form['siret'] = 'a' + resp = resp.form.submit() + assert 'must be a number' in resp.text + assert resp.form['siret']._value == 'a' - resp.forms[0]['label'] = 'label1-modified' - resp.forms[0]['slug'] = 'slug1' - resp.forms[0]['service'] = 'dummy' - resp.forms[0]['service_options'] = '{"siret": "1234"}' - resp = resp.forms[0].submit() + # wrong type + resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk) + resp.form['number'] = 'a' + resp = resp.form.submit() + assert 'Enter a whole number.' in resp.text + assert resp.form['number']._value == 'a' - assert resp.location.endswith('/manage/lingo/paymentbackends/') - payment_backend = PaymentBackend.objects.get(slug='slug1') - assert payment_backend.label == 'label1-modified' + # not in choices + resp = app.get('/manage/lingo/paymentbackends/%s/edit' % payment_backend.pk) + resp.form['choice'].force_value('c') + resp = resp.form.submit() + assert 'c is not one of the available choices' in resp.text def test_use_old_service_options_safely(app, admin_user): -- 2.20.1