From aabfd55f7c437e005653ff955d78b3e1dcf5762b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Mon, 26 Dec 2016 10:27:19 +0200
Subject: [PATCH] Cleanup and tests for cron preview.

---
 hc/front/forms.py                     |  6 +++--
 hc/front/tests/test_cron_preview.py   | 32 +++++++++++++++++++++++++++
 hc/front/tests/test_update_timeout.py | 24 +++++++++++++++++---
 hc/front/validators.py                |  9 ++++++++
 hc/front/views.py                     | 24 +++++++++++---------
 static/css/my_checks.css              | 11 +++------
 static/js/checks.js                   |  2 +-
 static/js/tab-native.js               |  4 +---
 templates/front/cron_preview.html     | 24 +++++++++-----------
 9 files changed, 95 insertions(+), 41 deletions(-)
 create mode 100644 hc/front/tests/test_cron_preview.py

diff --git a/hc/front/forms.py b/hc/front/forms.py
index 0e1c7363..d6dcac01 100644
--- a/hc/front/forms.py
+++ b/hc/front/forms.py
@@ -1,5 +1,6 @@
 from django import forms
-from hc.front.validators import CronExpressionValidator, WebhookValidator
+from hc.front.validators import (CronExpressionValidator, TimezoneValidator,
+                                 WebhookValidator)
 
 
 class NameTagsForm(forms.Form):
@@ -25,7 +26,8 @@ class TimeoutForm(forms.Form):
 class CronForm(forms.Form):
     schedule = forms.CharField(required=False, max_length=100,
                                validators=[CronExpressionValidator()])
-    tz = forms.CharField(required=False, max_length=36)
+    tz = forms.CharField(required=False, max_length=36,
+                         validators=[TimezoneValidator()])
     grace = forms.IntegerField(min_value=1, max_value=43200)
 
 
diff --git a/hc/front/tests/test_cron_preview.py b/hc/front/tests/test_cron_preview.py
new file mode 100644
index 00000000..975bfe2a
--- /dev/null
+++ b/hc/front/tests/test_cron_preview.py
@@ -0,0 +1,32 @@
+from hc.test import BaseTestCase
+
+
+class CronPreviewTestCase(BaseTestCase):
+
+    def test_it_works(self):
+        payload = {
+            "schedule": "* * * * *",
+            "tz": "UTC"
+        }
+        r = self.client.post("/checks/cron_preview/", payload)
+        self.assertContains(r, "cron-preview-title", status_code=200)
+
+    def test_it_handles_invalid_cron_expression(self):
+        for schedule in [None, "", "*", "100 100 100 100 100"]:
+            payload = {"schedule": schedule, "tz": "UTC"}
+            r = self.client.post("/checks/cron_preview/", payload)
+            self.assertContains(r, "Invalid cron expression", status_code=200)
+
+    def test_it_handles_invalid_timezone(self):
+        for tz in [None, "", "not-a-timezone"]:
+            payload = {"schedule": "* * * * *", "tz": tz}
+            r = self.client.post("/checks/cron_preview/", payload)
+            self.assertContains(r, "Invalid timezone", status_code=200)
+
+    def test_it_handles_missing_arguments(self):
+        r = self.client.post("/checks/cron_preview/", {})
+        self.assertContains(r, "Invalid cron expression", status_code=200)
+
+    def test_it_rejects_get(self):
+        r = self.client.get("/checks/cron_preview/", {})
+        self.assertEqual(r.status_code, 400)
diff --git a/hc/front/tests/test_update_timeout.py b/hc/front/tests/test_update_timeout.py
index ca2ba537..fc0a4a31 100644
--- a/hc/front/tests/test_update_timeout.py
+++ b/hc/front/tests/test_update_timeout.py
@@ -33,7 +33,6 @@ class UpdateTimeoutTestCase(BaseTestCase):
             "kind": "cron",
             "schedule": "5 * * * *",
             "tz": "UTC",
-            "timeout": 60,
             "grace": 60
         }
 
