Skip to content

feat: Adds discussions settings for new discusions experience [BD-38] [TNL-8621] [BB-4854] - #29131

Merged
asadazam93 merged 2 commits into
openedx:masterfrom
open-craft:kshiitj/discussions-structure-in-course
Oct 28, 2021
Merged

feat: Adds discussions settings for new discusions experience [BD-38] [TNL-8621] [BB-4854]#29131
asadazam93 merged 2 commits into
openedx:masterfrom
open-craft:kshiitj/discussions-structure-in-course

Conversation

@xitij2000

@xitij2000 xitij2000 commented Oct 27, 2021

Copy link
Copy Markdown
Contributor

Description

This commit adds new discussions settings for the new discussions experience. These are stored in the course so they can be a part of course import/export flow.
These are also added to the discussions configuration API to allow MFEs to update the settings.

Previously reverted due to a test failure.

Supporting information

Testing instructions

  1. Use the updates course authoring MFE from here: fix: Update the API path and fix saving settings [BD-38] [TNL-8621] [BB-4854] frontend-app-authoring#197
  2. Change the legacy provider settings for anonymous posting settings, and make sure they are persisted
  3. The settings should be saved to the coruse structure as well which you can verify using the shell

Deadline

"None"

Related PR: https://github.com/edx/edx-platform/pull/28749

This commit adds new discussions settings for the new discussions experience. These are stored in the course so they can be a part of course import/export flow.
These are also added to the discussions configuraiton API to allow MFEs to update the settings.
The discussions API is currently available via LMS, however that means it cannot save changes to the modulestore. This also adds the API to the studio config so it can now also be accessed from studio and be used to save course settings.
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program needs triage labels Oct 27, 2021
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @xitij2000! I've created BLENDED-998 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.

Comment on lines +22 to +26
self.discussion_config = DiscussionsConfiguration.objects.create(
context_key=self.course.id,
enabled=False,
provider_type="lti_provider",
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for deciding when the LTI tab is enabled was changed in this PR: https://github.com/edx/edx-platform/pull/29110

This PR updates the default provider type such that LTI support is disabled by default, and as such the test failes since it didn't take this condition into account.

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@awaisdar001 awaisdar001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 👍

@asadazam93

Copy link
Copy Markdown
Contributor

@xitij2000 Is this ready to be merged?

@xitij2000

Copy link
Copy Markdown
Contributor Author

@xitij2000 Is this ready to be merged?

If the only reason it was reverted was the failing test, then yes.

@asadazam93
asadazam93 merged commit 79cd0b1 into openedx:master Oct 28, 2021
@openedx-webhooks

Copy link
Copy Markdown

@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-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@xitij2000
xitij2000 deleted the kshiitj/discussions-structure-in-course branch November 2, 2021 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blended PR is managed through 2U's blended developmnt program merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants