Skip to content

Commit 843b599

Browse files
committed
docs: ADR for in context discussions
1 parent 5b9e84d commit 843b599

1 file changed

Lines changed: 137 additions & 0 deletions

File tree

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
Linking In-Context Discussions to Units
2+
=======================================
3+
4+
5+
Status
6+
------
7+
8+
Proposal
9+
10+
11+
Context
12+
-------
13+
14+
As part of the BD-38 initiative (Blended Development, Project 38), we are
15+
investigating a new way to set up discussions for a course that tries to
16+
simplify the way discussions are configured and ties them more closely to the
17+
course structure.
18+
19+
The first simplification that the new system makes is to remove the
20+
discussions XBlock, and instead supports simply marking individual units
21+
as discussable, in which case a discussion UI will show up in-context.
22+
23+
Currently this will only be supported for the edX Forums internal
24+
discussion provider, but it could be extended in the future to support
25+
other providers.
26+
27+
In addition to providing a way to mark Units as discussable, there will
28+
also be a way to simply enable discussion for all units in the course.
29+
One further control provided here will be to toggle discussions on or
30+
off for graded discussions.
31+
32+
Finally, it will also be possible to group discussions at a Sequence
33+
level instead of the Unit level.
34+
35+
36+
Requirements
37+
------------
38+
39+
In order to enable in-context discussions with the above features we
40+
need a way to:
41+
42+
- Mark a Unit as discussable
43+
- Store configuration options for discussions
44+
- Automatically mark/unmark units as discussable when settings change
45+
- Associate a unit with a corresponding discussion id
46+
- Make this discussion ID available via an API
47+
48+
49+
Consideration
50+
-------------
51+
52+
Most of these configuration entries would be right at home in the
53+
`DiscussionsConfiguration` model in `plugin_settings`, however since they need
54+
to be available during course import-export, they should be stored in the
55+
course object itself.
56+
57+
There is already a way to associate a discussion ID with a Discussion XBlock
58+
using its usage key. This same mechanism can be used to associate a Unit usage
59+
key with a corresponding discussion id.
60+
61+
However the current mechanism has a few issues. It is stored as a JSON
62+
structure in the `DiscussionsIdMapping` model which has course id and a mapping
63+
of the discussion id to the xblock usage key in a single dict.
64+
65+
This is OK for the existing setup because this is just a caching mechanism and
66+
the source of truth for this mapping is the XBlock itself, which stores the
67+
discussion id. On course publish this information is cached to
68+
`DiscussionsIdMapping`.
69+
70+
For the new discussions system though, this mapping would be the source of
71+
truth for the link between discussions and units, so we should use a model
72+
where each such link is encoded as a row in the database.
73+
74+
Decision
75+
--------
76+
77+
Since the discussions settings need to be stored in the course structure we
78+
should create a new JSON structure in the course that matches the structure
79+
of `plugin_settings`. This can then be used to store not just the settings
80+
for the inbuilt discussions provider, but for any discussions provider in the
81+
future.
82+
83+
When a course is published, we can copy over all the `plugin_settings` to the
84+
course in a JSON field called `dicussions_settings` with the following
85+
structure:
86+
87+
.. code-block:: JSON
88+
89+
{
90+
"discussions_enable_in_context": bool,
91+
"discussions_enable_graded_units": bool,
92+
"discussions_custom_visibility": bool,
93+
"edx-next": {
94+
"discussions_group_at_subsection": bool,
95+
}
96+
}
97+
98+
The `edx-next` key here represents the provider id, allowing for potentially
99+
multiple provider configs to coexist in case of switching providers etc.
100+
Settings outside this key are those that are applicable to all providers. Note
101+
that they may not be supported by all providers though, in which case they will
102+
simply be ignored.
103+
104+
To store Unit-level discussions settings, we can simply add a boolean field
105+
to the Unit block that specifies whether it is discussable or not. To be
106+
consistent with the above names we can call this field `discussions_enabled`.
107+
108+
A signal can be created using the new Hooks extension system proposed in OEP-50
109+
that is triggered when discussions settings change. This signal can encapsulate
110+
all the data needed for setting up discussions from the modulestore. It can
111+
traverse through all teh Units in the course that match the criterion from the
112+
dicussions settings and provide the needed details as part of the signal data.
113+
114+
A handler for the above signal, we create the discussion topics in
115+
`cs_comments_service` and add a mapping. If an existing unit with discussions
116+
is removed, we will disable the link but not delete the data.
117+
118+
The discussion grouping at subsections will simply combine the topics from all
119+
the units in the subsection and provide a unified view across the subsection.
120+
This setting will mainly be ignored and will likely only be used by the APIs
121+
or potentially the frontend directly.
122+
123+
The mapping between discussion ids and units is also a very simple model:
124+
125+
.. code-block:: python
126+
127+
class DiscussionTopicLink:
128+
course_key: CourseKey
129+
usage_key: UsageKey
130+
title: str
131+
group_id: int
132+
discussion_provider_id: str
133+
external_discussion_id: str
134+
enabled: bool
135+
136+
This structure is generic on purpose, to allow using this model for other
137+
providers in the future, and for switching providers without data loss.

0 commit comments

Comments
 (0)