diff --git a/controlpanel/frontend/forms.py b/controlpanel/frontend/forms.py index 8f619b346..c683acc82 100644 --- a/controlpanel/frontend/forms.py +++ b/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", ] diff --git a/controlpanel/frontend/jinja2/release-create.html b/controlpanel/frontend/jinja2/release-create.html index 3b2306106..d352dd77d 100644 --- a/controlpanel/frontend/jinja2/release-create.html +++ b/controlpanel/frontend/jinja2/release-create.html @@ -42,7 +42,7 @@

{{ page_title }}

}, "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": { diff --git a/controlpanel/frontend/jinja2/release-detail.html b/controlpanel/frontend/jinja2/release-detail.html index ecc2f7f31..a60e0e857 100644 --- a/controlpanel/frontend/jinja2/release-detail.html +++ b/controlpanel/frontend/jinja2/release-detail.html @@ -44,7 +44,7 @@

{{ page_title }}

}, "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": { diff --git a/controlpanel/frontend/views/release.py b/controlpanel/frontend/views/release.py index 43281ea7b..0671b7200 100644 --- a/controlpanel/frontend/views/release.py +++ b/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")) diff --git a/tests/frontend/views/test_release.py b/tests/frontend/views/test_release.py index 06937d342..577104688 100644 --- a/tests/frontend/views/test_release.py +++ b/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()