Merge pull request #865 from ministryofjustice/tool-release-values-an…

…d-chart-validation

Ensure chart name validation and chart context values re-used.

controlpanel/frontend/forms.py

@@ -285,6 +285,26 @@ def get_target_users(self):
         else:
             return []
 
+    def clean_chart_name(self):
+        """
+        Ensures that the helm chart name entered by the user is a valid one.
+
+        Why not use a "choice" argument in the Tool model's class? It would
+        result in a new Django migration for the database to enforce this (and
+        we'd have to keep adding new migrations to the database every time we
+        created a new helm chart for a new tool). Furthermore, the HTML select
+        field we'd use in the form isn't supported in the macros we use.
+
+        Hence the path of least resistance with this custom form validation.
+        """
+        valid_charts = ["airflow-sqlite", "jupyter-lab", "rstudio", ]
+        value = self.cleaned_data['chart_name']
+        if value not in valid_charts:
+            raise ValidationError(
+                f"'{value}' is not a valid helm chart name. ",
+            )
+        return value
+
     class Meta:
         model = Tool
         fields = ["name", "chart_name", "version", "is_restricted", ]

controlpanel/frontend/jinja2/release-create.html

@@ -42,7 +42,7 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
       },
       "classes": "govuk-!-width-two-thirds",
       "hint": {
-        "text": 'Helm chart name. 60 chars max, only lowercase letters and hyphens'
+        "text": 'Helm chart name. Use only one of: airflow-sqlite, jupyter-lab or rstudio'
       },
       "name": "chart_name",
       "attributes": {

controlpanel/frontend/jinja2/release-detail.html

@@ -44,7 +44,7 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
       },
       "classes": "govuk-!-width-two-thirds",
       "hint": {
-        "text": 'Helm chart name. 60 chars max, only lowercase letters and hyphens'
+        "text": 'Helm chart name. Use only one of: airflow-sqlite, jupyter-lab or rstudio'
       },
       "name": "chart_name",
       "attributes": {

controlpanel/frontend/views/release.py

@@ -86,5 +86,19 @@ def form_valid(self, form):
         target_list = form.get_target_users()
         if target_list:
             self.object.target_users.set(target_list)
+        # Populate the JSON values needed as arguments for the referenced helm
+        # chart to run.
+        # These are taken from the most recent valid version of the helm chart
+        # (these settings should remain the same across chart versions).
+        old_release = (
+            # This query is lazy, and starts by matching tools by chart_name...
+            Tool.objects.filter(chart_name=self.object.chart_name)
+            .exclude(values={})  # ...throw away those with empty values, and,
+            .order_by("-created")  # order by latest first, but...
+            .first()  # ...get only the first result.
+        )
+        # Update the values of the new release with those from the old release.
+        self.object.values = old_release.values
+        self.object.save()
         messages.success(self.request, "Successfully created new release")
         return HttpResponseRedirect(reverse_lazy("list-tool-releases"))

tests/frontend/views/test_release.py

@@ -19,3 +19,29 @@ def test_release_detail_context_data():
     v.object = mock_object
     result = v.get_context_data()
     assert result["target_users"] == "aldo, nicholas"
+
+
+def test_release_create_form_valid():
+    """
+    Ensure that JSON values from a previous release are annotated to the
+    new release.
+
+    HERE BE DRAGONS: use of mocking to avoid database related errors with
+    pytest test-runner (that doesn't create temp test databases).
+    """
+    JSONValues = {"foo": "bar", }  # placeholder mock data for JSON values.
+    v = release.ReleaseCreate()
+    v.request = mock.MagicMock()
+    old_tool = mock.MagicMock()
+    old_tool.values = JSONValues
+    v.request.method = "POST"
+    mock_form = mock.MagicMock()
+    mock_object = mock.MagicMock()
+    mock_form.save.return_value = mock_object
+    mock_form.get_target_users.return_value = []
+    mock_tool = mock.MagicMock()
+    mock_tool.objects.filter().exclude().order_by().first.return_value = old_tool
+    with mock.patch("controlpanel.frontend.views.release.Tool", mock_tool):
+        v.form_valid(mock_form)
+    assert mock_object.values == JSONValues
+    mock_object.save.assert_called_once_with()