[WIP] CLDT seat implementation #2689

froggleston · 2024-08-19T15:16:58Z

This PR is a WIP to implement CLDT seat allocation and tracking

pbanaszkiewicz · 2024-08-19T20:06:24Z

amy/workshops/models.py

@@ -140,6 +140,10 @@ class Meta:

 class MembershipManager(models.Manager):
    def annotate_with_seat_usage(self):
+        cldt_tag = Tag.objects.get(name="CLDT")
+        # TTT/ITT included
+        ttt_tags = Tag.objects.exclude(name__in=["SWC", "DC", "LC", "WiSE", "CLDT"])


In here (L143, L145) use tag names:

cldt_tag_name = "CLDT" ttt_tag_names = ["TTT", "ITT"]

Then the lookups using ttt_tags or cldt_tag have to be changed:

from task__event__tags=[ttt_tags] to task__event__tags__name__in=ttt_tag_names

from task__event__tags=[cldt_tag] to task__event__tags__name__in=[cldt_tag_name]

pbanaszkiewicz · 2024-09-23T18:37:36Z

amy/workshops/models.py

+    @cached_property
+    def cldt_seats_utilized(self) -> int:
+        """Count number of learner tasks that point to this membership."""
+        return self.task_set.filter(role__name="learner", seat_public=False).count()


Since this property should mimick the query in MembershipManager.annotate_with_seat_usage, there's additional filtering missing:

cld_seats_utilized() should also filter by event tag CLDT

in MembershipManager.annotate_with_seat_usage, _cldt_seats_utilized should also filter by seat_public

in MembershipManager.annotate_with_seat_usage, _cldt_seats_remaining should filter by seat_public in the same way (currently it uses public=True)

both public_instructor_training_seats_utilized() and inhouse_instructor_training_seats_utilized() should also filter by TTT tags (TTT, ITT)

Generally speaking the model properties should come down to the same equations as in annotations.

Updated all calculations and set CLDT to public=False as these events are member based and not open to the public.

pbanaszkiewicz · 2024-09-23T18:58:36Z

amy/templates/reports/membership_trainings_stats.html

@@ -35,6 +36,9 @@
        <td>{{ result.instructor_training_seats_total }}</td>
        <td>{{ result.instructor_training_seats_utilized }}</td>
        <td>{{ result.instructor_training_seats_remaining }}</td>
+        <td>{{ result._cldt_training_seats_total }}</td>
+        <td>{{ result._cldt_training_seats_utilized }}</td>
+        <td>{{ result._cldt_training_seats_remaining }}</td>


Django template is complaining about these names. Apparently they can't start with _, so my proposition is to use these names instead:

cldt_seats_total_annotation

cldt_seats_utilized_annotation

cldt_seats_remaining_annotation

Changes should be applied here and in MembershipManager.annotate_with_seat_usage.

I've changed the CLDT annotation variable names but not the others. We can do this after merge/rebase?

I think only CLDT annotations started with _, so only they needed to be updated with that prefix. Other annotations work just fine because they don't have name clash with model properties.

pbanaszkiewicz · 2024-09-23T19:00:04Z

amy/templates/reports/membership_trainings_stats.html

@@ -16,6 +16,7 @@
        <th rowspan="2">Agreement</th>
        <th rowspan="2">Contribution</th>
        <th colspan="3" class="text-center">Instructor training seats (combined public and in-house)</th>
+        <th colspan="3" class="text-center">CLDT seats</th>


This introduces 3 new columns in the row that's following (lines 23-25 below). Can you duplicate those lines? This way the header displays correctly:

pbanaszkiewicz · 2024-09-23T19:02:40Z

amy/templates/fiscal/all_memberships.html

+            {{ membership.instructor_training_seats_remaining }} / {{ membership.instructor_training_seats_total }}
+          </td>
+          <td {% if membership._cldt_seats_remaining <= 0 and membership._cldt_seats_total > 0 or membership._cldt_seats_remaining < 0 and membership._cldt_seats_total == 0 %}class="table-danger"{% endif %}>
+            {{ membership._cldt_seats_remaining }} / {{ membership._cldt_seats_total }}


