Merge pull request #54 from man-group/feature/scheduling

fix: 2 minor scheduling fixes

notebooker/utils/template_testing.py

@@ -39,7 +39,7 @@ def setup_test(template_dir):
 def sanity_check(template_dir):
     logger.info(f"Starting sanity check in {template_dir}")
     with setup_test(template_dir):
-        for template_name in notebooker.web.utils._all_templates():
+        for template_name in notebooker.web.utils.all_templates_flattened():
             logger.info(f"========================[ Sanity checking {template_name} ]========================")
             # Test conversion to ipynb - this will throw if stuff goes wrong
             generate_ipynb_from_py(
@@ -76,7 +76,7 @@ def regression_test(template_dir):
     logger.info("Starting regression test")
     with setup_test(template_dir):
         attempted_templates, failed_templates = [], set()
-        for template_name in notebooker.web.utils._all_templates():
+        for template_name in notebooker.web.utils.all_templates_flattened():
             logger.info(f"============================[ Testing {template_name} ]============================")
             try:
                 attempted_templates.append(template_name)

notebooker/web/routes/core.py

@@ -1,7 +1,7 @@
 from flask import Blueprint, jsonify, request
 
 from notebooker.utils.results import get_all_available_results_json
-from notebooker.web.utils import get_serializer, get_all_possible_templates
+from notebooker.web.utils import get_serializer, get_all_possible_templates, all_templates_flattened
 
 core_bp = Blueprint("core_bp", __name__)
 
@@ -43,3 +43,13 @@ def get_all_possible_templates_url():
     :returns: A JSON which points from a report name to either its children or None if it is a leaf node.
     """
     return jsonify(get_all_possible_templates())
+
+
+@core_bp.route("/core/all_possible_templates_flattened")
+def all_possible_templates_flattened():
+    """
+    Core function which returns a flattened list of possible reports which a user can execute from the webapp.
+
+    :returns: A JSON which is a list of all possible templates with their full names.
+    """
+    return jsonify({"result": all_templates_flattened()})

notebooker/web/routes/scheduling.py

@@ -8,7 +8,7 @@
 from notebooker.utils.web import json_to_python
 from notebooker.web.handle_overrides import handle_overrides
 from notebooker.web.routes.run_report import validate_run_params
-from notebooker.web.utils import get_all_possible_templates
+from notebooker.web.utils import get_all_possible_templates, all_templates_flattened
 from apscheduler.triggers import cron
 
 scheduling_bp = Blueprint("scheduling_bp", __name__)
@@ -35,7 +35,7 @@ def all_schedules():
     return jsonify(result), 200
 
 
-@scheduling_bp.route("/scheduler/<string:job_id>", methods=["DELETE"])
+@scheduling_bp.route("/scheduler/<path:job_id>", methods=["DELETE"])
 def remove_schedule(job_id):
     job = current_app.apscheduler.get_job(job_id)
     if job is None:
@@ -83,7 +83,7 @@ def update_schedule(report_name):
 
 @scheduling_bp.route("/scheduler/create/<path:report_name>", methods=["POST"])
 def create_schedule(report_name):
-    if report_name not in get_all_possible_templates():
+    if report_name not in all_templates_flattened():
         return jsonify({"status": "Not found"}), 404
     issues = []
     trigger = validate_crontab(request.values.get("cron_schedule", ""), issues)

notebooker/web/static/notebooker/scheduler.js

@@ -77,12 +77,13 @@ function setScheduleModalMode(mode) {
 
 load_all_templates = (callback) => {
     $.ajax({
-        url: '/core/all_possible_templates',
+        url: '/core/all_possible_templates_flattened',
         dataType: 'json',
         success: (result) => {
-            var templates = Array();
-            for (var key in result) {
-                templates = templates.concat({"name": key, "value": key})
+            let templates = Array();
+            for (let i = 0; i < result.result.length; i++) {
+                let value = result.result[i]
+                templates = templates.concat({"name": value, "value": value})
             }
             $('.selection.dropdown').dropdown({
                 values: templates,
@@ -111,13 +112,17 @@ function showSelection() {
     }
 }
 
+function removeSchedulerIdFromURL() {
+    let url = new URL(window.location.href);
+    url.searchParams.delete('id');
+    history.replaceState({}, null, url.toString());
+}
+
 function showSchedulerModal() {
     $('#schedulerModal').modal({
         closable: true,
         onHidden() {
-            let url = new URL(window.location.href);
-            url.searchParams.delete('id');
-            history.replaceState({}, null, url.toString());
+            removeSchedulerIdFromURL()
         }
     }).modal('show');
 }
@@ -266,6 +271,7 @@ $(document).ready(() => {
                     if (data.status === 'Failed') {
                         handleFormError(data.content);
                     } else {
+                        removeSchedulerIdFromURL();
                         location.reload();
                     }
                 },

notebooker/web/utils.py

@@ -60,6 +60,6 @@ def get_directory_structure(starting_point: Optional[str] = None) -> Dict[str, U
     return all_dirs[rootdir[start:]]
 
 
-def _all_templates():
+def all_templates_flattened():
     templates = list(_gen_all_templates(get_all_possible_templates(warn_on_local=False)))
     return templates

tests/integration/web/test_core_routes.py

@@ -0,0 +1,11 @@
+import json
+
+
+def test_create_schedule(flask_app, setup_workspace):
+    with flask_app.test_client() as client:
+        rv = client.get(
+            "/core/all_possible_templates_flattened",
+        )
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data == {"result": ["fake/report"]}

tests/integration/test_scheduling.py → tests/integration/web/test_scheduling.py

@@ -6,7 +6,7 @@
 def test_create_schedule(flask_app, setup_workspace):
     with flask_app.test_client() as client:
         rv = client.post(
-            "/scheduler/create/fake",
+            "/scheduler/create/fake/report",
             data={
                 "report_title": "test2",
                 "report_name": "fake/report",
@@ -20,16 +20,16 @@ def test_create_schedule(flask_app, setup_workspace):
         assert data.pop("next_run_time")
         assert data == {
             "cron_schedule": "* * * * *",
-            "delete_url": "/scheduler/fake_test2",
-            "id": "fake_test2",
+            "delete_url": "/scheduler/fake/report_test2",
+            "id": "fake/report_test2",
             "params": {
                 "generate_pdf": False,
                 "hide_code": False,
                 "mailto": "",
                 "overrides": "",
-                "report_name": "fake",
+                "report_name": "fake/report",
                 "report_title": "test2",
-                "scheduler_job_id": "fake_test2",
+                "scheduler_job_id": "fake/report_test2",
             },
             "trigger": {
                 "fields": {
@@ -64,7 +64,7 @@ def test_create_schedule_bad_report_name(flask_app, setup_workspace):
 def test_list_scheduled_jobs(flask_app, setup_workspace):
     with flask_app.test_client() as client:
         rv = client.post(
-            "/scheduler/create/fake",
+            "/scheduler/create/fake/report",
             data={
                 "report_title": "test2",
                 "report_name": "fake/report",
@@ -79,13 +79,13 @@ def test_list_scheduled_jobs(flask_app, setup_workspace):
         assert rv.status_code == 200
         jobs = json.loads(rv.data)
         assert len(jobs) == 1
-        assert jobs[0]["id"] == "fake_test2"
+        assert jobs[0]["id"] == "fake/report_test2"
 
 
 def test_delete_scheduled_jobs(flask_app, setup_workspace):
     with flask_app.test_client() as client:
         rv = client.post(
-            "/scheduler/create/fake",
+            "/scheduler/create/fake/report",
             data={
                 "report_title": "test2",
                 "report_name": "fake/report",
@@ -100,7 +100,7 @@ def test_delete_scheduled_jobs(flask_app, setup_workspace):
         assert rv.status_code == 200
         assert len(json.loads(rv.data)) == 1
 
-        rv = client.delete("/scheduler/fake_test2")
+        rv = client.delete("/scheduler/fake/report_test2")
         assert rv.status_code == 200
 
         rv = client.get("/scheduler/jobs")
@@ -119,7 +119,7 @@ def fake_post(url, params):
             assert len(json.loads(rv.data)) == 0
 
             rv = client.post(
-                "/scheduler/create/fake",
+                "/scheduler/create/fake/report",
                 data={
                     "report_title": "test2",
                     "report_name": "fake/report",
@@ -132,4 +132,4 @@ def fake_post(url, params):
 
             time.sleep(60)  # this is the highest resolution for running jobs
             rv = client.get("core/get_all_available_results?limit=50")
-            assert len(json.loads(rv.data)) > 0
+            assert len(json.loads(rv.data)) > 0