@@ -54,13 +53,32 @@ class UpdateTimeoutTestCase(BaseTestCase):
             "kind": "cron",
             "schedule": "* invalid *",
             "tz": "UTC",
-            "timeout": 60,
             "grace": 60
         }
 
         self.client.login(username="alice@example.org", password="password")
         r = self.client.post(url, data=payload)
-        self.assertRedirects(r, "/checks/")
+        self.assertEqual(r.status_code, 400)
+
+        # Check should still have its original data:
+        self.check.refresh_from_db()
+        self.assertEqual(self.check.kind, "simple")
+
+    def test_it_validates_tz(self):
+        self.check.last_ping = None
+        self.check.save()
+
+        url = "/checks/%s/timeout/" % self.check.code
+        payload = {
+            "kind": "cron",
+            "schedule": "* * * * *",
+            "tz": "not-a-tz",
+            "grace": 60
+        }
+
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.post(url, data=payload)
+        self.assertEqual(r.status_code, 400)
 
         # Check should still have its original data:
         self.check.refresh_from_db()
diff --git a/hc/front/validators.py b/hc/front/validators.py
index 7cdefb9e..1a843c8c 100644
--- a/hc/front/validators.py
+++ b/hc/front/validators.py
@@ -1,6 +1,7 @@
 from croniter import croniter
 from django.core.exceptions import ValidationError
 from six.moves.urllib_parse import urlparse
+from pytz import all_timezones
 
 
 class WebhookValidator(object):
@@ -23,3 +24,11 @@ class CronExpressionValidator(object):
             croniter(value)
         except:
             raise ValidationError(message=self.message)
+
+
+class TimezoneValidator(object):
+    message = "Not a valid time zone."
+
+    def __call__(self, value):
+        if value not in all_timezones:
+            raise ValidationError(message=self.message)
diff --git a/hc/front/views.py b/hc/front/views.py
index c5dea32a..774f10d8 100644
--- a/hc/front/views.py
+++ b/hc/front/views.py
@@ -22,6 +22,7 @@ from hc.front.forms import (AddWebhookForm, NameTagsForm,
                             TimeoutForm, AddUrlForm, AddPdForm, AddEmailForm,
                             AddOpsGenieForm, CronForm)
 from pytz import all_timezones
+from pytz.exceptions import UnknownTimeZoneError
 
 
 # from itertools recipes:
@@ -173,7 +174,7 @@ def update_timeout(request, code):
     if kind == "simple":
         form = TimeoutForm(request.POST)
         if not form.is_valid():
-            return redirect("hc-checks")
+            return HttpResponseBadRequest()
 
         check.kind = "simple"
         check.timeout = td(seconds=form.cleaned_data["timeout"])
@@ -181,7 +182,7 @@ def update_timeout(request, code):
     elif kind == "cron":
         form = CronForm(request.POST)
         if not form.is_valid():
-            return redirect("hc-checks")
+            return HttpResponseBadRequest()
 
         check.kind = "cron"
         check.schedule = form.cleaned_data["schedule"]
@@ -197,23 +198,24 @@ def update_timeout(request, code):
 
 @csrf_exempt
 def cron_preview(request):
+    if request.method != "POST":
+        return HttpResponseBadRequest()
+
     schedule = request.POST.get("schedule")
     tz = request.POST.get("tz")
-
-    ctx = {
-        "tz": tz,
-        "dates": []
-    }
-
+    ctx = {"tz": tz, "dates": []}
     try:
         with timezone.override(tz):
             now_naive = timezone.make_naive(timezone.now())
             it = croniter(schedule, now_naive)
             for i in range(0, 6):
-                date_naive = it.get_next(datetime)
-                ctx["dates"].append(timezone.make_aware(date_naive))
+                naive = it.get_next(datetime)
+                aware = timezone.make_aware(naive)
+                ctx["dates"].append((naive, aware))
+    except UnknownTimeZoneError:
+        ctx["bad_tz"] = True
     except:
-        ctx["error"] = True
+        ctx["bad_schedule"] = True
 
     return render(request, "front/cron_preview.html", ctx)
 
