tomgranuja / CayumanDjango

School workshops inscription web form for students enrollment.
0 stars 0 forks source link

Turn `Period.enrollment_start` to `DateTimeField` #49

Closed ralamosm closed 4 months ago

ralamosm commented 5 months ago

This task is about turn field Period.enrollment_start from models.py, to a DateTimeField so users can start enrolling at a better time than 00:00.

The change is straightforward and it should be something like this:

These lines of test_periods.py show how date(time)s are mocked in the tests

https://github.com/tomgranuja/CayumanDjango/blob/main/tests/test_periods.py#L170-L173

basically this thing forces running code to believe current date(time) is the one passed on those lines, so the execution environment is the one expected by your test.

Let me know of any problems

tomgranuja commented 5 months ago

@ralamosm me dio problemas generar la migración después de cambiar a DateTimeField:

[salva@hawk CayumanDjango]$ poetry run python manage.py makemigrations
/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/fields/__init__.py:1659: RuntimeWarning: DateTimeField Period.enrollment_start received a naive datetime (2024-06-02 16:42:32.574812) while time zone support is active.
  warnings.warn(
/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py:98: RuntimeWarning: Accessing the database during app initialization is discouraged. To fix this warning, avoid executing queries in AppConfig.ready() or when your app modules are imported.
  warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning)
