Merge branch 'feature/bankaccount' into 'master'

Add ability for users to change their bank accounts

Closes #812

See merge request !1241

docs/payments.management.commands.rst

+payments.management.commands package
+====================================
+
+.. automodule:: payments.management.commands
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+Submodules
+----------
+
+payments.management.commands.revokeoldmandates module
+-----------------------------------------------------
+
+.. automodule:: payments.management.commands.revokeoldmandates
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+

docs/payments.management.rst

+payments.management package
+===========================
+
+.. automodule:: payments.management
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+Subpackages
+-----------
+
+.. toctree::
+
+    payments.management.commands
+

docs/payments.rst

@@ -6,6 +6,13 @@ payments package
     :undoc-members:
     :show-inheritance:
 
+Subpackages
+-----------
+
+.. toctree::
+
+    payments.management
+
 Submodules
 ----------
 
@@ -33,6 +40,14 @@ payments.apps module
     :undoc-members:
     :show-inheritance:
 
+payments.forms module
+---------------------
+
+.. automodule:: payments.forms
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 payments.models module
 ----------------------
 
@@ -49,6 +64,22 @@ payments.services module
     :undoc-members:
     :show-inheritance:
 
+payments.urls module
+--------------------
+
+.. automodule:: payments.urls
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+payments.views module
+---------------------
+
+.. automodule:: payments.views
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 payments.widgets module
 -----------------------
 

website/members/admin.py

@@ -26,13 +26,12 @@ class ProfileInline(admin.StackedInline):
               'address_street2', 'address_postal_code', 'address_city',
               'address_country', 'student_number', 'phone_number',
               'receive_optin', 'receive_newsletter', 'birthday',
-              'show_birthday', 'direct_debit_authorized', 'bank_account',
-              'initials', 'nickname', 'display_name_preference',
-              'profile_description', 'website', 'photo', 'emergency_contact',
+              'show_birthday', 'auto_renew', 'initials',
+              'nickname', 'display_name_preference', 'profile_description',
+              'website', 'photo', 'emergency_contact',
               'emergency_contact_phone_number', 'language',
               'event_permissions')
     model = models.Profile
-    form = forms.ProfileForm
     can_delete = False
 
 
@@ -101,7 +100,8 @@ class UserAdmin(BaseUserAdmin):
                    'is_superuser',
                    AgeListFilter,
                    'profile__event_permissions',
-                   'profile__starting_year')
+                   'profile__starting_year',
+                   'profile__auto_renew')
 
     add_fieldsets = (
         (None, {

website/members/fixtures/members.json

@@ -94,8 +94,7 @@
         "display_name_preference": "full",
         "language": "nl",
         "receive_optin": true,
-        "direct_debit_authorized": false,
-        "bank_account": ""
+        "auto_renew": false
     }
 },
 {
@@ -121,8 +120,7 @@
         "display_name_preference": "full",
         "language": "nl",
         "receive_optin": true,
-        "direct_debit_authorized": false,
-        "bank_account": ""
+        "auto_renew": false
     }
 },
 {
@@ -148,8 +146,7 @@
         "display_name_preference": "full",
         "language": "nl",
         "receive_optin": true,
-        "direct_debit_authorized": false,
-        "bank_account": ""
+        "auto_renew": false
     }
 },
 {
@@ -175,8 +172,7 @@
         "display_name_preference": "full",
         "language": "nl",
         "receive_optin": true,
-        "direct_debit_authorized": false,
-        "bank_account": ""
+        "auto_renew": false
     }
 },
 {

website/members/forms.py

@@ -15,22 +15,11 @@ class ProfileForm(forms.ModelForm):
                   'phone_number', 'emergency_contact',
                   'emergency_contact_phone_number',
                   'show_birthday', 'website',
-                  'profile_description', 'nickname',
+                  'profile_description', 'nickname', 'initials',
                   'display_name_preference', 'photo', 'language',
                   'receive_optin', 'receive_newsletter']
         model = Profile
 
