diff --git a/lms/routes.py b/lms/routes.py index a655647217..34b032e9c1 100644 --- a/lms/routes.py +++ b/lms/routes.py @@ -148,6 +148,7 @@ def includeme(config): # pylint:disable=too-many-statements config.add_route("admin.instance.create", "/admin/instance/create") config.add_route("admin.instance.upgrade", "/admin/instance/upgrade") config.add_route("admin.instance", "/admin/instance/{id_}/") + config.add_route("admin.instance.settings", "/admin/instance/{id_}/settings") config.add_route("admin.instance.downgrade", "/admin/instance/{id_}/downgrade") config.add_route("admin.instance.move_org", "/admin/instance/{id_}/move_org") diff --git a/lms/templates/admin/application_instance/show.html.jinja2 b/lms/templates/admin/application_instance/show.html.jinja2 index 8e95d8cc1d..4cad8e6a2f 100644 --- a/lms/templates/admin/application_instance/show.html.jinja2 +++ b/lms/templates/admin/application_instance/show.html.jinja2 @@ -108,7 +108,7 @@
  • + action="{{ request.route_url("admin.instance.settings", id_=instance.id) }}">
    General settings diff --git a/lms/views/admin/application_instance/update.py b/lms/views/admin/application_instance/update.py index 3c3e249be9..14548a2324 100644 --- a/lms/views/admin/application_instance/update.py +++ b/lms/views/admin/application_instance/update.py @@ -19,8 +19,6 @@ class UpdateApplicationInstanceSchema(PyramidRequestSchema): name = fields.Str(required=True, validate=validate.Length(min=1)) lms_url = fields.URL(required=False) deployment_id = fields.Str(required=False) - developer_key = fields.Str(required=False) - developer_secret = fields.Str(required=False) class UpdateApplicationInstanceView(BaseApplicationInstanceView): @@ -51,7 +49,34 @@ def update_instance(self): developer_secret=self.request.params.get("developer_secret", "").strip(), ) + # Notes are displayed in the main info tab but are stored alongside notes + notes = self.request.params.get("hypothesis.notes") + notes = notes.strip() if notes else None + ai.settings.set("hypothesis", "notes", notes) + + self.request.session.flash(f"Updated application instance {ai.id}", "messages") + return self._redirect("admin.instance", id_=ai.id) + + @view_config( + route_name="admin.instance.settings", + request_method="POST", + require_csrf=True, + permission=Permissions.ADMIN, + ) + def update_instance_settings(self): + ai = self.application_instance + + self.application_instance_service.update_application_instance( + ai, + developer_key=self.request.params.get("developer_key", "").strip(), + developer_secret=self.request.params.get("developer_secret", "").strip(), + ) + for field in ApplicationSettings.fields: + # Notes are updated in the main `info` tab, skip it here + if field.compound_key == "hypothesis.notes": + continue + value = self.request.params.get(field.compound_key) value = value.strip() if value else None @@ -69,6 +94,8 @@ def update_instance(self): assert field.format == str ai.settings.set(field.group, field.key, value) - self.request.session.flash(f"Updated application instance {ai.id}", "messages") + self.request.session.flash( + f"Updated application instance settings for {ai.id}", "messages" + ) return self._redirect("admin.instance", id_=ai.id) diff --git a/tests/unit/lms/views/admin/application_instance/update_test.py b/tests/unit/lms/views/admin/application_instance/update_test.py index 4410f568f9..ae120f81da 100644 --- a/tests/unit/lms/views/admin/application_instance/update_test.py +++ b/tests/unit/lms/views/admin/application_instance/update_test.py @@ -17,8 +17,23 @@ def test_update_instance(self, views, pyramid_request, ai_from_matchdict): pyramid_request.route_url("admin.instance", id_=ai_from_matchdict.id) ) + @pytest.mark.parametrize( + "setting,sub_setting,value,expected", + ( + ("hypothesis", "notes", " NOTES ", "NOTES"), + ("hypothesis", "notes", "", None), + ), + ) def test_update_application_instance( - self, views, pyramid_request, ai_from_matchdict, application_instance_service + self, + views, + pyramid_request, + ai_from_matchdict, + application_instance_service, + setting, + sub_setting, + value, + expected, ): pyramid_request.params = pyramid_request.POST = { "name": " NAME ", @@ -27,6 +42,7 @@ def test_update_application_instance( "developer_key": " DEVELOPER KEY ", "developer_secret": " DEVELOPER SECRET ", } + pyramid_request.params[f"{setting}.{sub_setting}"] = value views.update_instance() @@ -38,6 +54,7 @@ def test_update_application_instance( developer_key="DEVELOPER KEY", developer_secret="DEVELOPER SECRET", ) + assert ai_from_matchdict.settings.get(setting, sub_setting) == expected @pytest.mark.usefixtures("with_minimal_fields_for_update") def test_update_application_instance_with_minimal_arguments( @@ -67,7 +84,6 @@ def test_update_application_instance_with_invalid_arguments( application_instance_service.update_application_instance.assert_not_called() - @pytest.mark.usefixtures("with_minimal_fields_for_update") @pytest.mark.parametrize( "setting,sub_setting,value,expected", ( @@ -89,8 +105,6 @@ def test_update_application_instance_with_invalid_arguments( ("vitalsource", "api_key", "api_key", "api_key"), ("vitalsource", "user_lti_param", "user_id", "user_id"), ("vitalsource", "user_lti_pattern", "name_(.*)", "name_(.*)"), - ("hypothesis", "notes", " NOTES ", "NOTES"), - ("hypothesis", "notes", "", None), ), ) def test_update_instance_saves_ai_settings( @@ -105,11 +119,10 @@ def test_update_instance_saves_ai_settings( ): pyramid_request.params[f"{setting}.{sub_setting}"] = value - views.update_instance() + views.update_instance_settings() assert ai_from_matchdict.settings.get(setting, sub_setting) == expected - @pytest.mark.usefixtures("with_minimal_fields_for_update") @pytest.mark.parametrize( "setting,sub_setting", (("desire2learn", "client_secret"),) ) @@ -125,7 +138,7 @@ def test_update_instance_saves_secret_settings( pyramid_request.params[f"{setting}.{sub_setting}"] = "SECRET" with patch.object(ai_from_matchdict.settings, "set_secret"): - views.update_instance() + views.update_instance_settings() ai_from_matchdict.settings.set_secret.assert_called_once_with( aes_service, setting, sub_setting, "SECRET"