-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Discussion service to enable permission and access provider #37912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 9 commits
3c5abde
45239cd
459406c
be10fd9
5a7e796
fb78b42
a461a75
f6d10e5
6718701
4c57989
d381093
e82a3b2
2514434
780129d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||
| """ | ||||||
| Discussion Configuration Service for XBlock runtime. | ||||||
|
|
||||||
| This service provides discussion-related configuration and feature flags | ||||||
| that are specific to the edx-platform implementation | ||||||
| for the extracted discussion block in xblocks-contrib repository. | ||||||
| """ | ||||||
|
|
||||||
| from django.conf import settings | ||||||
| from openedx.core.djangoapps.django_comment_common.models import has_permission | ||||||
| from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider | ||||||
|
|
||||||
|
|
||||||
| class DiscussionConfigService: | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method bodies look good. Could you add type annotations to all the args and return values? Check out the new VideoConfigService for an example. |
||||||
| """ | ||||||
| Service for providing discussion-related configuration and feature flags. | ||||||
| """ | ||||||
|
|
||||||
| def has_permission(self, user, permission, course_id=None): | ||||||
| """ | ||||||
| Return the discussion permission for a user in a given course. | ||||||
|
||||||
| 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.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') | |
| return settings.ENABLE_DISCUSSION_SERVICE |
FEATURES items can now be accessed directly on the settings object. This is preferrable because you will get an exception or linter warning if the setting is misspelled or later removed
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |
| from xblock.utils.studio_editable import StudioEditableXBlockMixin | ||
| 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 | ||
|
||
| from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider | ||
| from openedx.core.djangolib.markup import HTML, Text | ||
| from openedx.core.lib.xblock_utils import get_css_dependencies, get_js_dependencies | ||
|
|
@@ -282,8 +282,17 @@ def _apply_metadata_and_policy(cls, block, node, runtime): | |
| setattr(block, field_name, value) | ||
|
|
||
|
|
||
| DiscussionXBlock = ( | ||
| _ExtractedDiscussionXBlock if settings.USE_EXTRACTED_DISCUSSION_BLOCK | ||
| else _BuiltInDiscussionXBlock | ||
| ) | ||
| DiscussionXBlock = None | ||
|
|
||
|
|
||
| def reset_class(): | ||
| """Reset class as per django settings flag""" | ||
| global DiscussionXBlock | ||
| DiscussionXBlock = ( | ||
| _ExtractedDiscussionXBlock if settings.USE_EXTRACTED_DISCUSSION_BLOCK | ||
| else _BuiltInDiscussionXBlock | ||
| ) | ||
| return DiscussionXBlock | ||
|
|
||
| reset_class() | ||
| DiscussionXBlock.__name__ = "DiscussionXBlock" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.