Correctly represent the last week in rrules Part 2

integreat_cms/cms/constants/weeks.py

@@ -22,8 +22,10 @@
 THIRD: Final = 3
 #: Fourth week of the month
 FOURTH: Final = 4
-#: Last week of the month (either 4th or 5th)
-LAST: Final = 5
+#: Last week of the month
+LAST: Final = -1
+#: Second Last week of the month
+SECOND_LAST: Final = -2
 
 #: Choices to use these constants in a database field
 CHOICES: Final[list[tuple[int, Promise]]] = [
@@ -32,13 +34,5 @@
     (THIRD, _("Third week")),
     (FOURTH, _("Fourth week")),
     (LAST, _("Last week")),
+    (SECOND_LAST, _("Second last week")),
 ]
-
-#: A mapping from our week constants to the expected rrule values
-WEEK_TO_RRULE_WEEK = {
-    FIRST: 1,
-    SECOND: 2,
-    THIRD: 3,
-    FOURTH: 4,
-    LAST: -1,
-}

integreat_cms/cms/migrations/0099_alter_recurrencerule_week_for_monthly.py

@@ -0,0 +1,82 @@
+# Generated by Django 4.2.13 on 2024-08-27 09:08
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from django.db import migrations, models
+
+from ..constants import frequency
+
+if TYPE_CHECKING:
+    from django.apps.registry import Apps
+    from django.db.backends.base.schema import BaseDatabaseSchemaEditor as SchemaEditor
+
+
+# pylint: disable=unused-argument
+def forwards_func(apps: Apps, schema_editor: SchemaEditor) -> None:
+    """
+    Adopting the old data when applying this migration
+    :param apps: The configuration of installed applications
+    :type apps: ~django.apps.registry.Apps
+    :param schema_editor: The database abstraction layer that creates actual SQL code
+    :type schema_editor: ~django.db.backends.base.schema.BaseDatabaseSchemaEditor
+    """
+    RecurrenceRule = apps.get_model("cms", "RecurrenceRule")
+    for rr in RecurrenceRule.objects.filter(
+        frequency=frequency.MONTHLY, week_for_monthly=5
+    ):
+        rr.week_for_monthly = -1
+        rr.save()
+
+
+# pylint: disable=unused-argument
+def reverse_func(apps: Apps, schema_editor: SchemaEditor) -> None:
+    """
+    Reverting (most of the) newer data when reverting this migration
+    :param apps: The configuration of installed applications
+    :type apps: ~django.apps.registry.Apps
+    :param schema_editor: The database abstraction layer that creates actual SQL code
+    :type schema_editor: ~django.db.backends.base.schema.BaseDatabaseSchemaEditor
+    """
+    RecurrenceRule = apps.get_model("cms", "RecurrenceRule")
+    for rr in RecurrenceRule.objects.filter(
+        frequency=frequency.MONTHLY, week_for_monthly__lt=0
+    ):
+        if rr.week_for_monthly == -1:
+            rr.week_for_monthly = 5
+        else:
+            rr.week_for_monthly = 3
+        rr.save()
+
+
+class Migration(migrations.Migration):
+    """
+    Remove 5 as designation for last week, use -1 (and -2) instead.
+    """
+
+    dependencies = [
+        ("cms", "0098_add_contact"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="recurrencerule",
+            name="week_for_monthly",
+            field=models.IntegerField(
+                blank=True,
+                choices=[
+                    (1, "First week"),
+                    (2, "Second week"),
+                    (3, "Third week"),
+                    (4, "Fourth week"),
+                    (-1, "Last week"),
+                    (-2, "Second last week"),
+                ],
+                help_text="If the frequency is monthly, this field determines on which week of the month the event takes place",
+                null=True,
+                verbose_name="week",
+            ),
+        ),
+        migrations.RunPython(forwards_func, reverse_func),
+    ]

integreat_cms/cms/models/events/recurrence_rule.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from datetime import date, datetime, time, timedelta
+from functools import reduce
 from typing import TYPE_CHECKING
 
 from dateutil import rrule
@@ -86,7 +87,7 @@ def iter_after(self, start_date: date) -> Iterator[date]:
         """
         next_recurrence = start_date
 
-        def get_nth_weekday(month_date: date, weekday: int, n: int) -> date:
+        def get_nth_weekday(month_date: date, weekday: int, n: int) -> date | None:
             """
             Get the nth occurrence of a given weekday in a specific month
 
@@ -97,10 +98,18 @@ def get_nth_weekday(month_date: date, weekday: int, n: int) -> date:
             """
             month_date = month_date.replace(day=1)
             month_date += timedelta((weekday - month_date.weekday()) % 7)
-            n_th_occurrence = month_date + timedelta(weeks=n - 1)
-            # If the occurrence is not in the desired month (because the last week is 4 and not 5), retry with 4
+            if n < 0:
+                # Move past last occurence of date within month to properly count from the end
+                n_th_occurrence = month_date + timedelta(weeks=3)
+                while n_th_occurrence.month == month_date.month:
+                    n_th_occurrence += timedelta(weeks=1)
+                # We add the timedelta since n is already negative
+                n_th_occurrence += timedelta(weeks=n)
+            else:
+                n_th_occurrence = month_date + timedelta(weeks=n - 1)
+            # If the occurrence is not in the desired month we have no valid result
             if n_th_occurrence.month != month_date.month:
-                n_th_occurrence = month_date + timedelta(weeks=n - 2)
+                return None
             return n_th_occurrence
 
         def next_month(month_date: date) -> date:
@@ -139,15 +148,21 @@ def advance() -> Iterator[date]:
                 # advance to the next monday
                 next_recurrence += timedelta(days=7 - next_recurrence.weekday())
             elif self.frequency == frequency.MONTHLY:
-                next_recurrence = get_nth_weekday(
-                    next_recurrence, self.weekday_for_monthly, self.week_for_monthly
-                )
-                if next_recurrence < start_date:
-                    next_recurrence = get_nth_weekday(
-                        next_month(next_recurrence),
+                candidate = None
+                month_dif = 0
+                while candidate is None or candidate < start_date:
+                    candidate = get_nth_weekday(
+                        # Apply next_month() to original date month_dif times
+                        reduce(
+                            lambda x, _: next_month(x),
+                            range(month_dif),
+                            next_recurrence,
+                        ),
                         self.weekday_for_monthly,
                         self.week_for_monthly,
                     )
+                    month_dif += 1
+                next_recurrence = candidate
                 yield next_recurrence
                 next_recurrence = next_month(next_recurrence)
             elif self.frequency == frequency.YEARLY:
@@ -207,7 +222,7 @@ def to_ical_rrule(self) -> rrule.rrule:
         elif self.frequency == frequency.MONTHLY:
             kwargs["byweekday"] = rrule.weekday(
                 self.weekday_for_monthly,
-                weeks.WEEK_TO_RRULE_WEEK[self.week_for_monthly],
+                self.week_for_monthly,
             )
         if self.recurrence_end_date:
             kwargs["until"] = make_aware(

integreat_cms/locale/de/LC_MESSAGES/django.po

@@ -1798,6 +1798,10 @@ msgstr "Vierte Woche"
 msgid "Last week"
 msgstr "Letzte Woche"
 
+#: cms/constants/weeks.py
+msgid "Second last week"
+msgstr "Vorletzte Woche"
+
 #: cms/forms/custom_content_model_form.py
 msgid ""
 "Could not update because this content because it is already being edited by "