Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PADV-1246: Enable external configuration for all courses #11

Merged
merged 1 commit into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 7 additions & 25 deletions lti_consumer/lti_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
from .utils import (
_,
resolve_custom_parameter_template,
external_config_filter_enabled,
external_user_id_1p1_launches_enabled,
database_config_enabled,
)
Expand Down Expand Up @@ -142,15 +141,13 @@ def valid_config_type_values(block):
valid value options, depending on the state of the appropriate toggle.
"""
values = [
{"display_name": _("Configuration on block"), "value": "new"}
{"display_name": _("Configuration on block"), "value": "new"},
{"display_name": _("Reusable Configuration"), "value": "external"},
]

if database_config_enabled(block.scope_ids.usage_id.context_key):
values.append({"display_name": _("Database Configuration"), "value": "database"})

if external_config_filter_enabled(block.scope_ids.usage_id.context_key):
values.append({"display_name": _("Reusable Configuration"), "value": "external"})

return values


Expand Down Expand Up @@ -724,31 +721,16 @@ def editable_fields(self):
currently selected for these fields. Because editable_fields defines a list of fields when that's used rendering
the Studio edit view, it cannot support the dynamic experience we want the user to have when editing the XBlock.
This property should return the set of all properties the user should be able to modify based on the current
environment. For example, if the external_config_filter_enabled flag is not enabled, the external_config field
should not be a part of editable_fields, because no user can edit this field in this case. On the other hand, if
the currently selected config_type is 'database', the fields that are otherwise stored in the database should
still be a part of editable_fields, because a user may select a different config_type from the menu, and we want
those fields to become editable at that time. The Javascript will determine when to show or to hide a given
field.
environment. For example, if the currently selected config_type is 'database', the fields that are otherwise
stored in the database should still be a part of editable_fields, because a user may select a different
config_type from the menu, and we want those fields to become editable at that time. The Javascript will
determine when to show or to hide a given field.

Fields that are potentially filtered out include "config_type", "external_config", "ask_to_send_username", and
"ask_to_send_email".
Fields that are potentially filtered out include "config_type", "ask_to_send_username", and "ask_to_send_email".
"""
editable_fields = self.editable_field_names
noneditable_fields = []

is_database_config_enabled = database_config_enabled(self.scope_ids.usage_id.context_key)
is_external_config_filter_enabled = external_config_filter_enabled(self.scope_ids.usage_id.context_key)

# If neither additional config_types are enabled, do not display the "config_type" field to users, as "new" is
# the only option and does not make sense without other options.
if not is_database_config_enabled and not is_external_config_filter_enabled:
noneditable_fields.append('config_type')

# If the enable_external_config_filter is not enabled, do not display the "external_config" field to users.
if not is_external_config_filter_enabled:
noneditable_fields.append('external_config')

# update the editable fields if this XBlock is configured to not to allow the
# editing of 'ask_to_send_username' and 'ask_to_send_email'.
pii_sharing_enabled = self.get_pii_sharing_enabled()
Expand Down
21 changes: 0 additions & 21 deletions lti_consumer/plugin/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,6 @@
# Namespace
WAFFLE_NAMESPACE = 'lti_consumer'

# Course Waffle Flags
# .. toggle_name: lti_consumer.enable_external_config_filter
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Enables fetching of LTI configurations from external
# sources like plugins using openedx-filters mechanism.
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2022-03-31
# .. toggle_tickets: https://github.com/openedx/xblock-lti-consumer/pull/239
# .. toggle_warning: None.
ENABLE_EXTERNAL_CONFIG_FILTER = 'enable_external_config_filter'

# .. toggle_name: lti_consumer.enable_external_user_id_1p1_launches
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
Expand All @@ -57,15 +45,6 @@
ENABLE_DATABASE_CONFIG = 'enable_database_config'


def get_external_config_waffle_flag():
"""
Import and return Waffle flag for enabling external LTI configuration.
"""
# pylint: disable=import-error,import-outside-toplevel
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
return CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.{ENABLE_EXTERNAL_CONFIG_FILTER}', __name__)


