Finish tests for payments views

website/payments/admin.py

@@ -20,7 +20,8 @@ from payments.forms import BankAccountAdminForm
 from .models import Payment, BankAccount
 
 
-def _show_message(admin: ModelAdmin, request: HttpRequest, n: int, message: str, error: str) -> None:
+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:

website/payments/tests/test_views.py

+
+from django.contrib.auth import get_user_model
+from django.test import Client, TestCase
+from django.urls import reverse
+
+from members.models import Member
+from payments.models import BankAccount
+from payments.views import BankAccountCreateView, BankAccountListView
+
+
+class BankAccountCreateViewTest(TestCase):
+    """
+    Test for the BankAccountCreateView
+    """
+    fixtures = ['members.json']
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.login_user = Member.objects.filter(last_name="Wiggers").first()
+        cls.new_user = get_user_model().objects.create_user(
+            username='username',
+            first_name='Johnny',
+            last_name='Test'
+        )
+        cls.account = BankAccount.objects.create(
+            owner=cls.login_user,
+            initials='J',
+            last_name='Test',
+            iban='NL91ABNA0417164300'
+        )
+        BankAccount.objects.create(
+            owner=None,
+            initials='Someone',
+            last_name='Else',
+            iban='BE68539007547034'
+        )
+
+    def setUp(self):
+        self.view = BankAccountCreateView()
+        self.client = Client()
+        self.client.force_login(self.login_user)
+
+    def test_not_logged_in(self):
+        """
+        If there is no logged-in user they should redirect
+        to the authentication page
+        """
+        self.client.logout()
+
+        response = self.client.get(
+            reverse('payments:bankaccount-add'),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [('/login/?next=' +
+              reverse('payments:bankaccount-add'), 302)],
+            response.redirect_chain
+        )
+
+    def test_not_a_current_member(self):
+        """
+        If the logged-in user is not a member they should not be able to visit
+        this page
+        """
+        self.client.logout()
+        self.client.force_login(self.new_user)
+
+        response = self.client.get(reverse('payments:bankaccount-add'))
+        self.assertEqual(403, response.status_code)
+
+    def test_shows_correct_reference(self):
+        """
+        The page should show the reference that will be used to identify
+        a new mandate if direct debit is enabled
+        """
+        response = self.client.get(
+            reverse('payments:bankaccount-add'),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual('1-1', response.context['mandate_no'])
+        self.assertContains(
+            response,
+            '1-1'
+        )
+
+    def test_account_no_mandate_saves_correctly(self):
+        """
+        If an account with direct debit enabled is saved
+        we should redirect to the account list page
+        with a success alert showing. And the BankAccount should be the only
+        one in the account since the previous one had no mandate.
+        BankAccounts by others should be untouched.
+        """
+        response = self.client.post(
+            reverse('payments:bankaccount-add'),
+            data={
+                'initials': 'S',
+                'last_name': 'Versteeg',
+                'iban': 'DE12500105170648489890',
+                'bic': 'NBBEBEBB'
+            },
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [(reverse('payments:bankaccount-list'), 302)],
+            response.redirect_chain
+        )
+        self.assertContains(
+            response,
+            'Bank account saved successfully.'
+        )
+
+        self.assertEqual(
+            1,
+            BankAccount.objects.filter(
+                owner=self.login_user
+            ).count()
+        )
+
+        self.assertTrue(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='DE12500105170648489890'
+            ).exists()
+        )
+
+        self.assertTrue(
+            BankAccount.objects.filter(
+                iban='BE68539007547034'
+            ).exists()
+        )
+
+    def test_account_with_mandate_saves_correctly(self):
+        """
+        If an account with direct debit enabled is saved
+        we should redirect to the account list page
+        with a success alert showing. And the BankAccount should be the only
+        one in the account since the previous one had no mandate.
+        BankAccounts by others should be untouched.
+        """
+        response = self.client.post(
+            reverse('payments:bankaccount-add'),
+            data={
+                'initials': 'S',
+                'last_name': 'Versteeg',
+                'iban': 'DE12500105170648489890',
+                'bic': 'NBBEBEBB',
+                'direct_debit': '',
+                'signature': 'sig'
+            },
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [(reverse('payments:bankaccount-list'), 302)],
+            response.redirect_chain
+        )
+        self.assertContains(
+            response,
+            'Bank account saved successfully.'
+        )
+
+        self.assertEqual(
+            1,
+            BankAccount.objects.filter(
+                owner=self.login_user
+            ).count()
+        )
+
+        self.assertTrue(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='DE12500105170648489890',
+                mandate_no='1-1'
+            ).exists()
+        )
+
+        self.assertTrue(
+            BankAccount.objects.filter(
+                iban='BE68539007547034'
+            ).exists()
+        )
+
+    def test_account_save_keeps_old_mandates(self):
+        """
+        If an account is saved and there are previous accounts that
+        were authorised for direct debit then we should keep those
+        but they should be revoked.
+        """
+        BankAccount.objects.create(
+            owner=self.login_user,
+            initials='J',
+            last_name='Test',
+            iban='NL91ABNA0417164300',
+            valid_from='2019-03-01',
+            signature='sig',
+            mandate_no='11-2'
+        )
+
+        self.client.post(
+            reverse('payments:bankaccount-add'),
+            data={
+                'initials': 'S',
+                'last_name': 'Versteeg',
+                'iban': 'DE12500105170648489890',
+                'bic': 'NBBEBEBB'
+            },
+            follow=True
+        )
+
+        self.assertEqual(
+            2,
+            BankAccount.objects.filter(
+                owner=self.login_user
+            ).count()
+        )
+
+        self.assertTrue(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='DE12500105170648489890'
+            ).exists()
+        )
+
+        self.assertFalse(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='NL91ABNA0417164300'
+            ).first().valid
+        )
+
+
+class BankAccountRevokeViewTest(TestCase):
+    """
+    Test for the BankAccountRevokeView
+    """
+    fixtures = ['members.json']
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.login_user = Member.objects.filter(last_name="Wiggers").first()
+        cls.account = BankAccount.objects.create(
+            owner=cls.login_user,
+            initials='J',
+            last_name='Test',
+            iban='NL91ABNA0417164300'
+        )
+        BankAccount.objects.create(
+            owner=None,
+            initials='Someone',
+            last_name='Else',
+            iban='BE68539007547034',
+            bic='NBBEBEBB',
+        )
+
+    def setUp(self):
+        self.view = BankAccountCreateView()
+        self.client = Client()
+        self.client.force_login(self.login_user)
+
+    def test_not_logged_in(self):
+        """
+        If there is no logged-in user they should redirect
+        to the authentication page
+        """
+        self.client.logout()
+
+        response = self.client.get(
+            reverse('payments:bankaccount-add'),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [('/login/?next=' +
+              reverse('payments:bankaccount-add'), 302)],
+            response.redirect_chain
+        )
+
+
+class BankAccountListViewTest(TestCase):
+    """
+    Test for the BankAccountListView
+    """
+    fixtures = ['members.json']
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.login_user = Member.objects.filter(last_name="Wiggers").first()
+        cls.account1 = BankAccount.objects.create(
+            owner=cls.login_user,
+            initials='J1',
+            last_name='Test',
+            iban='NL91ABNA0417164300'
+        )
+        cls.account2 = BankAccount.objects.create(
+            owner=cls.login_user,
+            initials='J2',
+            last_name='Test',
+            iban='BE68539007547034',
+            bic='NBBEBEBB',
+            valid_from='2019-03-01',
+            signature='sig',
+            mandate_no='11-2'
+        )
+
+    def setUp(self):
+        self.account1.refresh_from_db()
+        self.account2.refresh_from_db()
+        self.view = BankAccountListView()
+        self.client = Client()
+        self.client.force_login(self.login_user)
+
+    def test_not_logged_in(self):
+        """
+        If there is no logged-in user they should redirect
+        to the authentication page
+        """
+        self.client.logout()
+
+        response = self.client.post(
+            reverse('payments:bankaccount-revoke', args=(self.account1.pk,)),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [('/login/?next=' +
+              reverse('payments:bankaccount-revoke',
+                      args=(self.account1.pk,)), 302)],
+            response.redirect_chain
+        )
+
+    def test_no_post(self):
+        """
+        If the request is not a post it should redirect to the list
+        """
+        response = self.client.get(
+            reverse('payments:bankaccount-revoke', args=(self.account2.pk,)),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [(reverse('payments:bankaccount-list'), 302)],
+            response.redirect_chain
+        )
+
+    def test_cannot_revoke_no_mandate(self):
+        """
+        If the selected account has no valid mandate it should return a 404
+        """
+        self.account2.valid_until = '2019-04-01'
+        self.account2.save()
+
+        response = self.client.post(
+            reverse('payments:bankaccount-revoke', args=(self.account1.pk,)),
+            follow=True
+        )
+        self.assertEqual(404, response.status_code)
+
+        response = self.client.post(
+            reverse('payments:bankaccount-revoke', args=(self.account2.pk,)),
+            follow=True
+        )
+        self.assertEqual(404, response.status_code)
+
+    def test_revoke_successful(self):
+        """
+        If an account with direct debit is revoked it should
+        redirect to the list with the right success message.
+        """
+        self.assertTrue(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='BE68539007547034',
+            ).first().valid
+        )
+
+        response = self.client.post(
+            reverse('payments:bankaccount-revoke', args=(self.account2.pk,)),
+            follow=True
+        )
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [(reverse('payments:bankaccount-list'), 302)],
+            response.redirect_chain
+        )
+        self.assertContains(
+            response,
+            'Direct debit authorisation successfully revoked.'
+        )
+
+        self.assertFalse(
+            BankAccount.objects.filter(
+                owner=self.login_user,
+                iban='BE68539007547034',
+            ).first().valid
+        )

website/payments/views.py

@@ -2,7 +2,8 @@ from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.messages.views import SuccessMessageMixin
 from django.db.models import QuerySet
-from django.http import Http404, HttpResponse
+from django.http import HttpResponse
+from django.shortcuts import redirect
 from django.urls import reverse_lazy
 from django.utils import timezone
 from django.utils.decorators import method_decorator
@@ -64,10 +65,15 @@ class BankAccountRevokeView(SuccessMessageMixin, UpdateView):
     success_message = _('Direct debit authorisation successfully revoked.')
 
     def get_queryset(self) -> QuerySet:
-        return super().get_queryset().filter(owner=self.request.member)
+        return super().get_queryset().filter(
+            owner=self.request.member,
+            valid_until=None,
+        ).exclude(
+            mandate_no=None
+        )
 
-    def get(self, **kwargs) -> HttpResponse:
-        raise Http404
+    def get(self, *args, **kwargs) -> HttpResponse:
+        return redirect('payments:bankaccount-list')
 
     def post(self, request, *args, **kwargs) -> HttpResponse:
         request.POST = request.POST.dict()