Merge pull request #348 from esrg-knights/hotfix/member-room-access-u…

…pdate

Room access requirements + card updates

core/templates/martor/bootstrap/guide.html

@@ -109,7 +109,7 @@ <h5 class="modal-title" id="modalMarkdownGuideTitle">
               <td>Ctrl+L</td>
               <td>Command+L</td>
               <!-- BEGIN CHANGES -->
-              <td><a href="https://www.youtube.com/watch?v=dQw4w9WgXcQ">Link</a></td>
+              <td><a href="https://www.youtube.com/watch?v=1iQzFLO8xeE">Link</a></td>
               <!-- END CHANGES -->
             </tr>
             <tr>

membership_file/admin.py

@@ -164,7 +164,7 @@ def get_export_filename(self, request, queryset, file_format):
             ('street', 'house_number', 'house_number_addition'), ('postal_code', 'city'), 'country']}),
         ('Room Access', {'fields':
             ['key_id', 'tue_card_number',
-            ('external_card_number', 'external_card_digits', 'external_card_cluster'),
+            ('external_card_number', 'external_card_digits'),
             'external_card_deposit', 'accessible_rooms']}),
         ('Legal Information', {'fields':
             ['educational_institution', 'student_number',
@@ -266,13 +266,29 @@ class MemberLogReadOnly(DisableModificationsAdminMixin, HideRelatedNameAdmin):
 class RoomAdmin(admin.ModelAdmin):
     model = Room
 
-    list_display = ("id", "name", "access")
+    list_display = ("id", "name", "room_number", "access_type", "access_specification")
     list_display_links = ("id", "name")
-    search_fields = ["name", "access"]
-
-    ordering = ("access",)
+    search_fields = ["name", "room_number"]
+    ordering = ("access_type", "access_specification")
     filter_horizontal = ("members_with_access",)
 
+    fieldsets = [
+        (
+            None,
+            {
+                "fields": [
+                    (
+                        "name",
+                        "room_number",
+                    ),
+                    ("access_type", "access_specification"),
+                    "notes",
+                    "members_with_access",
+                ]
+            },
+        ),
+    ]
+
 
 @admin.register(MemberYear)
 class MemberYearAdmin(ExportActionMixin, admin.ModelAdmin):

membership_file/fixtures/test_export_members.json

@@ -10,9 +10,8 @@
           "last_name": "Doe",
           "student_number": "1234567",
           "educational_institution": "TUe",
-          "external_card_number": "1234567",
+          "external_card_number": "01234567",
           "external_card_digits": "",
-          "external_card_cluster": "Luna Cluster 3",
           "email": "johndoe@example.com",
           "last_updated_date": "1970-01-01T12:00:00Z",
           "last_updated_by": null
@@ -38,7 +37,8 @@
         "pk": 1,
         "fields": {
           "name": "Room 1",
-          "access": "Key 12",
+          "access_type": "KEY",
+          "access_specification": "12",
           "members_with_access": [
             1
           ]
@@ -49,7 +49,7 @@
         "pk": 2,
         "fields": {
           "name": "Room 2",
-          "access": "Campus Card",
+          "access_type": "CARD",
           "members_with_access": [
             1
           ]

membership_file/fixtures/test_members.json

@@ -10,10 +10,9 @@
          "last_name": "Dommel",
          "student_number": "0987654",
          "educational_institution": "TUe",
-         "tue_card_number": "1234567",
+         "tue_card_number": "01234567",
          "external_card_number": null,
          "external_card_digits": "",
-         "external_card_cluster": "",
          "email": "linked_member@example.com",
          "phone_number": null,
          "street": "John F. Kennedylaan",
@@ -44,7 +43,6 @@
          "tue_card_number": null,
          "external_card_number": null,
          "external_card_digits": "",
-         "external_card_cluster": "",
          "email": "nonlinked_member@example.com",
          "phone_number": null,
          "street": "Limbopad",

membership_file/forms.py

@@ -327,10 +327,24 @@ def clean(self) -> Dict[str, Any]:
         res = super().clean()
         # Phone number requirements
         if not self.cleaned_data["phone_number"] and self.cleaned_data["room_access"]:
-            self.add_error(
-                "phone_number",
-                ValidationError("A phone number is required if room access is provided.", code="phone_required"),
+            any_room_not_card = any(
+                [
+                    Room.objects.get(id=room_id).access_type != Room.ACCESS_CARD
+                    for room_id in self.cleaned_data["room_access"]
+                ]
             )
+            # There is >=1 room that uses a key, or all rooms require card access and no TUe card number was provided
+            if any_room_not_card or not self.cleaned_data["tue_card_number"]:
+                msg = "A phone number is required; access was requested for rooms with keys."
+                if not any_room_not_card:
+                    msg = "A phone number is required, or a TUe card number should be entered. Access was only requested for rooms with card access."
+                self.add_error(
+                    "phone_number",
+                    ValidationError(
+                        msg,
+                        code="phone_required",
+                    ),
+                )
 
         # Educational institution requirements
         if (

membership_file/migrations/0019_room_cards.py

@@ -0,0 +1,42 @@
+# Generated by Django 3.2.25 on 2024-09-07 11:50
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("membership_file", "0018_membership_member_null"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="room",
+            options={},
+        ),
+        migrations.RenameField(
+            model_name="room",
+            old_name="access",
+            new_name="access_specification",
+        ),
+        migrations.AddField(
+            model_name="room",
+            name="access_type",
+            field=models.CharField(
+                choices=[("KEY", "Key"), ("CARD", "TUe Card/External Card"), ("MISC", "Other")],
+                default="MISC",
+                max_length=4,
+            ),
+            preserve_default=False,
+        ),
+        migrations.AddField(
+            model_name="room",
+            name="room_number",
+            field=models.CharField(blank=True, max_length=15),
+        ),
+        migrations.AlterField(
+            model_name="room",
+            name="access_specification",
+            field=models.CharField(blank=True, help_text="E.g. key number", max_length=15),
+        ),
+    ]

membership_file/migrations/0020_card_lengths.py

@@ -0,0 +1,32 @@
+# Generated by Django 3.2.25 on 2024-09-07 16:47
+
+from django.db import migrations
+
+
+def extend_card_numbers(apps, schema_editor):
+    """Extends the tue/external card numbers to 8 digits, and the external card digits to 4 digits"""
+    # We can't import the Person model directly as it may be a newer
+    # version than this migration expects. We use the historical version.
+    Member = apps.get_model("membership_file", "Member")
+    for member in Member.objects.all():
+        if member.external_card_digits:
+            member.external_card_digits = member.external_card_digits.zfill(4)
+            member.last_updated_by = None
+        if member.external_card_number:
+            member.external_card_number = member.external_card_number.zfill(8)
+            member.last_updated_by = None
+        if member.tue_card_number:
+            member.tue_card_number = member.tue_card_number.zfill(8)
+            member.last_updated_by = None
+        member.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("membership_file", "0019_room_cards"),
+    ]
+
+    operations = [
+        migrations.RunPython(extend_card_numbers),
+    ]

membership_file/migrations/0021_card_lengths_db.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.25 on 2024-09-07 16:48
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('membership_file', '0020_card_lengths'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='member',
+            name='external_card_cluster',
+        ),
+        migrations.AlterField(
+            model_name='member',
+            name='external_card_digits',
+            field=models.CharField(blank=True, max_length=4, validators=[django.core.validators.RegexValidator(message='External card digits must consist of exactly 4 digits. E.g. 0012', regex='^[0-9]{4}$')], verbose_name='digits'),
+        ),
+        migrations.AlterField(
+            model_name='member',
+            name='external_card_number',
+            field=models.CharField(blank=True, help_text="External cards mention 'FMC', whereas Tu/e cards are currently red (since sept. 2021) or orange (before sept. 2021).", max_length=15, null=True, validators=[django.core.validators.RegexValidator(message='TUe card numbers must only consist of exactly 8 digits. For older cards (pre 2017) that only used 7 digits, prepend a 0. E.g. 01457347', regex='^[0-9]{8}$')]),
+        ),
+        migrations.AlterField(
+            model_name='member',
+            name='tue_card_number',
+            field=models.CharField(blank=True, max_length=15, null=True, unique=True, validators=[django.core.validators.RegexValidator(message='TUe card numbers must only consist of exactly 8 digits. For older cards (pre 2017) that only used 7 digits, prepend a 0. E.g. 01457347', regex='^[0-9]{8}$')], verbose_name='TUe card number'),
+        ),
+    ]

membership_file/models.py

@@ -82,7 +82,8 @@ class Meta:
     # NB: These numbers may start with 0, which is why they are not IntegerFields
     ##################################
     tue_card_number_regex = RegexValidator(
-        regex=r"^[0-9]{7,8}$", message="TUe card numbers must only consist of exactly 7 or 8 digits. E.g. 1234567"
+        regex=r"^[0-9]{8}$",
+        message="TUe card numbers must only consist of exactly 8 digits. For older cards (pre 2017) that only used 7 digits, prepend a 0. E.g. 01457347",
     )
     tue_card_number = models.CharField(
         validators=[tue_card_number_regex],
@@ -94,7 +95,7 @@ class Meta:
     )
 
     external_card_digits_regex = RegexValidator(
-        regex=r"^[0-9]{3}$", message="External card digits must consist of exactly 3 digits. E.g. 012"
+        regex=r"^[0-9]{4}$", message="External card digits must consist of exactly 4 digits. E.g. 0012"
     )
 
     # External card uses the same number formatting as Tue cards, but its number does not necessarily need to be unique
@@ -103,14 +104,12 @@ class Meta:
         max_length=15,
         null=True,
         blank=True,
-        help_text="External cards are blue, whereas Tu/e cards are currently red (since sept. 2021) or orange (before sept. 2021).",
+        help_text="External cards mention 'FMC', whereas Tu/e cards are currently red (since sept. 2021) or orange (before sept. 2021).",
     )
-    # 3-digit code at the bottom of a card
+    # 4-digit code at the bottom of a card; only used for external cards
     external_card_digits = models.CharField(
-        validators=[external_card_digits_regex], max_length=3, blank=True, verbose_name="digits"
+        validators=[external_card_digits_regex], max_length=4, blank=True, verbose_name="digits"
     )
-    # The cluster contains additional information of an external card
-    external_card_cluster = models.CharField(max_length=255, blank=True, verbose_name="cluster")
 
     # External cards require a deposit, which has changed over the years
     external_card_deposit = models.DecimalField(
@@ -257,13 +256,9 @@ def display_external_card_number(self):
 
         display_card = self.external_card_number
         if self.external_card_digits:
-            # Not all external card have a 3-digit code (E.g. parking cards)
+            # Not all external card have a 4-digit code (E.g. parking cards)
             display_card += f"-{self.external_card_digits}"
 
-        if self.external_card_cluster:
-            # Not all external cards have a cluster
-            display_card += f" ({self.external_card_cluster})"
-
         return display_card
 
     # Displays a user's address
@@ -286,11 +281,22 @@ def display_address(self):
 
 
 class Room(models.Model):
-    class Meta:
-        ordering = ["access", "name"]
+    """A room that can be gained access through by a card or key."""
 
     name = models.CharField(max_length=63)
-    access = models.CharField(max_length=15, help_text="How access is provided. E.g. 'Key 12' or 'Campus Card'")
+    room_number = models.CharField(max_length=15, blank=True)
+
+    ACCESS_KEY = "KEY"
+    ACCESS_CARD = "CARD"
+    ACCESS_OTHER = "MISC"
+    _ACCESS_TYPES = [
+        (ACCESS_KEY, "Key"),
+        (ACCESS_CARD, "TUe Card/External Card"),
+        (ACCESS_OTHER, "Other"),
+    ]
+    access_type = models.CharField(max_length=4, choices=_ACCESS_TYPES)
+    access_specification = models.CharField(max_length=15, help_text="E.g. key number", blank=True)
+
     notes = models.TextField(blank=True)
 
     # Members who have access to this room
@@ -299,7 +305,12 @@ class Meta:
     members_with_access = models.ManyToManyField(Member, blank=True, related_name="accessible_rooms")
 
     def __str__(self):
-        return f"{self.name} ({self.access})"
+        access = self.get_access_type_display()
+        if self.access_specification:
+            access += " - " + self.access_specification
+        if self.room_number:
+            return f"{self.room_number} - {self.name} ({access})"
+        return f"{self.name} ({access})"
 
 
 ##################################################################################

membership_file/templates/membership_file/membership_edit.html

@@ -43,7 +43,7 @@ <h3>Contact Details</h3>
           <h3>Room Access</h3>
           {% generic_field form.key_id -1 %}
           {% generic_field form.tue_card_number -1 %}
-          {% generic_field form.external_card_number form.external_card_digits form.external_card_cluster 110 70 -1 %}
+          {% generic_field form.external_card_number form.external_card_digits 110 70 %}
           {% if form.external_card_number.value %}
             <!-- Only show deposits if the member has an external card -->
             {% generic_field form.external_card_deposit -1 %}

membership_file/templates/membership_file/membership_view.html

@@ -138,7 +138,7 @@ <h3>Membership Card</h3>
             {% if member.last_updated_by is not None %}
                 This information was last changed on {{member.last_updated_date|date:"M j, Y \a\t H\:i"}} by {{member.display_last_updated_name}}
             {% else %}
-                This information was never updated since its creation on {{member.last_updated_date|date:"M j, Y \a\t H\:i"}}
+                This information was last changed on {{member.last_updated_date|date:"M j, Y \a\t H\:i"}}
             {% endif %}
             <br>
             {% if member.marked_for_deletion %}

membership_file/tests/test_forms.py

@@ -110,7 +110,7 @@ def test_fields(self, _):
         )
 
         # Rooms become available once they exist
-        kitchen = Room.objects.create(name="Kitchen", access="Crowbar")
+        kitchen = Room.objects.create(name="Kitchen", access_type=Room.ACCESS_OTHER, access_specification="Crowbar")
         self.assertHasField("room_access", required=False, initial=None, choices=[(kitchen.id, str(kitchen))])
 
         # Fieldsets should exist
@@ -120,9 +120,26 @@ def test_fields(self, _):
 
     def test_clean(self, _):
         """Tests field cleaning"""
-        # Phone number should be provided if room access is requested
+        # Phone number should be provided if room access is requested for rooms with keys,
+        #   even if a tue card number is provided
         self.assertFormHasError(
-            {"room_access": [Room.objects.create(name="Kitchen", access="Crowbar").pk]},
+            {
+                "room_access": [
+                    Room.objects.create(name="Kitchen", access_type=Room.ACCESS_OTHER).pk,
+                    Room.objects.create(name="Basement", access_type=Room.ACCESS_CARD).pk,
+                ],
+                "tue_card_number": "01234567",
+            },
+            "phone_required",
+            field_name="phone_number",
+        )
+
+        # If all requested rooms requires card access, and a tue card is provided, then no phone number is needed
+        self.assertFormNotHasError(
+            {
+                "room_access": [Room.objects.create(name="Basement", access_type=Room.ACCESS_CARD).pk],
+                "tue_card_number": "01234567",
+            },
             "phone_required",
             field_name="phone_number",
         )
@@ -173,7 +190,7 @@ def test_save(self, _):
         self.assertFormValid(data)
 
         # Room Access
-        room = Room.objects.create(name="Kitchen", access="Crowbar")
+        room = Room.objects.create(name="Kitchen", access_type=Room.ACCESS_OTHER, access_specification="Crowbar")
         data = {**data, "room_access": [room.pk]}
         form = self.build_form(data)
         self.assertFormValid(data)