Merge branch 'new_data_models' into new_data_model_permissions

codeforlife/models/__init__.py

@@ -48,7 +48,7 @@ def update(self, **kwargs):
                 last_saved_at: When these models were last modified.
 
             Returns:
-                The number of models updated.
+                The number of models matched.
             """
 
             kwargs["last_saved_at"] = timezone.now()

codeforlife/user/fixtures/schools.json

@@ -5,7 +5,7 @@
     "fields": {
       "last_saved_at": "2023-01-01 00:00:00.0+00:00",
       "name": "Example School",
-      "country": "UK",
+      "country": "GB",
       "uk_county": "Surrey"
     }
   }

codeforlife/user/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 3.2.20 on 2023-12-11 17:42
+# Generated by Django 3.2.20 on 2023-12-15 17:11
 
 from django.conf import settings
 import django.core.validators
@@ -25,7 +25,7 @@ class Migration(migrations.Migration):
                 ('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')),
                 ('last_saved_at', models.DateTimeField(auto_now=True, help_text='Record the last time the model was saved. This is used by our data warehouse to know what data was modified since the last scheduled data transfer from the database to the data warehouse.', verbose_name='last saved at')),
                 ('delete_after', models.DateTimeField(blank=True, help_text="When this data is scheduled for deletion. Set to null if not scheduled for deletion. This is used by our data warehouse to transfer data that's been scheduled for deletion before it's actually deleted. Data will actually be deleted in a CRON job after this point in time.", null=True, verbose_name='delete after')),
-                ('first_name', models.CharField(blank=True, max_length=150, verbose_name='first name')),
+                ('first_name', models.CharField(max_length=150, verbose_name='first name')),
                 ('last_name', models.CharField(blank=True, max_length=150, null=True, verbose_name='last name')),
                 ('email', models.EmailField(blank=True, max_length=254, null=True, unique=True, verbose_name='email address')),
                 ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')),
@@ -114,7 +114,7 @@ class Migration(migrations.Migration):
                 ('last_saved_at', models.DateTimeField(auto_now=True, help_text='Record the last time the model was saved. This is used by our data warehouse to know what data was modified since the last scheduled data transfer from the database to the data warehouse.', verbose_name='last saved at')),
                 ('delete_after', models.DateTimeField(blank=True, help_text="When this data is scheduled for deletion. Set to null if not scheduled for deletion. This is used by our data warehouse to transfer data that's been scheduled for deletion before it's actually deleted. Data will actually be deleted in a CRON job after this point in time.", null=True, verbose_name='delete after')),
                 ('is_admin', models.BooleanField(default=False, help_text='Designates if the teacher has admin privileges.', verbose_name='is administrator')),
-                ('school', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='teachers', to='user.school')),
+                ('school', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='teachers', to='user.school')),
             ],
             options={
                 'verbose_name': 'teacher',
@@ -150,7 +150,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AddConstraint(
             model_name='school',
-            constraint=models.CheckConstraint(check=models.Q(('uk_county__isnull', True), ('country', 'UK'), _connector='OR'), name='school__no_uk_county_if_country_not_uk'),
+            constraint=models.CheckConstraint(check=models.Q(('uk_county__isnull', True), ('country', 'GB'), _connector='OR'), name='school__no_uk_county_if_country_not_uk'),
         ),
         migrations.AddField(
             model_name='otpbypasstoken',
@@ -210,7 +210,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AddConstraint(
             model_name='user',
-            constraint=models.CheckConstraint(check=models.Q(models.Q(('student__isnull', False), ('teacher__isnull', True)), models.Q(('student__isnull', True), ('teacher__isnull', False)), models.Q(('student__isnull', True), ('teacher__isnull', True)), _connector='OR'), name='user__profile'),
+            constraint=models.CheckConstraint(check=models.Q(('student__isnull', False), ('teacher__isnull', False), _negated=True), name='user__profile'),
         ),
         migrations.AddConstraint(
             model_name='user',
@@ -220,4 +220,12 @@ class Migration(migrations.Migration):
             model_name='user',
             constraint=models.CheckConstraint(check=models.Q(models.Q(('last_name__isnull', False), ('teacher__isnull', False)), models.Q(('last_name__isnull', True), ('student__isnull', False)), models.Q(('last_name__isnull', False), ('student__isnull', True), ('teacher__isnull', True)), _connector='OR'), name='user__last_name'),
         ),
+        migrations.AddConstraint(
+            model_name='user',
+            constraint=models.CheckConstraint(check=models.Q(('is_staff', True), ('student__isnull', False), _negated=True), name='user__is_staff'),
+        ),
+        migrations.AddConstraint(
+            model_name='user',
+            constraint=models.CheckConstraint(check=models.Q(('is_superuser', True), ('student__isnull', False), _negated=True), name='user__is_superuser'),
+        ),
     ]

codeforlife/user/models/otp_bypass_token.py

@@ -65,10 +65,7 @@ def key(otp_bypass_token: OtpBypassToken):
             for user_id, group in groupby(otp_bypass_tokens, key=key):
                 if (
                     len(list(group))
-                    + OtpBypassToken.objects.filter(
-                        user_id=user_id,
-                        delete_after__isnull=True,
-                    ).count()
+                    + OtpBypassToken.objects.filter(user_id=user_id).count()
                     > OtpBypassToken.max_count
                 ):
                     raise OtpBypassToken.max_count_validation_error
@@ -100,10 +97,7 @@ class Meta(TypedModelMeta):
     def save(self, *args, **kwargs):
         if self.id is None:
             if (
-                OtpBypassToken.objects.filter(
-                    user=self.user,
-                    delete_after__isnull=True,
-                ).count()
+                OtpBypassToken.objects.filter(user=self.user).count()
                 >= OtpBypassToken.max_count
             ):
                 raise OtpBypassToken.max_count_validation_error
@@ -117,7 +111,7 @@ def check_token(self, token: str):
             token: Token to check.
 
         Returns:
-            A boolean designating if the token is matches.
+            A boolean designating if the token matches.
         """
 
         if check_password(token, self.token):