Traceback (most recent call last):
  File "/home/salva/Cmd/CayumanDjango/manage.py", line 22, in <module>
    main()
  File "/home/salva/Cmd/CayumanDjango/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 416, in execute
    django.setup()
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/apps/registry.py", line 124, in populate
    app_config.ready()
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/contrib/admin/apps.py", line 27, in ready
    self.module.autodiscover()
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/contrib/admin/__init__.py", line 52, in autodiscover
    autodiscover_modules("admin", register_to=site)
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/utils/module_loading.py", line 58, in autodiscover_modules
    import_module("%s.%s" % (app_config.name, module_to_search))
  File "/usr/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/salva/Cmd/CayumanDjango/cayuman/admin.py", line 301, in <module>
    class StudentCycleAdmin(admin.ModelAdmin):
  File "/home/salva/Cmd/CayumanDjango/cayuman/admin.py", line 341, in StudentCycleAdmin
    @admin.display(description=_("Workshops %s") % (Period.objects.current()))
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/salva/Cmd/CayumanDjango/cayuman/models.py", line 212, in current
    val = self.get_queryset().get(date_condition & end_condition)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/query.py", line 645, in get
    num = len(clone)
          ^^^^^^^^^^
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/query.py", line 382, in __len__
    self._fetch_all()
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/query.py", line 1928, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/query.py", line 123, in __iter__
    for row in compiler.results_iter(results):
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1500, in apply_converters
    value = converter(value, expression, connection)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/backends/sqlite3/operations.py", line 314, in convert_datetimefield_value
    value = parse_datetime(value)
            ^^^^^^^^^^^^^^^^^^^^^
  File "/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/utils/dateparse.py", line 114, in parse_datetime
    return datetime.datetime.fromisoformat(value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: fromisoformat: argument must be str

Estos son los cambios de cayuman.models:

diff --git a/cayuman/models.py b/cayuman/models.py
index b5e2cda..dc20155 100644
--- a/cayuman/models.py
+++ b/cayuman/models.py
@@ -204,14 +204,15 @@ class PeriodManager(models.Manager):
         """
         from django.db.models import Q

-        now = datetime.now().date()
+        now = datetime.now()
+        now_date = now.date()
         try:
-            date_condition = Q(enrollment_start__lte=now) | Q(preview_date__lte=now)
-            end_condition = Q(date_end__gte=now)
+            date_condition = Q(enrollment_start__lte=now) | Q(preview_date__lte=now_date)
+            end_condition = Q(date_end__gte=now_date)
             val = self.get_queryset().get(date_condition & end_condition)
         except self.model.MultipleObjectsReturned:
             try:
-                val = self.get_queryset().get(date_start__lte=now, date_end__gte=now)
+                val = self.get_queryset().get(date_start__lte=now_date, date_end__gte=now_date)
             except self.model.DoesNotExist:
                 val = None
         except self.model.DoesNotExist:
@@ -230,7 +231,7 @@ class Period(models.Model):
     name = models.CharField(max_length=50, verbose_name=_("Name"))
     description = models.TextField(blank=True, verbose_name=_("Description"))
     preview_date = models.DateField(blank=True, null=True, verbose_name=_("Preview date"))
-    enrollment_start = models.DateField(blank=True, verbose_name=_("Enrollment start date"))
+    enrollment_start = models.DateTimeField(blank=True, verbose_name=_("Enrollment start date and time"))
     enrollment_end = models.DateField(blank=True, null=True, verbose_name=_("Enrollment end date"))
     date_start = models.DateField(verbose_name=_("Start date"))
     date_end = models.DateField(verbose_name=_("End date"))
@@ -277,22 +278,22 @@ class Period(models.Model):

     def clean(self):
         if not self.preview_date:
-            self.preview_date = self.enrollment_start
+            self.preview_date = self.enrollment_start.date()

-        if self.preview_date > self.enrollment_start:
+        if self.preview_date > self.enrollment_start.date():
             raise ValidationError({"preview_date": _("Preview date must be before enrollment start date")})

         if not self.enrollment_end and self.enrollment_start:
             # Fill enrollment_end automatically to enrollment_start + 5 days
-            self.enrollment_end = self.enrollment_start + timezone.timedelta(days=5)
+            self.enrollment_end = self.enrollment_start.date() + timezone.timedelta(days=5)

         if self.date_start >= self.date_end:
             raise ValidationError({"date_start": _("Start date must be before end date")})

-        if self.enrollment_start >= self.enrollment_end:
+        if self.enrollment_start.date() >= self.enrollment_end:
             raise ValidationError({"enrollment_start": _("Enrollment start date must be before enrollment end date")})

-        if self.enrollment_start and self.enrollment_start > self.date_start:
+        if self.enrollment_start and self.enrollment_start.date() > self.date_start:
             raise ValidationError({"enrollment_start": _("Enrollment start date must be before start date")})

         # Can't collide with another period in terms of date_start and date_end
@@ -316,10 +317,11 @@ class Period(models.Model):
         return self == Period.objects.current()

     def can_be_previewed(self):
-        now = datetime.now().date()
+        now = datetime.now()
+        now_date = now.date()
         if self.preview_date:
-            return self.preview_date <= now and self.date_end >= now
-        return self.enrollment_start <= now and self.date_end >= now
+            return self.preview_date <= now_date and self.date_end >= now_date
+        return self.enrollment_start <= now and self.date_end >= now_date

     def save(self, *args, **kwargs):
         self.clean()
@@ -519,15 +521,16 @@ class StudentCycle(models.Model):
         if not period:
             return False

-        now = datetime.now().date()
+        now = datetime.now()
+        now_date = now.date()

         # It's never possible to enroll before `enrollment_start` and after `date_end`
-        if now > period.date_end or now < period.enrollment_start:
+        if now_date > period.date_end or now < period.enrollment_start:
             return False

         # students with full schedule can only re-enroll between `enrollment_start` and `enrollment_end`
         if self.is_schedule_full(period):
-            if period.enrollment_start <= now <= period.enrollment_end:
+            if period.enrollment_start <= now and now_date <= period.enrollment_end:
                 return True
         else:
             # students without full schedule can enroll anytime until `date_end`

Maybe needs a manual update to db?

ralamosm commented 5 months ago

@tomgranuja el problema es que la fecha guardada no es timezone aware. Te propongo que hagas commit de estos cambios de tu rama y yo los reviso y busco el fix a partir de ahi

tomgranuja commented 4 months ago

@ralamosm Investigué un poco más el problema y tampoco me dejó correr makemigrations ni la shell usando timezone.now() en PeriodManager. Pero si pude correr makemigrations y hacer migración cambiando momentaneamente en el admin un par de @admin.display:

[salva@hawk CayumanDjango]$ git diff
diff --git a/cayuman/admin.py b/cayuman/admin.py
index 512be4f..e03aa11 100644
--- a/cayuman/admin.py
+++ b/cayuman/admin.py
@@ -338,7 +338,8 @@ class StudentCycleAdmin(admin.ModelAdmin):
         queryset = super().get_queryset(request)
         return queryset.filter(student__is_active=True)

-    @admin.display(description=_("Workshops %s") % (Period.objects.current()))
+    # @admin.display(description=_("Workshops %s") % (Period.objects.current()))
+    @admin.display(description=_("Workshops %s") % 'PlaceHolder')
     def this_period_workshops_html(self, obj):
         """Display function to use in Django admin list for this model"""
         wps = obj.workshop_periods_by_period(Period.objects.current())
@@ -352,7 +353,8 @@ class StudentCycleAdmin(admin.ModelAdmin):
         else:
             return _("No workshops yet")

-    @admin.display(description=_("Workshops %s") % (Period.objects.current()))
+    # @admin.display(description=_("Workshops %s") % (Period.objects.current()))
+    @admin.display(description=_("Workshops %s") % 'PlaceHolder')
     def this_period_workshops_list(self, obj):
         """Display function to use when exporting these entries to CSV"""
         wps = obj.workshop_periods_by_period(Period.objects.current())

Lo curioso es que después de generar y hacer la migración, reestablecí las lineas cambiadas del admin para dejar los @admin.display tal como estaban antes y aparentemente funciona todo bien, aunque la shell da un warning:

[salva@hawk CayumanDjango]$ git restore cayuman/admin.py
[salva@hawk CayumanDjango]$ poetry run python manage.py shell
/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/fields/__init__.py:1659: RuntimeWarning: DateTimeField Period.enrollment_start received a naive datetime (2024-06-15 21:53:11.054989) while time zone support is active.
  warnings.warn(
/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py:98: RuntimeWarning: Accessing the database during app initialization is discouraged. To fix this warning, avoid executing queries in AppConfig.ready() or when your app modules are imported.
  warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning)
/home/salva/.cache/pypoetry/virtualenvs/cayuman-WGqrk5ch-py3.11/lib/python3.11/site-packages/django/db/models/fields/__init__.py:1659: RuntimeWarning: DateTimeField Period.enrollment_start received a naive datetime (2024-06-15 21:53:11.060058) while time zone support is active.
  warnings.warn(
Python 3.11.7 (main, Jan 29 2024, 16:03:57) [GCC 13.2.1 20230801] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>>

El warning desaparece al usar timezone.now() en vez de datetime.now() en models.py

Hice el commit 9ae52d7 con estos cambios para buscar el fix.

ralamosm commented 4 months ago

gracias por la explicacion @tomgranuja

Si esto ya esta listo lo tomo yo para pasarlo a produccion junto a mis cambios de #52. El merge voy a tener que trabajarlo a mano seguramente.

ralamosm commented 4 months ago

en prod