fixing cron conversion (#147)

fixing cron conversion

Author: marcinapostoluk

CHANGELOG.md

@@ -2,7 +2,7 @@
 ------------------
 * Feature: Allow configuring error email addresses via UI. 
 * Bugfix: . and .. should now be allowed to be used when specifying the templates directory.
-
+* Bugfix: corrected cron schedule incorrectly shifting back one day upon save.
 
 
 0.5.1 (2023-02-22)

notebooker/web/routes/scheduling.py

@@ -1,5 +1,5 @@
 import json
-from typing import Optional, List
+from typing import Optional, List, Callable
 import logging
 
 from apscheduler.jobstores.base import ConflictingIdError
@@ -125,51 +125,53 @@ def create_schedule(report_name):
         return jsonify({"status": "Failed", "content": str(e)})
 
 
-def convert_day_of_week(day_of_week: str) -> str:
+def _convert_day_of_week(day_of_week: str, convert_func: Callable) -> str:
     """
-    UNIX standard maps days-of-week to ints as:
-    SUN: 0
-    MON: 1
-    TUE: 2....
-
-    APScheduler uses:
-    MON: 0
-    TUE: 1
-    WED: 2.....
-
-    Given we are providing a UNIX-style crontab, convert ints accordingly.  Don't shift char-based descriptors,
-    ie., 'MON-FRI'.
+    Given we are providing a crontab converts the int-based day specification according to the function passed.
+    Does not shift char-based descriptors i.e. 'MON-FRI'.
     Parameters
     ----------
     day_of_week - str - "FRI", "MON-FRI", "1"(UNIX Monday)
-
-    Returns
-    -------
-    day_of_week formatted to APScheduler standard
+    convert_func - function to use for conversion
     """
 
     def shift(mychar):
         if mychar.isnumeric():
             myint = int(mychar)
-            return str((myint + 6) % 7)
+            return str(convert_func(myint))
         else:
             return mychar
 
     return "".join([shift(char) for char in day_of_week])
 
 
+def crontab_to_apscheduler_day_of_week(day_of_week: str) -> str:
+    """
+    Converts UNIX standard days-of-week (SUN=0, MON=1, ...) to APScheduler ones (MON=0, TUE=1, ...)
+    """
+    return _convert_day_of_week(day_of_week, lambda dow: (dow + 6) % 7)
+
+
+def apscheduler_to_crontab_day_of_week(day_of_week: str) -> str:
+    """
+    Converts APScheduler days-of-week (MON=0, TUE=1, ...) to UNIX standard ones (SUN=0, MON=1, ...)
+    """
+    return _convert_day_of_week(day_of_week, lambda dow: (dow - 6) % 7)
+
+
 def validate_crontab(crontab: str, issues: List[str]) -> cron.CronTrigger:
     parts = crontab.split()
     if len(parts) != 5:
         issues.append("The crontab key must be passed with a string using 5 crontab parts")
     else:
-        parts[4] = convert_day_of_week(parts[4])
+        parts[4] = crontab_to_apscheduler_day_of_week(parts[4])
         return cron.CronTrigger(minute=parts[0], hour=parts[1], day=parts[2], month=parts[3], day_of_week=parts[4])
 
 
 def trigger_to_crontab(trigger: cron.CronTrigger) -> str:
     fields = {f.name: str(f) for f in trigger.fields}
-    return f"{fields['minute']} {fields['hour']} {fields['day']} {fields['month']} {fields['day_of_week']}"
+    day_of_week = apscheduler_to_crontab_day_of_week(fields["day_of_week"])
+    return f"{fields['minute']} {fields['hour']} {fields['day']} {fields['month']} {day_of_week}"
 
 
 def _job_to_json(job):

tests/unit/test_convert_day_of_week.py

@@ -1,16 +1,42 @@
-from notebooker.web.routes.scheduling import convert_day_of_week
+from notebooker.web.routes.scheduling import crontab_to_apscheduler_day_of_week
+from notebooker.web.routes.scheduling import apscheduler_to_crontab_day_of_week
 
 
-def test_weekdays():
-    result = convert_day_of_week("1-5")
+def test_to_appscheduler_days():
+    result = crontab_to_apscheduler_day_of_week("0,1,2,3,4,5,6")
+    assert result == "6,0,1,2,3,4,5"
+
+
+def test_to_crontab_days():
+    result = apscheduler_to_crontab_day_of_week("6,0,1,2,3,4,5")
+    assert result == "0,1,2,3,4,5,6"
+
+
+def test_to_appscheduler_weekdays():
+    result = crontab_to_apscheduler_day_of_week("1-5")
     assert result == "0-4"
 
 
-def test_sunday():
-    result = convert_day_of_week("0")
+def test_to_crontab_weekdays():
+    result = apscheduler_to_crontab_day_of_week("0-4")
+    assert result == "1-5"
+
+
+def test_to_appscheduler_sunday():
+    result = crontab_to_apscheduler_day_of_week("0")
     assert result == "6"
 
 
-def test_string_days():
-    result = convert_day_of_week("MON-FRI")
-    assert result == "MON-FRI"
+def test_to_crontab_sunday():
+    result = apscheduler_to_crontab_day_of_week("6")
+    assert result == "0"
+
+
+def test_to_appscheduler_string_days():
+    result = crontab_to_apscheduler_day_of_week("MON-FRI")
+    assert result == "MON-FRI"
+
+
+def test_to_crontab_string_days():
+    result = apscheduler_to_crontab_day_of_week("MON-FRI")
+    assert result == "MON-FRI"