From d5bf7093708e73127ec4acc1ca8c893fb813c78b Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Wed, 14 Aug 2024 10:11:19 +0200 Subject: [PATCH] Enable dashboard for all instructors --- lms/security.py | 7 +------ .../admin/application_instance/show.html.jinja2 | 1 - lms/views/lti/basic_launch.py | 6 +----- tests/unit/lms/security_test.py | 11 +---------- tests/unit/lms/views/lti/basic_launch_test.py | 7 +------ 5 files changed, 4 insertions(+), 28 deletions(-) diff --git a/lms/security.py b/lms/security.py index 93dce390e3..36dfa8efbe 100644 --- a/lms/security.py +++ b/lms/security.py @@ -212,12 +212,7 @@ def identity(self, request) -> Identity | None: if lti_user.is_instructor: permissions.append(Permissions.LTI_CONFIGURE_ASSIGNMENT) permissions.append(Permissions.GRADE_ASSIGNMENT) - - if lti_user.application_instance.settings.get( - "hypothesis", "instructor_dashboard" - ): - # Only include this permission if the feature is enabled - permissions.append(Permissions.DASHBOARD_VIEW) + permissions.append(Permissions.DASHBOARD_VIEW) return Identity(self._get_userid(lti_user), permissions, lti_user) diff --git a/lms/templates/admin/application_instance/show.html.jinja2 b/lms/templates/admin/application_instance/show.html.jinja2 index a6c58b64d2..3f55c20139 100644 --- a/lms/templates/admin/application_instance/show.html.jinja2 +++ b/lms/templates/admin/application_instance/show.html.jinja2 @@ -113,7 +113,6 @@
General settings {{ settings_checkbox('Enable instructor email digests', 'hypothesis', 'instructor_email_digests_enabled') }} - {{ settings_checkbox('Enable instructor dashboard', 'hypothesis', 'instructor_dashboard') }} {{ settings_checkbox("Use alternative parameter for LTI1.3 grading", "hypothesis", "lti_13_sourcedid_for_grading", default=False) }}
diff --git a/lms/views/lti/basic_launch.py b/lms/views/lti/basic_launch.py index 76bc99d7b8..519bb50a13 100644 --- a/lms/views/lti/basic_launch.py +++ b/lms/views/lti/basic_launch.py @@ -220,15 +220,11 @@ def _show_document(self, assignment): self.request.lti_params.get("lis_outcome_service_url"), ) - application_instance = self.request.lti_user.application_instance - # Set up the JS config for the front-end self.context.js_config.add_document_url(assignment.document_url) self.context.js_config.enable_lti_launch_mode(self.course, assignment) - if self.request.lti_user.is_instructor and application_instance.settings.get( - "hypothesis", "instructor_dashboard" - ): + if self.request.lti_user.is_instructor: self.context.js_config.enable_instructor_dashboard_entry_point(assignment) # If there are any Hypothesis client feature flags that need to be diff --git a/tests/unit/lms/security_test.py b/tests/unit/lms/security_test.py index 995be603bf..92eb84f187 100644 --- a/tests/unit/lms/security_test.py +++ b/tests/unit/lms/security_test.py @@ -4,7 +4,6 @@ from pyramid.interfaces import ISecurityPolicy from pyramid.security import Allowed, Denied -from lms.models import ApplicationSettings from lms.security import ( CookiesBearerTokenLTIUserPolicy, DeniedWithException, @@ -277,18 +276,12 @@ def test_identity_when_theres_an_lti_user_with_no_roles( assert not identity.permissions assert identity.userid - @pytest.mark.parametrize( - "settings", - [{}, {"hypothesis": {"instructor_dashboard": True}}], - ) @pytest.mark.parametrize("is_instructor", [True, False]) def test_identity_when_theres_an_lti_user( self, request, pyramid_request, is_instructor, - application_instance, - settings, policy, get_lti_user, ): @@ -296,7 +289,6 @@ def test_identity_when_theres_an_lti_user( _ = request.getfixturevalue("user_is_instructor") else: _ = request.getfixturevalue("user_is_learner") - settings = application_instance.settings = ApplicationSettings(settings) get_lti_user.return_value = pyramid_request.lti_user identity = policy.identity(pyramid_request) @@ -313,10 +305,9 @@ def test_identity_when_theres_an_lti_user( [ Permissions.LTI_CONFIGURE_ASSIGNMENT, Permissions.GRADE_ASSIGNMENT, + Permissions.DASHBOARD_VIEW, ] ) - if settings.get("hypothesis", "instructor_dashboard"): - expected_permissions.append(Permissions.DASHBOARD_VIEW) assert identity.permissions == expected_permissions diff --git a/tests/unit/lms/views/lti/basic_launch_test.py b/tests/unit/lms/views/lti/basic_launch_test.py index fa9c1966c5..89b2e58eba 100644 --- a/tests/unit/lms/views/lti/basic_launch_test.py +++ b/tests/unit/lms/views/lti/basic_launch_test.py @@ -274,7 +274,6 @@ def test__show_document( @pytest.mark.parametrize("use_toolbar_editing", [True, False]) @pytest.mark.parametrize("use_toolbar_grading", [True, False]) - @pytest.mark.parametrize("instructor_dashboard_enabled", [True, False]) @pytest.mark.parametrize("is_gradable", [True, False]) @pytest.mark.parametrize("is_instructor", [True, False]) def test__show_document_configures_toolbar( @@ -286,7 +285,6 @@ def test__show_document_configures_toolbar( lti_user, use_toolbar_editing, use_toolbar_grading, - instructor_dashboard_enabled, is_gradable, is_instructor, grading_info_service, @@ -297,9 +295,6 @@ def test__show_document_configures_toolbar( request.getfixturevalue("user_is_instructor") pyramid_request.product.use_toolbar_grading = use_toolbar_grading pyramid_request.product.use_toolbar_editing = use_toolbar_editing - lti_user.application_instance.settings.set( - "hypothesis", "instructor_dashboard", instructor_dashboard_enabled - ) result = svc._show_document(assignment) # noqa: SLF001 @@ -309,7 +304,7 @@ def test__show_document_configures_toolbar( else: context.js_config.enable_toolbar_editing.assert_not_called() - if instructor_dashboard_enabled and is_instructor: + if is_instructor: context.js_config.enable_instructor_dashboard_entry_point.assert_called_once() else: context.js_config.enable_instructor_dashboard_entry_point.assert_not_called()