Discussion service to enable permission and access provider by salman2013 · Pull Request #37912 · openedx/openedx-platform

salman2013 · 2026-01-20T17:44:20Z

Description

In edx-platform discussion 's specific permissions and discussion 's provider settings are managing with django models which are difficult to manage in discussion block extraction.
Ref: Discussion permissions
https://github.com/openedx/edx-platform/blob/5b1951228efca6afe0a3f18837236bd797cfadf4/lms/djangoapps/discussion/django_comment_client/permissions.py#L20

https://github.com/openedx/edx-platform/blob/5b1951228efca6afe0a3f18837236bd797cfadf4/openedx/core/djangoapps/django_comment_common/models.py#L163

Ref: DiscussionProvider
https://github.com/openedx/edx-platform/blob/5b1951228efca6afe0a3f18837236bd797cfadf4/openedx/core/djangoapps/discussions/models.py#L408

https://github.com/openedx/edx-platform/blob/5b1951228efca6afe0a3f18837236bd797cfadf4/openedx/core/djangoapps/discussions/models.py#L37

In this PR we created a service DiscussionConfig with the required methods to get the permissions and provider settings, added the service in runtime so that it can be easily accessible in xblocks-contrib.

Ticket: openedx/public-engineering#481

Acceptance Criteria

Discussion block UI and functionality should be functional.
All test should be green

How to test

Enable Discussion xblock

 tutor plugins install forum
 tutor plugins enable forum
 tutor dev do init -l forum

There are two types of provider (openedx, legacy) to load the discussion block. We can set these provider in Discussions/Discussions configurations in django admin.
Set the provider to legacy.
Add discussion xblock in studio from components.
Test the discussion xblock functionality and UI on LMS.

Testing Results

openedx-webhooks · 2026-01-20T17:44:25Z

Thanks for the pull request, @salman2013!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

farhan · 2026-01-21T13:11:28Z

openedx/core/djangoapps/discussions/services.py

+    Service for providing video-related configuration and feature flags.
+    """
+
+    def has_permission(self, user, permission, course_id=None):  # lint-amnesty, pylint: disable=missing-function-docstring


user_has_permission
seems better

@kdmccormick - as discussed in the meeting I will follow up with Dave on whether this should be in the UserService or stay here

@salman2013 After reading the code more myself, I think your approach is great. The has_permission check is not a general check--it is particular to discussions permissions. All the values of permission that you can pass in are like vote_for_comment, delete_comment`, and so on. So, I think it makes sense to keep this method in the new DiscussionConfigService.

Just two requests:

Please clarify in the docstring that this method only applies to discussion permissions, not permissions in general.

Please move the definition of has_permission from lms/djangoapps/discussion/django_comment_client/permissions.py over to openedx/core/djangoapps/django_comment_common/models.py. This way, the DiscussionConfigService can use the original definition without having to import from the lms code tree, which as you mentioned was causing an importlinter failure.

FYI @feanil

open-craft-grove · 2026-01-26T12:16:13Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2026-01-27T05:41:30Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

openedx/core/djangoapps/discussions/services.py

kdmccormick · 2026-01-28T18:07:29Z

openedx/core/djangoapps/discussions/services.py

+
+    def has_permission(self, user, permission, course_id=None):
+        """
+        Return the discussion permission for a user in a given course.


Suggested change

Return the discussion permission for a user in a given course.

Return whether the user has the given discussion permission for a given course.

We're not returning a permission, we're returning a boolean stating whether the user has the permission. Here's a suggested rewording.

kdmccormick · 2026-01-28T18:13:54Z

xmodule/discussion_block.py

 from xblocks_contrib.discussion import DiscussionXBlock as _ExtractedDiscussionXBlock

-from lms.djangoapps.discussion.django_comment_client.permissions import has_permission
+from openedx.core.djangoapps.django_comment_common.models import has_permission


Instead of importing has_permission here, could you update the discussion block to request the service and use its has_permission method? Same thing for the other two service methods you've extracted. That way, the discussion service code will be used and tested right away. It'll also make the discussion block extraction cleaner.

openedx/core/djangoapps/discussions/services.py

kdmccormick · 2026-01-28T18:17:29Z

cms/djangoapps/contentstore/views/preview.py

        "cache": CacheService(cache),
        'replace_urls': ReplaceURLService,
        'video_config': VideoConfigService(),
+        'discussion_config': DiscussionConfigService(),


Will this service be needed to edit courses/discussions in studio? If so, we'll need to add it to load_services_for_studio too. If not, then this is fine.

We do not need to add the service to load_services_for_studio, because the discussion view is only available in the LMS.

open-craft-grove · 2026-01-29T12:24:55Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2026-01-29T13:00:18Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2026-02-02T08:11:38Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2026-02-02T11:13:34Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

kdmccormick

Looks good @salman2013 !

Thanks for adding those type annotations. I've added these two small commits, ensuring that they are checked in CI:

add the file to mypy.ini so it's checked on every PR: 9299ed5
adds | None to course_id. Otherwise, mypy shows the error Incompatible default for argument "course_id" (default has type "None", argument has type "CourseKey"). c388130

openedx/core/djangoapps/xblock/runtime/runtime.py

xmodule/discussion_block.py

…ervice

