diff --git a/apps/member/views.py b/apps/member/views.py index 06da8513..c1eb4df8 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -625,9 +625,6 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView): # Retrieve form data credit_type = form.cleaned_data["credit_type"] credit_amount = form.cleaned_data["credit_amount"] - last_name = form.cleaned_data["last_name"] - first_name = form.cleaned_data["first_name"] - bank = form.cleaned_data["bank"] soge = form.cleaned_data["soge"] and not user.profile.soge and (club.name == "BDE" or club.name == "Kfet") if not credit_type: @@ -674,17 +671,9 @@ class ClubAddMemberView(ProtectQuerysetMixin, ProtectedCreateView): .format(form.instance.club.membership_end)) error = True - if credit_amount: - if not last_name or not first_name or (not bank and credit_type.special_type == "Chèque"): - if not last_name: - form.add_error('last_name', _("This field is required.")) - error = True - if not first_name: - form.add_error('first_name', _("This field is required.")) - error = True - if not bank and credit_type.special_type == "Chèque": - form.add_error('bank', _("This field is required.")) - error = True + if credit_amount and not SpecialTransaction.validate_payment_form(form): + # Check that special information for payment are filled + error = True return not error diff --git a/apps/note/api/views.py b/apps/note/api/views.py index 6f882ab6..594b2b9c 100644 --- a/apps/note/api/views.py +++ b/apps/note/api/views.py @@ -74,7 +74,7 @@ class AliasViewSet(ReadProtectedModelViewSet): serializer_class = self.serializer_class if self.request.method in ['PUT', 'PATCH']: # alias owner cannot be change once establish - setattr(serializer_class.Meta, 'read_only_fields', ('note',)) + serializer_class.Meta.read_only_fields = ('note',) return serializer_class def destroy(self, request, *args, **kwargs): @@ -82,7 +82,7 @@ class AliasViewSet(ReadProtectedModelViewSet): try: self.perform_destroy(instance) except ValidationError as e: - return Response({e.code: e.message}, status.HTTP_400_BAD_REQUEST) + return Response({e.code: str(e)}, status.HTTP_400_BAD_REQUEST) return Response(status=status.HTTP_204_NO_CONTENT) def get_queryset(self): diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index 36183e94..3c6b4c7d 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -333,6 +333,36 @@ class SpecialTransaction(Transaction): self.clean() super().save(*args, **kwargs) + @staticmethod + def validate_payment_form(form): + """ + Ensure that last name and first name are filled for a form that creates a SpecialTransaction, + and check that if the user pays with a check, then the bank field is filled. + + Return True iff there is no error. + Whenever there is an error, they are inserted in the form errors. + """ + + credit_type = form.cleaned_data["credit_type"] + last_name = form.cleaned_data["last_name"] + first_name = form.cleaned_data["first_name"] + bank = form.cleaned_data["bank"] + + error = False + + if not last_name or not first_name or (not bank and credit_type.special_type == "Chèque"): + if not last_name: + form.add_error('last_name', _("This field is required.")) + error = True + if not first_name: + form.add_error('first_name', _("This field is required.")) + error = True + if not bank and credit_type.special_type == "Chèque": + form.add_error('bank', _("This field is required.")) + error = True + + return not error + class Meta: verbose_name = _("Special transaction") verbose_name_plural = _("Special transactions") diff --git a/apps/note/static/note/js/consos.js b/apps/note/static/note/js/consos.js index 81a71e6c..5999ffc3 100644 --- a/apps/note/static/note/js/consos.js +++ b/apps/note/static/note/js/consos.js @@ -28,7 +28,7 @@ $(document).ready(function () { // Switching in double consumptions mode should update the layout $('#double_conso').change(function () { - $('#consos_list_div').removeClass('d-none') + document.getElementById('consos_list_div').classList.remove('d-none') $('#infos_div').attr('class', 'col-sm-5 col-xl-6') const note_list_obj = $('#note_list') @@ -37,7 +37,7 @@ $(document).ready(function () { note_list_obj.html('') buttons.forEach(function (button) { - $('#conso_button_' + button.id).click(function () { + document.getElementById(`conso_button_${button.id}`).addEventListener('click', () => { if (LOCK) { return } removeNote(button, 'conso_button', buttons, 'consos_list')() }) @@ -46,7 +46,7 @@ $(document).ready(function () { }) $('#single_conso').change(function () { - $('#consos_list_div').addClass('d-none') + document.getElementById('consos_list_div').classList.add('d-none') $('#infos_div').attr('class', 'col-sm-5 col-md-4') const consos_list_obj = $('#consos_list') @@ -68,9 +68,9 @@ $(document).ready(function () { }) // Ensure we begin in single consumption. Fix issue with TurboLinks and BootstrapJS - $("label[for='double_conso']").removeClass('active') + document.querySelector("label[for='double_conso']").classList.remove('active') - $('#consume_all').click(consumeAll) + document.getElementById("consume_all").addEventListener('click', consumeAll) }) notes = [] @@ -127,11 +127,10 @@ function addConso (dest, amount, type, category_id, category_name, template_id, html += li('conso_button_' + button.id, button.name + '' + button.quantity + '') }) + document.getElementById(list).innerHTML = html - $('#' + list).html(html) - - buttons.forEach(function (button) { - $('#conso_button_' + button.id).click(function () { + buttons.forEach((button) => { + document.getElementById(`conso_button_${button.id}`).addEventListener('click', () => { if (LOCK) { return } removeNote(button, 'conso_button', buttons, list)() }) @@ -146,12 +145,13 @@ function reset () { notes_display.length = 0 notes.length = 0 buttons.length = 0 - $('#note_list').html('') - $('#consos_list').html('') - $('#note').val('') - $('#note').attr('data-original-title', '').tooltip('hide') - $('#profile_pic').attr('src', '/static/member/img/default_picture.png') - $('#profile_pic_link').attr('href', '#') + document.getElementById('note_list').innerHTML = '' + document.getElementById('consos_list').innerHTML = '' + document.getElementById('note').value = '' + document.getElementById('note').dataset.originTitle = '' + $('#note').tooltip('hide') + document.getElementById('profile_pic').src = '/static/member/img/default_picture.png' + document.getElementById('profile_pic_link').href = '#' refreshHistory() refreshBalance() LOCK = false @@ -168,7 +168,7 @@ function consumeAll () { let error = false if (notes_display.length === 0) { - $('#note').addClass('is-invalid') + document.getElementById('note').classList.add('is-invalid') $('#note_list').html(li('', 'Ajoutez des émetteurs.', 'text-danger')) error = true } diff --git a/apps/registration/forms.py b/apps/registration/forms.py index 7b2670ca..17b89c33 100644 --- a/apps/registration/forms.py +++ b/apps/registration/forms.py @@ -101,14 +101,14 @@ class ValidationForm(forms.Form): required=False, ) - join_BDE = forms.BooleanField( + join_bde = forms.BooleanField( label=_("Join BDE Club"), required=False, initial=True, ) # The user can join the Kfet club at the inscription - join_Kfet = forms.BooleanField( + join_kfet = forms.BooleanField( label=_("Join Kfet Club"), required=False, initial=True, diff --git a/apps/registration/tables.py b/apps/registration/tables.py index caa73186..960866a6 100644 --- a/apps/registration/tables.py +++ b/apps/registration/tables.py @@ -3,7 +3,6 @@ import django_tables2 as tables from django.contrib.auth.models import User - from treasury.models import SogeCredit diff --git a/apps/registration/templates/registration/future_profile_detail.html b/apps/registration/templates/registration/future_profile_detail.html index d8f061c0..577ad219 100644 --- a/apps/registration/templates/registration/future_profile_detail.html +++ b/apps/registration/templates/registration/future_profile_detail.html @@ -100,13 +100,14 @@ SPDX-License-Identifier: GPL-3.0-or-later bank.attr('disabled', true); bank.val('Société générale'); - let join_BDE = $("#id_join_BDE"); - join_BDE.attr('disabled', true); - join_BDE.attr('checked', 'checked'); + let join_bde = $("#id_join_bde"); - let join_Kfet = $("#id_join_Kfet"); - join_Kfet.attr('disabled', true); - join_Kfet.attr('checked', 'checked'); + join_bde.attr('disabled', true); + join_bde.attr('checked', 'checked'); + + let join_kfet = $("#id_join_kfet"); + join_kfet.attr('disabled', true); + join_kfet.attr('checked', 'checked'); } soge_field.change(fillFields); @@ -116,4 +117,4 @@ SPDX-License-Identifier: GPL-3.0-or-later fillFields(); {% endif %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/apps/registration/tests/test_registration.py b/apps/registration/tests/test_registration.py index 22628b16..18cf16db 100644 --- a/apps/registration/tests/test_registration.py +++ b/apps/registration/tests/test_registration.py @@ -196,8 +196,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=False, - join_Kfet=False, + join_bde=False, + join_kfet=False, )) self.assertEqual(response.status_code, 200) self.assertTrue(response.context["form"].errors) @@ -210,8 +210,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=False, - join_Kfet=True, + join_bde=False, + join_kfet=True, )) self.assertEqual(response.status_code, 200) self.assertTrue(response.context["form"].errors) @@ -224,8 +224,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="J'ai pas d'argent", - join_BDE=True, - join_Kfet=True, + join_bde=True, + join_kfet=True, )) self.assertEqual(response.status_code, 200) self.assertTrue(response.context["form"].errors) @@ -238,8 +238,8 @@ class TestValidateRegistration(TestCase): last_name="", first_name="", bank="", - join_BDE=True, - join_Kfet=True, + join_bde=True, + join_kfet=True, )) self.assertEqual(response.status_code, 200) self.assertTrue(response.context["form"].errors) @@ -255,8 +255,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=True, - join_Kfet=False, + join_bde=True, + join_kfet=False, )) self.assertEqual(response.status_code, 200) self.assertTrue(response.context["form"].errors) @@ -281,8 +281,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=True, - join_Kfet=False, + join_bde=True, + join_kfet=False, )) self.assertRedirects(response, self.user.profile.get_absolute_url(), 302, 200) self.user.profile.refresh_from_db() @@ -317,8 +317,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=True, - join_Kfet=True, + join_bde=True, + join_kfet=True, )) self.assertRedirects(response, self.user.profile.get_absolute_url(), 302, 200) self.user.profile.refresh_from_db() @@ -353,8 +353,8 @@ class TestValidateRegistration(TestCase): last_name="TOTO", first_name="Toto", bank="Société générale", - join_BDE=True, - join_Kfet=True, + join_bde=True, + join_kfet=True, )) self.assertRedirects(response, self.user.profile.get_absolute_url(), 302, 200) self.user.profile.refresh_from_db() diff --git a/apps/registration/tokens.py b/apps/registration/tokens.py index 98104997..ef8ddb02 100644 --- a/apps/registration/tokens.py +++ b/apps/registration/tokens.py @@ -9,6 +9,7 @@ class AccountActivationTokenGenerator(PasswordResetTokenGenerator): """ Create a unique token generator to confirm email addresses. """ + def _make_hash_value(self, user, timestamp): """ Hash the user's primary key and some user state that's sure to change @@ -23,9 +24,18 @@ class AccountActivationTokenGenerator(PasswordResetTokenGenerator): """ # Truncate microseconds so that tokens are consistent even if the # database doesn't support microseconds. - login_timestamp = '' if user.last_login is None else user.last_login.replace(microsecond=0, tzinfo=None) - return str(user.pk) + str(user.email) + str(user.profile.email_confirmed)\ - + str(login_timestamp) + str(timestamp) + login_timestamp = ( + "" + if user.last_login is None + else user.last_login.replace(microsecond=0, tzinfo=None) + ) + return ( + str(user.pk) + + str(user.email) + + str(user.profile.email_confirmed) + + str(login_timestamp) + + str(timestamp) + ) email_validation_token = AccountActivationTokenGenerator() diff --git a/apps/registration/views.py b/apps/registration/views.py index cb2a2900..746c856b 100644 --- a/apps/registration/views.py +++ b/apps/registration/views.py @@ -248,9 +248,13 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, @transaction.atomic def form_valid(self, form): + """ + Finally validate the registration, with creating the membership. + """ user = self.get_object() if Alias.objects.filter(normalized_name=Alias.normalize(user.username)).exists(): + # Don't try to hack an existing account. form.add_error(None, _("An alias with a similar name already exists.")) return self.form_invalid(form) @@ -261,35 +265,36 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, last_name = form.cleaned_data["last_name"] first_name = form.cleaned_data["first_name"] bank = form.cleaned_data["bank"] - join_BDE = form.cleaned_data["join_BDE"] - join_Kfet = form.cleaned_data["join_Kfet"] + join_bde = form.cleaned_data["join_bde"] + join_kfet = form.cleaned_data["join_kfet"] if soge: - # If Société Générale pays the inscription, the user joins the two clubs - join_BDE = True - join_Kfet = True + # If Société Générale pays the inscription, the user automatically joins the two clubs. + join_bde = True + join_kfet = True - if not join_BDE: - form.add_error('join_BDE', _("You must join the BDE.")) + if not join_bde: + # This software belongs to the BDE. + form.add_error('join_bde', _("You must join the BDE.")) return super().form_invalid(form) + # Calculate required registration fee fee = 0 bde = Club.objects.get(name="BDE") bde_fee = bde.membership_fee_paid if user.profile.paid else bde.membership_fee_unpaid - if join_BDE: - fee += bde_fee + # This is mandatory. + fee += bde_fee if join_bde else 0 kfet = Club.objects.get(name="Kfet") kfet_fee = kfet.membership_fee_paid if user.profile.paid else kfet.membership_fee_unpaid - if join_Kfet: - fee += kfet_fee + # Add extra fee for the full membership + fee += kfet_fee if join_kfet else 0 - if soge: - # If the bank pays, then we don't credit now. Treasurers will validate the transaction - # and credit the note later. - credit_type = None + # If the bank pays, then we don't credit now. Treasurers will validate the transaction + # and credit the note later. + credit_type = None if soge else credit_type - if credit_type is None: - credit_amount = 0 + # If the user does not select any payment method, then no credit will be performed. + credit_amount = 0 if credit_type is None else credit_amount if fee > credit_amount and not soge: # Check if the user credits enough money @@ -298,15 +303,9 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, .format(pretty_money(fee))) return self.form_invalid(form) - if credit_type is not None and credit_amount > 0: - if not last_name or not first_name or (not bank and credit_type.special_type == "Chèque"): - if not last_name: - form.add_error('last_name', _("This field is required.")) - if not first_name: - form.add_error('first_name', _("This field is required.")) - if not bank and credit_type.special_type == "Chèque": - form.add_error('bank', _("This field is required.")) - return self.form_invalid(form) + # Check that payment information are filled, like last name and first name + if credit_type is not None and credit_amount > 0 and not SpecialTransaction.validate_payment_form(form): + return self.form_invalid(form) # Save the user and finally validate the registration # Saving the user creates the associated note @@ -338,7 +337,7 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, valid=True, ) - if join_BDE: + if join_bde: # Create membership for the user to the BDE starting today membership = Membership( club=bde, @@ -352,7 +351,7 @@ class FutureUserDetailView(ProtectQuerysetMixin, LoginRequiredMixin, FormMixin, membership.roles.add(Role.objects.get(name="Adhérent BDE")) membership.save() - if join_Kfet: + if join_kfet: # Create membership for the user to the Kfet starting today membership = Membership( club=kfet, diff --git a/apps/treasury/views.py b/apps/treasury/views.py index d18f8418..47544cc1 100644 --- a/apps/treasury/views.py +++ b/apps/treasury/views.py @@ -210,7 +210,7 @@ class InvoiceRenderView(LoginRequiredMixin, View): del tex # The file has to be rendered twice - for ignored in range(2): + for _ignored in range(2): error = subprocess.Popen( ["/usr/bin/xelatex", "-interaction=nonstopmode", "invoice-{}.tex".format(pk)], cwd=tmp_dir, diff --git a/tox.ini b/tox.ini index c900d524..73e80b37 100644 --- a/tox.ini +++ b/tox.ini @@ -24,6 +24,7 @@ commands = [testenv:linters] deps = flake8 + flake8-bugbear flake8-colors flake8-django flake8-import-order @@ -31,7 +32,7 @@ deps = pep8-naming pyflakes commands = - flake8 apps/activity apps/api apps/logs apps/member apps/note apps/permission apps/treasury apps/wei + flake8 apps --extend-exclude apps/scripts [flake8] ignore = W503, I100, I101