-    def clean(self):
-        super().clean()
-        errors = {}
-        direct_debit_authorized = self.cleaned_data\
-            .get('direct_debit_authorized')
-        bank_account = self.cleaned_data.get('bank_account')
-        if direct_debit_authorized and not bank_account:
-            errors.update({'bank_account': _('Please enter a bank account')})
-
-        raise forms.ValidationError(errors)
-
 
 class UserCreationForm(BaseUserCreationForm):
     # Don't forget to edit the formset in admin.py!

website/members/management/commands/conscribosync.py

@@ -45,6 +45,20 @@ class Command(BaseCommand):
                     code = current_relations.pop(member.pk, None)
                     profile = member.profile
 
+                    if member.bank_accounts.exists():
+                        account = member.bank_accounts.last()
+                        bank_account = {
+                            'name': account.name,
+                            'bic': account.bic or '',
+                            'iban': account.iban,
+                        }
+                    else:
+                        bank_account = {
+                            'name': '',
+                            'bic': '',
+                            'iban': '',
+                        }
+
                     fields = {
                         'website_id': member.pk,
                         'voornaam': member.first_name,
@@ -57,11 +71,7 @@ class Command(BaseCommand):
                         'postcode': profile.address_postal_code,
                         'plaats': profile.address_city,
                         'land': profile.get_address_country_display(),
-                        'bankrekeningnummer': {
-                            'name': f'{profile.initials} {member.last_name}',
-                            'bic': '',
-                            'iban': profile.bank_account,
-                        },
+                        'bankrekeningnummer': bank_account,
                     }
 
                     replace_commands.append(ApiCommand(

website/members/migrations/0032_remove_profile_bank_account.py

+from django.db import migrations
+from django.utils import timezone
+
+
+def forwards_func(apps, schema_editor):
+    Member = apps.get_model('members', 'profile')
+    BankAccount = apps.get_model('payments', 'BankAccount')
+    db_alias = schema_editor.connection.alias
+    for profile in Member.objects.using(db_alias).exclude(
+            bank_account=None).all():
+        BankAccount.objects.using(db_alias).create(
+            owner=profile.user,
+            initials=profile.initials or profile.user.first_name[0],
+            last_name=profile.user.last_name,
+            valid_from=timezone.now(),
+            mandate_no=f'{profile.user.pk}-1',
+            signature="data:image/png;base64,",
+            iban=profile.bank_account
+        )
+
+
+def reverse_func(apps, schema_editor):
+    BankAccount = apps.get_model('payments', 'BankAccount')
+    db_alias = schema_editor.connection.alias
+    for account in BankAccount.objects.using(db_alias).all():
+        account.owner.profile.bank_account = account.iban
+        account.owner.profile.save()
+        account.delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('members', '0031_benefactor_model_value'),
+        ('payments', '0004_bankaccount'),
+    ]
+
+    operations = [
+        migrations.RunPython(forwards_func, reverse_func),
+    ]

website/members/migrations/0033_auto_20190428_1255.py

+# Generated by Django 2.2 on 2019-04-28 10:55
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('members', '0032_remove_profile_bank_account'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='profile',
+            name='bank_account',
+        ),
+        migrations.RenameField(
+            model_name='profile',
+            old_name='direct_debit_authorized',
+            new_name='auto_renew',
+        ),
+        migrations.AlterField(
+            model_name='profile',
+            name='auto_renew',
+            field=models.BooleanField(choices=[(True, 'Yes, enable auto renewal.'), (False, 'No, manual renewal required.')], default=False, verbose_name='Automatically renew membership'),
+        ),
+    ]

website/members/models.py

@@ -15,8 +15,6 @@ from django.db.models import Q
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import pgettext_lazy, gettext_lazy as _
-from localflavor.generic.countries.sepa import IBAN_SEPA_COUNTRIES
-from localflavor.generic.models import IBANField
 
 from activemembers.models import MemberGroup, MemberGroupMembership
 from utils import countries