diff --git a/static/css/my_checks.css b/static/css/my_checks.css
index 2d1b7c5c..3f907096 100644
--- a/static/css/my_checks.css
+++ b/static/css/my_checks.css
@@ -30,10 +30,6 @@
     width: 70px;
 }
 
-#update-timeout-simple {
-    display: none;
-}
-
 #update-cron-form .modal-body {
     padding: 40px;
 }
@@ -62,15 +58,15 @@
     font-size: small;
 }
 
-.cron-preview-date {
+#cron-preview-table tr td:nth-child(1) {
     width: 120px;
 }
 
-.cron-preview-rel {
+#cron-preview-table tr td:nth-child(2) {
     font-size: small;
 }
 
-.cron-preview-timestamp {
+#cron-preview-table tr td:nth-child(3) {
     font-size: small;
     font-family: monospace;
     text-align: right;
@@ -106,7 +102,6 @@
     margin: 0;
 }
 
-
 .update-timeout-terms span {
     font-weight: bold;
 }
diff --git a/static/js/checks.js b/static/js/checks.js
index b76d9919..ec3baa3f 100644
--- a/static/js/checks.js
+++ b/static/js/checks.js
@@ -126,7 +126,7 @@ $(function () {
                 }
 
                 $("#cron-preview" ).html(data);
-                var haveError = $("#invalid-cron-expression").size() > 0;
+                var haveError = $("#invalid-arguments").size() > 0;
                 $("#update-cron-submit").prop("disabled", haveError);
             }
         );
diff --git a/static/js/tab-native.js b/static/js/tab-native.js
index 6fa5c84d..fe9cbfb4 100644
--- a/static/js/tab-native.js
+++ b/static/js/tab-native.js
@@ -24,7 +24,7 @@
 	// ===================
 	var Tab = function( element,options ) {
 		options = options || {};
-		
+
 		this.tab = typeof element === 'object' ? element : document.querySelector(element);
 		this.tabs = this.tab.parentNode.parentNode;
 		this.dropdown = this.tabs.querySelector('.dropdown');
@@ -94,8 +94,6 @@
 				} else if ( activeTabs.length > 1 ) {
 					return activeTabs[activeTabs.length-1]
 				}
-
-				console.log(activeTabs.length)
 			},
 			this.getActiveContent = function() {
 				var a = self.getActiveTab().getElementsByTagName('A')[0].getAttribute('href').replace('#','');
diff --git a/templates/front/cron_preview.html b/templates/front/cron_preview.html
index 60b2e1bb..63a8fe30 100644
--- a/templates/front/cron_preview.html
+++ b/templates/front/cron_preview.html
@@ -1,20 +1,18 @@
 {% load humanize tz %}
 
-{% if error %}
-<p id="invalid-cron-expression">Invalid cron expression</p>
+{% if bad_schedule %}
+<p id="invalid-arguments">Invalid cron expression</p>
+{% elif bad_tz %}
+<p id="invalid-arguments">Invalid timezone</p>
 {% else %}
-<table class="table">
+<table id="cron-preview-table" class="table">
+    <tr><th id="cron-preview-title" colspan="3">Expected Ping Dates</th></tr>
+    {% for naive, aware in dates %}
     <tr>
-        <th id="cron-preview-title" colspan="3">Expected Ping Dates</th>
-    </tr>
-    {% for date in dates %}
-    <tr>
-        {% timezone tz %}
-        <td class="cron-preview-date">{{ date|date:"M j, H:i" }}</td>
-        {% endtimezone %}
-        <td class="cron-preview-rel">{{ date|naturaltime }}</td>
-        <td class="cron-preview-timestamp">{{ date|date:"c" }}</td>
+        <td>{{ naive|date:"M j, H:i" }}</td>
+        <td>{{ aware|naturaltime }}</td>
+        <td>{{ aware|date:"c" }}</td>
     </tr>
     {% endfor %}
 </table>
-{% endif %}
\ No newline at end of file
+{% endif %}