Repository: carpentries/amy
Issue: Counting workshops with memberships

I have seen some discrepancies in how workshops are being counted on the membership page. For example: 

On a [member view page like this](https://amy.carpentries.org/fiscal/membership/253/) we see that  they have completed 4 of 6 allowed workshops plus 1 self organized workshop.

Yet when I do `SELECT * FROM workshops_event WHERE membership_id = 253`  I get 8 workshops total (1 self organized and 7 centrally organized).  

Where is this discrepancy coming from? 

Changed files:
amy/fiscal/tests/test_membership.py, amy/workshops/models.py

diff --git a/amy/fiscal/tests/test_membership.py b/amy/fiscal/tests/test_membership.py
index e703e744..9df1d951 100644
--- a/amy/fiscal/tests/test_membership.py
+++ b/amy/fiscal/tests/test_membership.py
@@ -172,8 +172,8 @@ class TestMembership(TestBase):
         self.setUpTasks()
         self.assertEqual(self.current.workshops_without_admin_fee_per_agreement, 10)
         self.assertEqual(self.current.workshops_without_admin_fee_total_allowed, 10)
-        self.assertEqual(self.current.workshops_without_admin_fee_completed, 6)
-        self.assertEqual(self.current.workshops_without_admin_fee_planned, 4)
+        self.assertEqual(self.current.workshops_without_admin_fee_completed, 7)
+        self.assertEqual(self.current.workshops_without_admin_fee_planned, 3)
         self.assertEqual(self.current.workshops_without_admin_fee_remaining, 0)
 
     def test_delete_membership(self):
diff --git a/amy/workshops/models.py b/amy/workshops/models.py
index 2014da6e..ce028d54 100644
--- a/amy/workshops/models.py
+++ b/amy/workshops/models.py
@@ -311,20 +311,18 @@ class Membership(models.Model):
     def get_absolute_url(self):
         return reverse("membership_details", args=[self.id])
 
+    def _base_queryset(self):
+        """Provide universal queryset for looking up workshops for this membership."""
+        cancelled = Q(tags__name="cancelled") | Q(tags__name="stalled")
+        return Event.objects.filter(membership=self).exclude(cancelled).distinct()
+
     def _workshops_without_admin_fee_queryset(self):
         """Provide universal queryset for looking up centrally-organised workshops for
         this membership."""
-        during_membership = Q(
-            start__gte=self.agreement_start, start__lt=self.agreement_end
-        )
-        cancelled = Q(tags__name="cancelled") | Q(tags__name="stalled")
         return (
-            Event.objects.filter(during_membership)
-            .filter(membership=self)
+            self._base_queryset()
             .filter(administrator__in=Organization.objects.administrators())
             .exclude(administrator__domain="self-organized")
-            .exclude(cancelled)
-            .distinct()
         )
 
     def _workshops_without_admin_fee_completed_queryset(self):
@@ -365,7 +363,7 @@ class Membership(models.Model):
 
     @cached_property
     def workshops_without_admin_fee_completed(self) -> int:
-        """Count centrally-organised workshops already hosted during the agreement.
+        """Count centrally-organised workshops already hosted by this membership.
 
         This value must not be higher than "contracted" (or available for counting)
         no-fee workshops.
@@ -378,7 +376,7 @@ class Membership(models.Model):
 
     @cached_property
     def workshops_without_admin_fee_planned(self) -> int:
-        """Count centrally-organised workshops hosted in future during the agreement.
+        """Count centrally-organised workshops hosted in future by this membership.
 
         This value must not be higher than "contracted" (or available for counting)
         no-fee workshops reduced by already completed no-fee workshops.
@@ -423,20 +421,10 @@ class Membership(models.Model):
     def _self_organized_workshops_queryset(self):
         """Provide universal queryset for looking up self-organised events for this
         membership."""
-        during_membership = Q(
-            start__gte=self.agreement_start, start__lt=self.agreement_end
-        )
-        cancelled = Q(tags__name="cancelled") | Q(tags__name="stalled")
         self_organized = Q(administrator=None) | Q(
             administrator__domain="self-organized"
         )
-        return (
-            Event.objects.filter(during_membership)
-            .filter(membership=self)
-            .filter(self_organized)
-            .exclude(cancelled)
-            .distinct()
-        )
+        return self._base_queryset().filter(self_organized)
 
     @cached_property
     def self_organized_workshops_completed(self):