@@ -404,27 +402,15 @@ class Profile(models.Model):
         default=True,
     )
 
-    # --- Direct debit information ----
+    # --- Membership preference ----
 
-    direct_debit_authorized = models.BooleanField(
-        choices=((True, _('Yes, I want Thalia to take the membership fees '
-                          'from my bank account through direct debit for '
-                          'each year.')),
-                 (False, _('No, I will pay the contribution myself'))),
-        verbose_name=_('Direct debit'),
-        help_text=_('Each year, have Thalia take the membership fees from my '
-                    'bank account'),
+    auto_renew = models.BooleanField(
+        choices=((True, _('Yes, enable auto renewal.')),
+                 (False, _('No, manual renewal required.'))),
+        verbose_name=_('Automatically renew membership'),
         default=False,
     )
 
-    bank_account = IBANField(
-        verbose_name=_('Bank account'),
-        help_text=_('Bank account for direct debit'),
-        include_countries=IBAN_SEPA_COUNTRIES,
-        blank=True,
-        null=True,
-    )
-
     def display_name(self):
         pref = self.display_name_preference
         if pref == 'nickname' and self.nickname is not None:

website/members/services.py

@@ -199,7 +199,7 @@ def execute_data_minimisation(dry_run=False, members=None):
             profile.emergency_contact_phone_number = None
             profile.emergency_contact = None
             profile.website = None
-            profile.bank_account = None
+            member.bank_accounts.all().delete()
             if not dry_run:
                 profile.save()
 

website/members/templates/members/account.html → website/members/templates/members/user.html

@@ -43,6 +43,13 @@
 
                     <hr>
 
+                    <div>
+                        <a href="{% url 'payments:bankaccount-list' %}">{% trans "manage bank account(s)"|capfirst %}</a>
+                        <p>{% blocktrans %}Change the financial information known to Thalia.{% endblocktrans %}</p>
+                    </div>
+
+                    <hr>
+
                     <div>
                         <a href="{% url 'password_change' %}">{% trans "change password"|capfirst %}</a>
                         <p>{% blocktrans %}Change your accounts' password.{% endblocktrans %}</p>

website/members/urls.py

-from django.conf.urls import url
-from django.urls import path
+from django.urls import path, include
 
 from . import views
 
 app_name = "members"
 
 urlpatterns = [
-    url('^profile/(?P<pk>[0-9]*)$', views.profile, name='profile'),
-    url('^profile/edit/$', views.edit_profile, name='edit-profile'),
-    url('^members/iban-export/$', views.iban_export, name='iban-export'),
-    path('profile/change-email/', views.EmailChangeFormView.as_view(),
-         name='email-change'),
-    path('profile/change-email/verify/<uuid:key>/',
-         views.EmailChangeVerifyView.as_view(), name='email-change-verify'),
-    path('profile/change-email/confirm/<uuid:key>/',
-         views.EmailChangeConfirmView.as_view(), name='email-change-confirm'),
-    url('^$', views.index, name='index'),
+    path('iban-export/', views.iban_export,
+         name='iban-export'),
+    path('members/', include([
+        path('', views.index,
+             name='index'),
+        path('statistics/', views.statistics,
+             name='statistics'),
+        path('profile/<int:pk>', views.profile,
+             name='profile'),
+    ])),
+    path('user/', include([
+        path('', views.user,
+             name='user'),
+        path('edit-profile/', views.edit_profile,
+             name='edit-profile'),
+        path('change-email/', views.EmailChangeFormView.as_view(),
+             name='email-change'),
+        path('change-email/verify/<uuid:key>/',
+             views.EmailChangeVerifyView.as_view(),
+             name='email-change-verify'),
+        path('change-email/confirm/<uuid:key>/',
+             views.EmailChangeConfirmView.as_view(),
+             name='email-change-confirm'),
+    ])),
 ]

website/members/views.py

@@ -168,8 +168,8 @@ def profile(request, pk=None):
 
 
 @login_required
-def account(request):
-    return render(request, 'members/account.html')
+def user(request):
+    return render(request, 'members/user.html')
 
 
 @login_required
@@ -191,18 +191,21 @@ def edit_profile(request):
 
 @permission_required('auth.change_user')
 def iban_export(request):
-    header_fields = ['name', 'username', 'iban']
+    header_fields = ['name', 'username', 'iban', 'bic']
     rows = []
 
     members = models.Member.current_members.filter(
-        profile__direct_debit_authorized=True)
+        profile__auto_renew=True)
 
     for member in members:
-        if member.current_membership.type != 'honorary':
+        if (member.current_membership.type != 'honorary' and
+                member.bank_accounts.exists()):
+            bank_account = member.bank_accounts.last()
             rows.append({
-                'name': member.get_full_name(),
+                'name': bank_account.name,
                 'username': member.username,
-                'iban': member.profile.bank_account
+                'iban': bank_account.iban,
+                'bic': bank_account.bic
             })
 
     response = HttpResponse(content_type='text/csv')

website/payments/admin.py

 """Registers admin interfaces for the payments module"""
 import csv
+from collections import OrderedDict
 
 from django.contrib import admin, messages
+from django.contrib.admin import ModelAdmin
 from django.contrib.admin.utils import model_ngettext
-from django.http import HttpResponse
-from django.urls import path
+from django.db.models import QuerySet
+from django.db.models.query_utils import Q
+from django.http import HttpResponse, HttpRequest
+from django.urls import path, reverse
+from django.utils import timezone
 from django.utils.html import format_html
 from django.utils.text import capfirst
 from django.utils.translation import ugettext_lazy as _
 
+from members.models import Member
 from payments import services, admin_views
-from .models import Payment
+from payments.forms import BankAccountAdminForm
+from .models import Payment, BankAccount
 
 
-def _show_message(admin, request, n, message, error):
+def _show_message(admin: ModelAdmin, request: HttpRequest,
+                  n: int, message: str, error: str) -> None:
     if n == 0:
         admin.message_user(request, error, messages.ERROR)
     else:
@@ -47,7 +55,7 @@ class PaymentAdmin(admin.ModelAdmin):
                'process_wire_selected', 'export_csv']
 
     @staticmethod