codeforlife/user/models/school.py

@@ -66,7 +66,7 @@ class Meta(TypedModelMeta):
         verbose_name_plural = _("schools")
         constraints = [
             models.CheckConstraint(
-                check=Q(uk_county__isnull=True) | Q(country="UK"),
+                check=Q(uk_county__isnull=True) | Q(country=Country.GB),
                 name="school__no_uk_county_if_country_not_uk",
             ),
         ]

codeforlife/user/models/student.py

@@ -69,10 +69,10 @@ def create_user(self, student: t.Dict[str, t.Any], **fields):
             """Create a user with a student profile.
 
             Args:
-                user: The user fields.
+                student: The student fields.
 
             Returns:
-                A student profile.
+                A user with a student profile.
             """
 
             return _user.User.objects.create_user(
@@ -94,7 +94,7 @@ def bulk_create_users(
                     profile belongs to.
 
             Returns:
-                A list of users that have been assigned their student profile.
+                A list of users with a student profile.
             """
 
             students = [student for (student, _) in student_users]
@@ -143,7 +143,7 @@ class Meta(TypedModelMeta):
 
     @property
     def teacher(self):
-        """The student's teacher (if they have one).
+        """The student's teacher.
 
         Returns:
             The student's class-teacher.

codeforlife/user/models/teacher.py

@@ -51,6 +51,7 @@ def create_user(self, teacher: t.Dict[str, t.Any], **fields):
         "user.School",
         related_name="teachers",
         null=True,
+        blank=True,
         on_delete=models.SET_NULL,
     )
 
@@ -73,6 +74,4 @@ def students(self) -> QuerySet["_student.Student"]:
             A queryset
         """
 
-        return _student.Student.objects.filter(
-            klass_id__in=list(self.classes.values_list("id", flat=True)),
-        )
+        return _student.Student.objects.filter(klass__in=self.classes.all())

codeforlife/user/models/user.py

@@ -32,7 +32,7 @@
 class User(AbstractBaseUser, WarehouseModel, PermissionsMixin):
     """A user within the CFL system."""
 
-    USERNAME_FIELD = "email"
+    USERNAME_FIELD = "id"
 
     class Manager(BaseUserManager["User"], WarehouseModel.Manager["User"]):
         """
@@ -116,7 +116,6 @@ def create_superuser(self, password: str, first_name: str, **fields):
     first_name = models.CharField(
         _("first name"),
         max_length=150,
-        blank=True,
     )
 
     last_name = models.CharField(
@@ -200,19 +199,9 @@ class Meta(TypedModelMeta):
         constraints = [
             # pylint: disable=unsupported-binary-operation
             models.CheckConstraint(
-                check=(
-                    Q(
-                        teacher__isnull=True,
-                        student__isnull=False,
-                    )
-                    | Q(
-                        teacher__isnull=False,
-                        student__isnull=True,
-                    )
-                    | Q(
-                        teacher__isnull=True,
-                        student__isnull=True,
-                    )
+                check=~Q(
+                    teacher__isnull=False,
+                    student__isnull=False,
                 ),
                 name="user__profile",
             ),
@@ -252,6 +241,20 @@ class Meta(TypedModelMeta):
                 ),
                 name="user__last_name",
             ),
+            models.CheckConstraint(
+                check=~Q(
+                    student__isnull=False,
+                    is_staff=True,
+                ),
+                name="user__is_staff",
+            ),
+            models.CheckConstraint(
+                check=~Q(
+                    student__isnull=False,
+                    is_superuser=True,
+                ),
+                name="user__is_superuser",
+            ),
             # pylint: enable=unsupported-binary-operation
         ]
 

codeforlife/user/tests/models/test_user.py

@@ -165,10 +165,44 @@ def test_constraints__last_name__indy(self):
         with self.assert_raises_integrity_error():
             User.objects.create_user(
                 password="password",
-                first_name="teacher",
+                first_name="Indiana",
                 email="[email protected]",
             )
 
+    def test_constraints__is_staff(self):
+        """
+        Students cannot be a staff user.
+        """
+
+        with self.assert_raises_integrity_error():
+            User.objects.create_user(
+                first_name="Indiana",
+                password="password",
+                student=Student.objects.create(
+                    auto_gen_password="password",
+                    klass=self.klass__AB123,
+                    school=self.school__1,
+                ),
+                is_staff=True,
+            )
+
+    def test_constraints__is_superuser(self):
+        """
+        Students cannot be a super user.
+        """
+
+        with self.assert_raises_integrity_error():
+            User.objects.create_user(
+                first_name="Indiana",
+                password="password",
+                student=Student.objects.create(
+                    auto_gen_password="password",
+                    klass=self.klass__AB123,
+                    school=self.school__1,
+                ),
+                is_superuser=True,
+            )
+
     def test_objects__create(self):
         """
         Cannot call objects.create.
@@ -263,31 +297,6 @@ def test_objects__create_superuser__teacher(self):
         assert user.is_staff
         assert user.is_superuser
 
-    def test_objects__create_superuser__student(self):
-        """
-        Create a student super user.
-        """
-
-        user_fields = {
-            "first_name": "first_name",
-            "password": "password",
-            "student": Student.objects.create(
-                auto_gen_password="password",
-                klass=self.klass__AB123,
-                school=self.school__1,
-            ),
-        }
-
-        user = User.objects.create_superuser(
-            **user_fields  # type: ignore[arg-type]
-        )
-        assert user.first_name == user_fields["first_name"]
-        assert user.password != user_fields["password"]
-        assert user.check_password(user_fields["password"])
-        assert user.student == user_fields["student"]
-        assert user.is_staff
-        assert user.is_superuser
-
     def test_objects__create_superuser__indy(self):
         """
         Create an independent super user.