From 373064511f5dbf7679248c9bc2a4a0bee69d6193 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Fri, 15 Dec 2023 10:32:33 +0000 Subject: [PATCH] feedback --- codeforlife/models/__init__.py | 2 +- codeforlife/user/migrations/0001_initial.py | 6 +++--- codeforlife/user/models/otp_bypass_token.py | 12 +++--------- codeforlife/user/models/student.py | 8 ++++---- codeforlife/user/models/teacher.py | 5 ++--- codeforlife/user/models/user.py | 3 +-- 6 files changed, 14 insertions(+), 22 deletions(-) diff --git a/codeforlife/models/__init__.py b/codeforlife/models/__init__.py index 190771ac..a20c73ae 100644 --- a/codeforlife/models/__init__.py +++ b/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() diff --git a/codeforlife/user/migrations/0001_initial.py b/codeforlife/user/migrations/0001_initial.py index bce97322..9dbbc001 100644 --- a/codeforlife/user/migrations/0001_initial.py +++ b/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 10:31 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', diff --git a/codeforlife/user/models/otp_bypass_token.py b/codeforlife/user/models/otp_bypass_token.py index c7ef9aeb..1be993a1 100644 --- a/codeforlife/user/models/otp_bypass_token.py +++ b/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): diff --git a/codeforlife/user/models/student.py b/codeforlife/user/models/student.py index c6818ebe..dea45142 100644 --- a/codeforlife/user/models/student.py +++ b/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. diff --git a/codeforlife/user/models/teacher.py b/codeforlife/user/models/teacher.py index 65b998d4..2710af84 100644 --- a/codeforlife/user/models/teacher.py +++ b/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()) diff --git a/codeforlife/user/models/user.py b/codeforlife/user/models/user.py index bc6dfa29..f519fccf 100644 --- a/codeforlife/user/models/user.py +++ b/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(