-    def _member_link(member):
+    def _member_link(member: Member) -> str:
         if member:
             return format_html("<a href='{}'>{}</a>",
                                member.get_absolute_url(),
@@ -55,18 +63,19 @@ class PaymentAdmin(admin.ModelAdmin):
         else:
             return "-"
 
-    def paid_by_link(self, obj):
+    def paid_by_link(self, obj: Payment) -> str:
         return self._member_link(obj.paid_by)
     paid_by_link.admin_order_field = 'paid_by'
     paid_by_link.short_description = _('paid by')
 
-    def processed_by_link(self, obj):
+    def processed_by_link(self, obj: Payment) -> str:
         return self._member_link(obj.processed_by)
     processed_by_link.admin_order_field = 'processed_by'
     processed_by_link.short_description = _('processed by')
 
-    def changeform_view(self, request, object_id=None, form_url='',
-                        extra_context=None):
+    def changeform_view(self, request: HttpRequest, object_id: int = None,
+                        form_url: str = '', extra_context: dict = None
+                        ) -> HttpResponse:
         """
         Renders the change formview
         Only allow when the payment has not been processed yet
@@ -80,12 +89,12 @@ class PaymentAdmin(admin.ModelAdmin):
         return super().changeform_view(
             request, object_id, form_url, {'payment': obj})
 
-    def get_readonly_fields(self, request, obj=None):
+    def get_readonly_fields(self, request: HttpRequest, obj: Payment = None):
         if not obj:
             return 'created_at', 'type', 'processing_date', 'processed_by'
         return super().get_readonly_fields(request, obj)
 
-    def get_actions(self, request):
+    def get_actions(self, request: HttpRequest) -> OrderedDict:
         """Get the actions for the payments"""
         """Hide the processing actions if the right permissions are missing"""
         actions = super().get_actions(request)
@@ -95,7 +104,8 @@ class PaymentAdmin(admin.ModelAdmin):
             del(actions['process_wire_selected'])
         return actions
 
-    def process_cash_selected(self, request, queryset):
+    def process_cash_selected(self, request: HttpRequest,
+                              queryset: QuerySet) -> None:
         """Process the selected payment as cash"""
         if request.user.has_perm('payments.process_payments'):
             updated_payments = services.process_payment(
@@ -105,7 +115,8 @@ class PaymentAdmin(admin.ModelAdmin):
     process_cash_selected.short_description = _(
         'Process selected payments (cash)')
 
-    def process_card_selected(self, request, queryset):
+    def process_card_selected(self, request: HttpRequest,
+                              queryset: QuerySet) -> None:
         """Process the selected payment as card"""
         if request.user.has_perm('payments.process_payments'):
             updated_payments = services.process_payment(
@@ -115,7 +126,8 @@ class PaymentAdmin(admin.ModelAdmin):
     process_card_selected.short_description = _(
         'Process selected payments (card)')
 
-    def process_wire_selected(self, request, queryset):
+    def process_wire_selected(self, request: HttpRequest,
+                              queryset: QuerySet) -> None:
         """Process the selected payment as wire"""
         if request.user.has_perm('payments.process_payments'):
             updated_payments = services.process_payment(
@@ -125,7 +137,7 @@ class PaymentAdmin(admin.ModelAdmin):
     process_wire_selected.short_description = _(
         'Process selected payments (wire)')
 
-    def _process_feedback(self, request, updated_payments):
+    def _process_feedback(self, request, updated_payments: list) -> None:
         """Show a feedback message for the processing result"""
         rows_updated = len(updated_payments)
         _show_message(
@@ -134,7 +146,7 @@ class PaymentAdmin(admin.ModelAdmin):
             error=_('The selected payment(s) could not be processed.')
         )
 
-    def get_urls(self):
+    def get_urls(self) -> list:
         urls = super().get_urls()
         custom_urls = [
             path('<uuid:pk>/process/',
@@ -144,7 +156,13 @@ class PaymentAdmin(admin.ModelAdmin):
         ]
         return custom_urls + urls
 
-    def export_csv(self, request, queryset):
+    def export_csv(self, request: HttpRequest,
+                   queryset: QuerySet) -> HttpResponse:
+        """
+        Export a CSV of payments
+        :param request: Request
+        :param queryset: Items to be exported
+        """
         response = HttpResponse(content_type='text/csv')
         response['Content-Disposition'] = 'attachment;\
                                            filename="payments.csv"'
@@ -169,3 +187,94 @@ class PaymentAdmin(admin.ModelAdmin):
             ])
         return response
     export_csv.short_description = _('Export')
+
+
+class ValidAccountFilter(admin.SimpleListFilter):
+    """Filter the memberships by whether they are active or not"""
+    title = _('mandates')
+    parameter_name = 'active'
+
+    def lookups(self, request, model_name) -> tuple:
+        return (
+            ('valid', _('Valid')),
+            ('invalid', _('Invalid')),
+            ('none', _('None')),
+        )
+
+    def queryset(self, request, queryset) -> QuerySet:
+        now = timezone.now()
+
+        if self.value() == 'valid':
+            return queryset.filter(Q(valid_from__lte=now) &
+                                   Q(valid_until=None) |
+                                   Q(valid_until__lt=now))
+
+        if self.value() == 'invalid':
+            return queryset.filter(valid_until__gte=now)
+
+        if self.value() == 'none':
+            return queryset.filter(valid_from=None)
+
+        return queryset
+
+
+@admin.register(BankAccount)
+class BankAccountAdmin(admin.ModelAdmin):
+    """Manage bank accounts"""
+
+    list_display = ('iban', 'owner_link', 'last_used',
+                    'valid_from', 'valid_until')
+    list_filter = (ValidAccountFilter,)