Django template also complains here, so I suggest applying these changes in the lines above:

_cldt_seats_remaining -> cldt_seats_remaining_annotation

_cldt_seats_total -> cldt_seats_total_annotation

pbanaszkiewicz · 2024-09-23T19:04:32Z

amy/templates/fiscal/all_memberships.html

@@ -41,7 +42,10 @@
          <td>{{ membership.agreement_start|date:"Y-m-d" }} &mdash; {{ membership.agreement_end|date:"Y-m-d" }}</td>
          <td>{{ membership.get_contribution_type_display }}</td>
          <td {% if membership.instructor_training_seats_remaining <= 0 and membership.instructor_training_seats_total > 0 or membership.instructor_training_seats_remaining < 0 and membership.instructor_training_seats_total == 0 %}class="table-danger"{% endif %}>
-            {{ membership.instructor_training_seats_remaining }}
+            {{ membership.instructor_training_seats_remaining }} / {{ membership.instructor_training_seats_total }}


This notation (remaining count / total count) is clever, but I'm wondering if users would know this is remaining / total, and not utilized / total.

I'll check this with the CT, but as these calculations have been in place for the other seat types, I wouldn't want to change it without consulting them.

pbanaszkiewicz · 2024-09-30T19:43:03Z

amy/workshops/models.py

-        cldt_tag_name = "CLDT"
-        ttt_tag_names = ["TTT", "ITT"]
+        # cldt_tag_name = "CLDT"
+        # ttt_tag_names = ["TTT", "ITT"]


Can you remove this commented-out code?

pbanaszkiewicz · 2024-09-30T19:44:12Z

amy/workshops/consts.py

@@ -5,3 +5,6 @@
 STR_LONG = 100  # length of long strings
 STR_LONGEST = 255  # length of the longest strings
 STR_REG_KEY = 20  # length of Eventbrite registration key
+
+CLDT_TAG_NAME = ["CLDT"]


I'd keep this plural (nameS) since this is a list.

froggleston added 5 commits June 21, 2024 16:13

[WIP] First pass at adding CLDT seat functionality

b00cd6c

Merge develop

5eb218a

Refactor based on first review

a0ec5a7

Adjust memberships table to include CLDT

37303cd

Adjust organisation view template to include CLDT

bc7c4b3

froggleston changed the title ~~Cldt seats~~ [WIP] CLDT seat implementation Aug 19, 2024

pbanaszkiewicz reviewed Aug 19, 2024

View reviewed changes

Fix CLDT seat membership models, update templates, add CLDT migrations

4665d17

pbanaszkiewicz reviewed Sep 23, 2024

View reviewed changes

Update annotation names, add filters, update templates

31a6b92

pbanaszkiewicz reviewed Sep 30, 2024

View reviewed changes

pbanaszkiewicz added the priority: defer label Jan 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] CLDT seat implementation #2689

[WIP] CLDT seat implementation #2689

froggleston commented Aug 19, 2024

pbanaszkiewicz Aug 19, 2024

pbanaszkiewicz Sep 23, 2024

froggleston Sep 30, 2024

pbanaszkiewicz Sep 23, 2024

froggleston Sep 30, 2024

pbanaszkiewicz Sep 30, 2024

pbanaszkiewicz Sep 23, 2024

froggleston Sep 30, 2024

pbanaszkiewicz Sep 23, 2024

froggleston Sep 30, 2024

pbanaszkiewicz Sep 23, 2024 •

edited

Loading

froggleston Sep 30, 2024

pbanaszkiewicz Sep 30, 2024

pbanaszkiewicz Sep 30, 2024

[WIP] CLDT seat implementation #2689

Are you sure you want to change the base?

[WIP] CLDT seat implementation #2689

Conversation

froggleston commented Aug 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbanaszkiewicz Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbanaszkiewicz Sep 23, 2024 •

edited

Loading