fix(*): various bugs discovered by tests

jdav_web/finance/tests/admin.py

@@ -110,13 +110,13 @@ class StatementUnSubmittedAdminTestCase(AdminTestCase):
         readonly_fields = self.admin.get_readonly_fields(None, self.statement)
         self.assertEqual(readonly_fields, ['submitted', 'excursion'])
 
-    @unittest.skip('Request returns 200, but should give insufficient permissions.')
     def test_submit_view_insufficient_permission(self):
         url = reverse('admin:finance_statementunsubmitted_submit',
                       args=(self.statement.pk,))
         c = self._login('standard')
         response = c.get(url, follow=True)
-        self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN)
+        self.assertEqual(response.status_code, HTTPStatus.OK)
+        self.assertContains(response, _('Insufficient permissions.'))
 
     def test_submit_view_get(self):
         url = reverse('admin:finance_statementunsubmitted_submit',
@@ -247,16 +247,6 @@ class StatementSubmittedAdminTestCase(AdminTestCase):
             paid_by=self.member
         )
 
-    def _add_session_to_request(self, request):
-        """Add session to request"""
-        middleware = SessionMiddleware(lambda req: None)
-        middleware.process_request(request)
-        request.session.save()
-
-        middleware = MessageMiddleware(lambda req: None)
-        middleware.process_request(request)
-        request._messages = FallbackStorage(request)
-
     def test_has_add_permission(self):
         """Test that add permission is disabled"""
         request = self.factory.get('/')

jdav_web/members/admin.py

@@ -480,13 +480,15 @@ class MemberUnconfirmedAdmin(CommonAdminMixin, admin.ModelAdmin):
         notify_individual = len(queryset.all()) < 10
         success = True
         for member in queryset:
-            if member.confirm() and notify_individual:
+            confirmed = member.confirm()
+            if not confirmed:
+                success = False
+            if notify_individual:
+                if confirmed:
                     messages.success(request, _("Successfully confirmed %(name)s.") % {'name': member.name})
                 else:
-                if notify_individual:
                     messages.error(request,
                             _("Can't confirm. %(name)s has unconfirmed email addresses.") % {'name': member.name})
-                success = False
         if notify_individual:
             return
         if success:
@@ -1340,13 +1342,13 @@ class KlettertreffAdmin(admin.ModelAdmin):
     inlines = [KlettertreffAttendeeInline]
     list_display = ['__str__', 'date', 'get_jugendleiter']
     search_fields = ('date', 'location', 'topic')
-    list_filter = [('date', DateFieldListFilter), 'group__name']
+    list_filter = [('date', DateFieldListFilter), 'group']
     actions = ['overview']
 
     def overview(self, request, queryset):
-        group = request.GET.get('group__name')
+        group = request.GET.get('group__id__exact')
         if group != None:
-            members = Member.objects.filter(group__name__contains=group)
+            members = Member.objects.filter(group=group)
         else:
             members = Member.objects.all()
         context = {

jdav_web/members/models.py

@@ -1305,9 +1305,19 @@ class Freizeit(CommonModel):
     @property
     def duration(self):
         # number of nights is number of full days + 1
-        full_days = self.night_count - 1
+        full_days = max(self.night_count - 1, 0)
         extra_days = 0
 
+        if self.date.date() == self.end.date():
+            # excursion starts and ends on the same day
+            hours = max(self.end.hour - self.date.hour, 0)
+            # at least 6 hours counts as full day
+            if hours >= 6:
+                extra_days = 1.0
+            # otherwise half day
+            else:
+                extra_days = 0.5
+        else:
             if self.date.hour <= 12:
                 extra_days += 1.0
             else:

jdav_web/members/tests/basic.py

@@ -936,14 +936,31 @@ class FreizeitTestCase(BasicMemberTestCase):
 
     def test_duration(self):
         self.assertGreaterEqual(self.ex.duration, 0)
+
+        # less than 6 hours
         self.ex.date = timezone.datetime(2000, 1, 1, 8, 0, 0)
         self.ex.end = timezone.datetime(2000, 1, 1, 10, 0, 0)
         self.assertEqual(self.ex.duration, 0.5)
 
-        # TODO: fix this in the model, the duration of this excursion should be 0
+        # at least 6 hours
+        self.ex.date = timezone.datetime(2000, 1, 1, 8, 0, 0)
+        self.ex.end = timezone.datetime(2000, 1, 1, 14, 0, 0)
+        self.assertEqual(self.ex.duration, 1)
+
+        # one full day and two extra days on beginning and end
+        self.ex.date = timezone.datetime(2000, 1, 1, 8, 0, 0)
+        self.ex.end = timezone.datetime(2000, 1, 3, 14, 0, 0)
+        self.assertEqual(self.ex.duration, 3)
+
+        # one full day and two half days on beginning and end
+        self.ex.date = timezone.datetime(2000, 1, 1, 16, 0, 0)
+        self.ex.end = timezone.datetime(2000, 1, 3, 8, 0, 0)
+        self.assertEqual(self.ex.duration, 2)
+
+    def test_duration_midday_midday(self):
         self.ex.date = timezone.datetime(2000, 1, 1, 12, 0, 0)
         self.ex.end = timezone.datetime(2000, 1, 1, 12, 0, 0)
-        self.assertEqual(self.ex.duration, 1)
+        self.assertEqual(self.ex.duration, 0.5)
 
     def test_generate_ljp_vbk_no_proposal_raises_error(self):
         """Test generate_ljp_vbk raises ValueError when excursion has no LJP proposal"""
@@ -1438,7 +1455,6 @@ class MemberUnconfirmedAdminTestCase(AdminTestCase):
         response = c.post(url, data={'action': 'confirm', '_selected_action': [self.reg.pk]}, follow=True)
         self.assertEqual(response.status_code, HTTPStatus.OK)
 
-    @skip('Even when every `member.confirm()` succeeds, it still shows the error message.')
     def test_confirm_multiple(self):
         c = self._login('superuser')
         url = reverse('admin:members_memberunconfirmedproxy_changelist')
@@ -2190,18 +2206,17 @@ class KlettertreffAdminTestCase(AdminTestCase):
                                      '_selected_action': [kl.pk for kl in qs]}, follow=True)
         self.assertEqual(response.status_code, HTTPStatus.OK)
 
-    @skip('Members are not filtered by group, because group attribute is retrieved from GET data.')
     def test_overview_filtered(self):
         qs = Klettertreff.objects.all()
-        url = reverse('admin:members_klettertreff_changelist')
+        cool_kids = Group.objects.get(name='cool kids')
+        url = reverse('admin:members_klettertreff_changelist') + f"?group__id__exact={cool_kids.pk}"
 
         # expect: success and filtered by group
         c = self._login('superuser')
         response = c.post(url, data={'action': 'overview',
-                                     'group__name': 'cool kids',
                                      '_selected_action': [kl.pk for kl in qs]}, follow=True)
         self.assertEqual(response.status_code, HTTPStatus.OK)
-        self.assertNotContains(response, 'Lulla')
+        self.assertNotContains(response, 'Lise')
 
 
 class GroupAdminTestCase(AdminTestCase):