docs: ADR that documents how MFEs can access in-context discussions [BD-38] [TNL-8396] [BB-4367] #28452
Conversation
openedx-webhooks
added
blended
|
Thanks for the pull request, @xitij2000! I've created BLENDED-939 to keep track of it in Jira. More details are on the BD-38 project page. When this pull request is ready, tag your edX technical lead. |
arjunsinghy96
left a comment
arjunsinghy96
left a comment
There was a problem hiding this comment.
👍 @xitij2000 LGTM. I have suggested a couple of minor nits. Could you please look at them before moving forward?
-
I tested this: (describe what you tested) - I read through the code
-
I checked for accessibility issues - Includes documentation
-
I made sure any change in configuration variables is reflected in the corresponding client'sconfiguration-securerepository.
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
xitij2000
force-pushed
the
kshitij/tnl-8396/discussions-course-blocks
branch
3 times, most recently
from
6113bf3 to
d3a1f89
Compare
This ADR proposes the mechanism for MFEs to get access to the discussion embed for units.
xitij2000
force-pushed
the
kshitij/tnl-8396/discussions-course-blocks
branch
from
d3a1f89 to
cda4d0e
Compare
|
@awaisdar001 I have updated the ADR. Please have another look. |
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
openedx/core/djangoapps/discussions/docs/decisions/0005-in-context-discussion-course-blocks.rst
Outdated
Show resolved
Hide resolved
| For units that don't have a linked discussion, no link will be returned. | ||
|
|
||
| The new discussions experience includes a setting called | ||
| `discussions_group_at_subsection` to group discussions at the subsection |
There was a problem hiding this comment.
Is the discussion MFE is the provider of this setting? How this setting is turned on or off?
There was a problem hiding this comment.
I think the previous ADR is good reference for this.
It's a setting stored in the course structure and will be configurable using an API, and using the course authoring MFE. You can see these in the figma mockup.
| The new discussions experience includes a setting called | ||
| `discussions_group_at_subsection` to group discussions at the subsection | ||
| level instead of the unit level. Normally the sidebar next to a unit will | ||
| only show threads from that unit. However, if this setting is enabled |
There was a problem hiding this comment.
if this setting is enabled => Yes - Same question, how this setting is enabled/disabled?
There was a problem hiding this comment.
|
|
||
| The new discussions experience includes a setting called | ||
| `discussions_group_at_subsection` to group discussions at the subsection | ||
| level instead of the unit level. Normally the sidebar next to a unit will |
There was a problem hiding this comment.
by Normally do you mean when the discussions_group_at_subsection setting is off/disabled?
| Decision | ||
| -------- | ||
|
|
||
| A direct link to the topic that needs to be embedded can be generated |
There was a problem hiding this comment.
You said that "a direct link to the topic" will be provided by edx-platform using the course blocks API, but when you access the API using requested_fields=discussions_embed_url, it returns an object with different kinds of URLs.
Can specify which is the embeddable URL among those when you call it "a direct link"?
There was a problem hiding this comment.
This is just the standard default response of the course blocks API. It returns some standard fields, and other fields can be requested in addition as needed.
In this case, the idea is to add a new field option as specified above and have that add a new return value called "discussions_embed_url". I've cleaned up some the other URLs so it's easier to focus on this new field.
|
Your PR has finished running tests. There were no failures. |
|
@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
7 participants
This ADR proposes the mechanism for MFEs to get access to the discussion embed for units.
Depends/builds on: https://github.com/edx/edx-platform/pull/28221