The behavior of the MariaDB backend has changed behavior for UUIDField from a `CharField(32)` to an actual `uuid` type. This is not converted automatically, which results in all writes to the affected columns to error with a message about the data being too long. This is because the actual string being written is a UUID with the `-` included, resulting in a 36 character value which can't be inserted into a 32 character column. fix: renamed migration to 0016 Update cms/djangoapps/contentstore/migrations/0016_mariadb_uuid_conversion.py Co-authored-by: David Ormsbee <dave@axim.org> refactor: remove deprecated sidebar toggles (openedx#37983) DEPR ticket: openedx/public-engineering#316 feat: Add Waffle flag for AuthZ for Course Authoring (openedx#37985) build: Upgrade to `ora2==6.17.2` which removes loremipsum base dep (openedx#37991) We hope this will fix an error where loremipsum is using pkg_resources, which setuptools dropped support for as of v82 (released yesterday). fix: Nits on styles of library icon [FC-0114] (openedx#37980) - Fixes the issues described in openedx/frontend-app-authoring#2762 (comment): - Changed the background color for the library icon in the unit page. - Update punctuation for the library icon tooltip in the unit page. - Allows breaking the tooltip into multiple lines. refactor: xblock api upstream info and course details api (openedx#37971) - Returns top parent key instead of boolean in upstream info api - Adds edited_on raw time in course outline api - Adds has_changes to course details api feat: Add enable_authz_course_authoring flag to course_waffle_flags endpoint (openedx#37990) Discussion service to enable permission and access provider (openedx#37912) * chore: discussion service to enable permission and access provider

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 20, 2026

openedx-webhooks added this to Contributions Jan 20, 2026

github-project-automation bot moved this to Needs Triage in Contributions Jan 20, 2026

salman2013 mentioned this pull request Jan 21, 2026

Extracted Discussion XBlock: DiscussionsConfiguration and Permission openedx/public-engineering#481

Open

farhan reviewed Jan 21, 2026

View reviewed changes

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Jan 22, 2026

openedx-webhooks added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Jan 23, 2026

salman2013 added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jan 26, 2026

salman2013 changed the title ~~chore: discussion service to enable permission and access provider~~ Discussion service to enable permission and access provider Jan 26, 2026

salman2013 marked this pull request as ready for review January 26, 2026 13:23

salman2013 requested a review from irtazaakram as a code owner January 26, 2026 13:23

salman2013 requested review from farhan and kdmccormick January 28, 2026 05:51

kdmccormick requested changes Jan 28, 2026

View reviewed changes

salman2013 requested a review from kdmccormick February 2, 2026 10:52

salman2013 mentioned this pull request Feb 3, 2026

Allow tests for Extracted & Builtin Discussion XBlock #37873

Open

salman2013 added 7 commits February 5, 2026 16:50

chore: discussion service to enable permission and access provider

277b3e2

fix: fix quality tests

3182738

fix: fix quality tests

17ddb91

fix: fix quality tests

94ee18a

fix: fix quality tests

3fb05db

fix: fix quality tests

77d16e7

chore: move discussion permission method's place and use it in service

ec6291f

salman2013 added 8 commits February 5, 2026 16:50

fix: quality test

4ad30dc

fix: remove unecessory warning

892ba7e

fix: fix comments as received in review

f4746f0

fix: add type annotation for argument

5f085f6

fix: fix linting error

a64fc09

fix: service was not available in tests

56e561b

fix: tests fixes after using discussion service in discussion block

6d2b2a3

fix: revert commented code

b7ec54c

kdmccormick force-pushed the salman/discussion_service branch from 86706ff to 65b51ba Compare February 5, 2026 21:50

kdmccormick approved these changes Feb 5, 2026

View reviewed changes

kdmccormick added 2 commits February 5, 2026 16:53

build: Type-check the DiscussionsConfigService

9299ed5

build: Annotate course_id param to discussion has_permission as optional

c388130

kdmccormick force-pushed the salman/discussion_service branch from 65b51ba to c388130 Compare February 5, 2026 21:53

farhan requested changes Feb 6, 2026

View reviewed changes

openedx/core/djangoapps/xblock/runtime/runtime.py Show resolved Hide resolved

xmodule/discussion_block.py Outdated Show resolved Hide resolved

fix: use wants instead of need for discussion service

e0e6f33

salman2013 closed this Feb 9, 2026

github-project-automation bot moved this from Waiting on Author to Done in Contributions Feb 9, 2026

salman2013 reopened this Feb 9, 2026

salman2013 closed this Feb 10, 2026

salman2013 reopened this Feb 10, 2026

Merge remote-tracking branch 'origin/master' into salman/discussion_s…

58b0303

…ervice

salman2013 requested a review from farhan February 10, 2026 10:30

farhan approved these changes Feb 11, 2026

View reviewed changes

salman2013 merged commit d20b87b into master Feb 11, 2026
69 of 70 checks passed

salman2013 deleted the salman/discussion_service branch February 11, 2026 14:37

	Return the discussion permission for a user in a given course.
	Return whether the user has the given discussion permission for a given course.

Conversation

salman2013 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Acceptance Criteria

How to test

Testing Results

Uh oh!

openedx-webhooks commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

farhan Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

open-craft-grove commented Jan 26, 2026

Uh oh!

open-craft-grove commented Jan 27, 2026

Uh oh!

Uh oh!

kdmccormick Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kdmccormick Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

salman2013 Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

open-craft-grove commented Jan 29, 2026

Uh oh!

open-craft-grove commented Jan 29, 2026

Uh oh!

open-craft-grove commented Feb 2, 2026

Uh oh!

open-craft-grove commented Feb 2, 2026

Uh oh!

kdmccormick left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

salman2013 commented Jan 20, 2026 •

edited

Loading

openedx-webhooks commented Jan 20, 2026 •

edited

Loading

kdmccormick left a comment •

edited

Loading