def get_external_user_id_1p1_launches_waffle_flag():
"""
Import and return Waffle flag for enabling sending external user IDs in LTI 1.1 launches.
Expand Down
46 changes: 4 additions & 42 deletions lti_consumer/tests/unit/test_lti_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,13 +536,10 @@ class TestEditableFields(TestLtiConsumerXBlock):

def setUp(self):
super().setUp()
self.mock_filter_enabled_patcher = patch("lti_consumer.lti_xblock.external_config_filter_enabled")
self.mock_database_config_enabled_patcher = patch("lti_consumer.lti_xblock.database_config_enabled")
self.mock_filter_enabled = self.mock_filter_enabled_patcher.start()
self.mock_database_config_enabled = self.mock_database_config_enabled_patcher.start()

def tearDown(self):
self.mock_filter_enabled_patcher.stop()
self.mock_database_config_enabled_patcher.stop()
super().tearDown()

Expand Down Expand Up @@ -604,50 +601,18 @@ def test_lti_1p3_fields_appear(self):
)
)

def test_external_config_fields_are_editable_only_when_waffle_flag_is_set(self):
"""
Test that the external configuration fields are editable only when the waffle flag is set.
"""
self.mock_filter_enabled.return_value = True
self.assertTrue(self.are_fields_editable(fields=['config_type', 'external_config']))

self.mock_filter_enabled.return_value = False
self.assertFalse(self.are_fields_editable(fields=['config_type', 'external_config']))

@ddt.idata(product([True, False], [True, False]))
@ddt.unpack
def test_database_config_fields_are_editable_only_when_waffle_flag_is_set(self, filter_enabled, db_enabled):
"""
Test that the database configuration fields are editable only when the waffle flag is set.
"""
self.mock_filter_enabled.return_value = filter_enabled

assert_fn = None
# If either flag is enabled, 'config_type' should be editable.
if db_enabled or filter_enabled:
assert_fn = self.assertTrue
else:
assert_fn = self.assertFalse

self.mock_database_config_enabled.return_value = db_enabled

assert_fn(self.are_fields_editable(fields=['config_type']))

@ddt.idata(product([True, False], [True, False]))
@ddt.unpack
def test_config_type_values(self, filter_enabled, db_enabled):
@ddt.data(True, False)
def test_config_type_values(self, db_enabled):
"""
Test that only the appropriate values for config_type are available as options, depending on the state of the
appropriate waffle flags.
"""
self.mock_filter_enabled.return_value = filter_enabled
self.mock_database_config_enabled.return_value = db_enabled

values = valid_config_type_values(self.xblock)

expected_values = ["new"]
if self.mock_filter_enabled:
expected_values.append('external')
expected_values = ["new", "external"]

if self.mock_database_config_enabled:
expected_values.append('database')

Expand Down Expand Up @@ -1667,11 +1632,8 @@ def setUp(self):
}
self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes)

self.mock_filter_enabled_patcher = patch("lti_consumer.lti_xblock.external_config_filter_enabled")
self.mock_database_config_enabled_patcher = patch("lti_consumer.lti_xblock.database_config_enabled")
self.mock_filter_enabled = self.mock_filter_enabled_patcher.start()
self.mock_database_config_enabled = self.mock_database_config_enabled_patcher.start()
self.addCleanup(self.mock_filter_enabled_patcher.stop)
self.addCleanup(self.mock_database_config_enabled_patcher.stop)

@patch.object(LtiConsumerXBlock, 'get_parameter_processors')
Expand Down
11 changes: 0 additions & 11 deletions lti_consumer/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from edx_django_utils.cache import get_cache_key, TieredCache

from lti_consumer.plugin.compat import (
get_external_config_waffle_flag,
get_external_user_id_1p1_launches_waffle_flag,
get_database_config_waffle_flag,
)
Expand Down Expand Up @@ -197,16 +196,6 @@ def resolve_custom_parameter_template(xblock, template):
return template_value


def external_config_filter_enabled(course_key):
"""
Returns True if external config filter is enabled for the course via Waffle Flag.

Arguments:
course_key (opaque_keys.edx.locator.CourseLocator): Course Key
"""
return get_external_config_waffle_flag().is_enabled(course_key)


def external_user_id_1p1_launches_enabled(course_key):
"""
Returns whether the lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag is enabled.
Expand Down
Loading