From 4ac3caab236270f10823586a2f8c4c6966e9cc97 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2019 13:06:08 -0500 Subject: [PATCH] Add ADRs for FBE decisions REVMI-39 --- .../0001-xblock-content-overrides.rst | 50 +++++++++++++++++ .../decisions/0001-group-access-overrides.rst | 56 +++++++++++++++++++ .../docs/decisions/0002-no-new-track.rst | 37 ++++++++++++ .../docs/decisions/0001-access-messages.rst | 43 ++++++++++++++ 4 files changed, 186 insertions(+) create mode 100644 lms/djangoapps/courseware/docs/decisions/0001-xblock-content-overrides.rst create mode 100644 openedx/features/content_type_gating/docs/decisions/0001-group-access-overrides.rst create mode 100644 openedx/features/content_type_gating/docs/decisions/0002-no-new-track.rst create mode 100644 openedx/features/course_duration_limits/docs/decisions/0001-access-messages.rst diff --git a/lms/djangoapps/courseware/docs/decisions/0001-xblock-content-overrides.rst b/lms/djangoapps/courseware/docs/decisions/0001-xblock-content-overrides.rst new file mode 100644 index 0000000000..0a067f691c --- /dev/null +++ b/lms/djangoapps/courseware/docs/decisions/0001-xblock-content-overrides.rst @@ -0,0 +1,50 @@ +1. XBlock Content Replacement for Access Control +================================================ + +Status +------ + +Accepted + +Context +------- + +Content Type Gating requires that users be prevented from viewing +or interacting with an XBlock, and instead being shown a message +suggesting how to gain access. XBlocks are rendered in a tree, +and the rendering is managed by the XBlocks themselves. XBlocks +expect to be able to retrieve their own children, inspect attributes +of those children, and then render those children w/ a method call. + +XBlock access control removes a child from the render tree, which +makes it difficult to know where to generate the replacement message +during rendering. Substituting one xblock for another of a different +type at runtime has historically problematic, because the different +XBlock types don't behave the same way. Replacing an XBlock with a +non-XBlock type is difficult for the same reason. + +Decision +-------- + +We will continue to manage access-control decisions using ``has_access``. +When access is denied, we will generate a custom ``Fragment`` containing +the error message, and attach it to the ``AccessResponse`` object. + +While rendering the XBlock hierarchy, we will continue to check access +as is currently implemented. If the ``AccessResponse`` returned has a +user-facing error fragment (stored in ``AccessResponse.user_fragment``), +then we will act as though the XBlock was not access restricted, and +allow it to be loaded. + +In order to actually restrict access to the XBlock, we will add an +``xblock_wrapper`` that will again check whether a user has permission +to view the xblock. If the user doesn't, and the ``AccessResponse`` +has a ``user_fragment`` set, then that content will be substituted +for the XBlock's originally rendered ``Fragment``. + +Consequences +------------ + +XBlocks can be access-limited with custom user-facing messaging. +There is a performance cost to those messages, because the XBlock +that is being access-limited will still render its own ``Fragment``. diff --git a/openedx/features/content_type_gating/docs/decisions/0001-group-access-overrides.rst b/openedx/features/content_type_gating/docs/decisions/0001-group-access-overrides.rst new file mode 100644 index 0000000000..a81de005ea --- /dev/null +++ b/openedx/features/content_type_gating/docs/decisions/0001-group-access-overrides.rst @@ -0,0 +1,56 @@ +1. Group Access Overrides +========================= + +Status +------ + +Accepted + +Context +------- + +In order to implement Feature Based Enrollment (FBE), we need to be able +to automatically restrict content to a specific set of authorized +users. This automatic restriction needs to be based on whether content +is graded and capable of producing a score. Whether a particular XBlock +is graded and scorable is denoted by attributes on that XBlock. + +There are two separate systems that need load XBlocks and need to +be restricted: the courseware rendering on the web, and the courseware +rendering in the mobile app. Courseware rendering accesses XBlock +attributes via the ``FieldData`` api, and can be modified by +``FieldDataOverrides``. The mobile app uses the course_blocks api, +which is backed by ``BlockTransformers``. + +Most differentiated content is managed by ``UserPartitions``. These +segment users by customizable criteria, and then can be applied as +access control by setting the ``group_access`` field on an ``XBlock``. +There is an existing ``UserPartition`` that segments users based on +their enrollment track (the ``EnrollmentTrackUserPartition``. However, +many courses already manage content using the ``EnrollmentTrackUserPartition``, +so it isn't a good candidate to overwrite for FBE. + + +Decision +-------- + +In order to restrict access to graded content, we will create a new +``UserPartition`` specifically for FBE. That partition will segment +users based on their enrollment track and any other criteria that become +relevant for FBE implementation. We will override the ``group_access`` +of all graded and scorable XBlocks by using a new ``FieldDataOverride`` +for in-courseware acccess, and a new ``BlockTransformer`` for mobile/ +course_blocks api access. Both of those will respect the same rules for +when to override the ``group_access`` attribute. + +Consequences +------------ + +All graded content (with possible manual exceptions) will have ``group_access`` +overridden to assign the content to the "Full Access Only" partition of +the new ``ContentTypeGatingPartition``. Studio Authors will see reference +to both the ``ContentTypeGatingPartition`` (Feature Based Enrollment Partition) +and the ``EnrollmentTrack`` partition in Studio, if they are authorized to +modify them. Masquerade, which only modifies a single group at a time, is +unable to provide an accurate simulation of the interaction between having +graded content restricted behind the ``EnrollmentTrack`` partition. diff --git a/openedx/features/content_type_gating/docs/decisions/0002-no-new-track.rst b/openedx/features/content_type_gating/docs/decisions/0002-no-new-track.rst new file mode 100644 index 0000000000..35ce653762 --- /dev/null +++ b/openedx/features/content_type_gating/docs/decisions/0002-no-new-track.rst @@ -0,0 +1,37 @@ +2. No New Enrollment Mode +========================= + +Status +------ + +Accepted + +Context +------- + +In order to implement Feature Based Enrollment (FBE), we need a way +to differentiate between users who have access to graded content +and those who don't. We need to be able to move users between those +groups. Users that have access to graded content should also get +all of the current behavior associated with Verified users. We +need to leave existing users (in any track) with their existing behavior. +Many permissions related to Enrollment Modes are checked by checking +whether an enrollment has a specific mode, which makes it hard +to add a new mode that has the same set of permissions. + + +Decision +-------- + +Rather than adding a new Enrollment Track that we move Full-Access users +into, we will add a new ``UserPartition`` to distinguish Limited-Access Users +and Full-Access Users. + +Consequences +------------ + +Some Studio Authors see both the new ``ContentTypeGatingPartition`` and the +``EnrollmentTrackUserPartition`` in their UI. Masquerade is unable to +masquerade as a user in a specific group and see graded content that has +been limited using the ``EnrollemntTrackUserPartition``, because it doesn't +multi-select groups. diff --git a/openedx/features/course_duration_limits/docs/decisions/0001-access-messages.rst b/openedx/features/course_duration_limits/docs/decisions/0001-access-messages.rst new file mode 100644 index 0000000000..2ada6e0c03 --- /dev/null +++ b/openedx/features/course_duration_limits/docs/decisions/0001-access-messages.rst @@ -0,0 +1,43 @@ +1. Pre-formatted Access Messages +================================ + +Status +------ + +Accepted + +Context +------- + +In ``course_duration_limits``, we are adding a new permissions +state that will restrict the user from entering the course. +The point at which this condition is checked is deep within +edx-platform, but the error message needs to display at the +surface of the application. In order to preserve information +display consistency, we want to receive the error message +in the UI in a standard format. We can use ``AccessResponse.user_message`` +to store that permission-specific error message. However, +different pages require more or less context. In particular, +when displaying an access error message inside the courseware, +we don't need to specify the current course, but when displaying +the same message on the course dashboard, we do. + +Decision +-------- + +We will add a field to ``AccessResponse``, ``additional_context_user_message``, +which will be used in non-course-specific contexts (for +``course_duration_limits``. The name is non-specific in order to enable it +to be used more generally by other access-control schemes that might have +different levels of context display needs. + +Consequences +------------ + +``AccessResponse`` messages can be more detailed, and more specific +to the required context. The additional attribute on ``AccessResponse`` +is somewhat vague, and potentially confusing to a new reader. Which additional +context is relevant is not specified, so if we need more or different +context (rather than just course-context), we will need to rework the +current system or add more attributes. +