Discussion service to enable permission and access provider (#37912)
* chore: discussion service to enable permission and access provider
This commit is contained in:
@@ -19,6 +19,7 @@ from xblock.exceptions import NoSuchHandlerError, NotFoundError, ProcessingError
|
||||
from xblock.runtime import KvsFieldData
|
||||
|
||||
from openedx.core.djangoapps.video_config.services import VideoConfigService
|
||||
from openedx.core.djangoapps.discussions.services import DiscussionConfigService
|
||||
from xmodule.contentstore.django import contentstore
|
||||
from xmodule.exceptions import NotFoundError as XModuleNotFoundError
|
||||
from xmodule.modulestore.django import XBlockI18nService, modulestore
|
||||
@@ -228,6 +229,7 @@ def _prepare_runtime_for_preview(request, block):
|
||||
"cache": CacheService(cache),
|
||||
'replace_urls': ReplaceURLService,
|
||||
'video_config': VideoConfigService(),
|
||||
'discussion_config_service': DiscussionConfigService(),
|
||||
}
|
||||
|
||||
block.runtime.get_block_for_descriptor = partial(_load_preview_block, request)
|
||||
|
||||
@@ -43,6 +43,7 @@ from xblock.runtime import KvsFieldData
|
||||
|
||||
from lms.djangoapps.teams.services import TeamsService
|
||||
from openedx.core.djangoapps.video_config.services import VideoConfigService
|
||||
from openedx.core.djangoapps.discussions.services import DiscussionConfigService
|
||||
from openedx.core.lib.xblock_services.call_to_action import CallToActionService
|
||||
from xmodule.contentstore.django import contentstore
|
||||
from xmodule.exceptions import NotFoundError as XModuleNotFoundError
|
||||
@@ -637,6 +638,7 @@ def prepare_runtime_for_user(
|
||||
'publish': EventPublishingService(user, course_id, track_function),
|
||||
'enrollments': EnrollmentsService(),
|
||||
'video_config': VideoConfigService(),
|
||||
'discussion_config_service': DiscussionConfigService(),
|
||||
}
|
||||
|
||||
runtime.get_block_for_descriptor = inner_get_block
|
||||
|
||||
@@ -28,6 +28,7 @@ from lms.djangoapps.course_api.blocks.tests.helpers import deserialize_usage_key
|
||||
from lms.djangoapps.courseware.block_render import get_block_for_descriptor
|
||||
from lms.djangoapps.courseware.tests.helpers import XModuleRenderingTestBase
|
||||
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
|
||||
from openedx.core.djangoapps.discussions.services import DiscussionConfigService
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
|
||||
|
||||
|
||||
@@ -193,6 +194,14 @@ class TestViews(TestDiscussionXBlock):
|
||||
'can_create_subcomment': permission_dict['create_sub_comment'],
|
||||
}
|
||||
|
||||
self.add_patcher(
|
||||
patch.multiple(
|
||||
DiscussionConfigService,
|
||||
is_discussion_visible=mock.Mock(return_value=True),
|
||||
is_discussion_enabled=mock.Mock(return_value=True)
|
||||
)
|
||||
)
|
||||
|
||||
self.block.has_permission = lambda perm: permission_dict[perm]
|
||||
with mock.patch('xmodule.discussion_block.render_to_string', return_value='') as mock_render:
|
||||
self.block.student_view()
|
||||
@@ -223,13 +232,10 @@ class TestTemplates(TestDiscussionXBlock):
|
||||
Test for has_permission method.
|
||||
"""
|
||||
permission_canary = object()
|
||||
with mock.patch(
|
||||
'xmodule.discussion_block.has_permission',
|
||||
return_value=permission_canary,
|
||||
) as has_perm:
|
||||
actual_permission = self.block.has_permission("test_permission")
|
||||
self.block.has_permission = mock.Mock(return_value=permission_canary)
|
||||
actual_permission = self.block.has_permission("test_permission")
|
||||
assert actual_permission == permission_canary
|
||||
has_perm.assert_called_once_with(self.django_user_canary, 'test_permission', self.course_id)
|
||||
self.block.has_permission.assert_called_once_with("test_permission")
|
||||
|
||||
def test_studio_view(self):
|
||||
"""Test for studio view."""
|
||||
@@ -252,6 +258,14 @@ class TestTemplates(TestDiscussionXBlock):
|
||||
'create_sub_comment': permissions[2]
|
||||
}
|
||||
|
||||
self.add_patcher(
|
||||
patch.multiple(
|
||||
DiscussionConfigService,
|
||||
is_discussion_visible=mock.Mock(return_value=True),
|
||||
is_discussion_enabled=mock.Mock(return_value=True)
|
||||
)
|
||||
)
|
||||
|
||||
self.block.has_permission = lambda perm: permission_dict[perm]
|
||||
fragment = self.block.student_view()
|
||||
read_only = 'false' if permissions[0] else 'true'
|
||||
@@ -296,7 +310,7 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
|
||||
block = block.get_parent()
|
||||
return block
|
||||
|
||||
@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
|
||||
@override_settings(ENABLE_DISCUSSION_SERVICE=True)
|
||||
def test_html_with_user(self):
|
||||
"""
|
||||
Test rendered DiscussionXBlock permissions.
|
||||
@@ -317,7 +331,7 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
|
||||
assert 'data-user-create-comment="false"' in html
|
||||
assert 'data-user-create-subcomment="false"' in html
|
||||
|
||||
@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
|
||||
@override_settings(ENABLE_DISCUSSION_SERVICE=True)
|
||||
def test_discussion_render_successfully_with_orphan_parent(self):
|
||||
"""
|
||||
Test that discussion xblock render successfully
|
||||
@@ -421,7 +435,7 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase):
|
||||
Test the number of queries executed when rendering the XBlock.
|
||||
"""
|
||||
|
||||
@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
|
||||
@override_settings(ENABLE_DISCUSSION_SERVICE=True)
|
||||
def test_permissions_query_load(self):
|
||||
"""
|
||||
Tests that the permissions queries are cached when rendering numerous discussion XBlocks.
|
||||
|
||||
@@ -25,6 +25,7 @@ from openedx_events.learning.signals import (
|
||||
|
||||
import lms.djangoapps.discussion.django_comment_client.settings as cc_settings
|
||||
import openedx.core.djangoapps.django_comment_common.comment_client as cc
|
||||
from openedx.core.djangoapps.django_comment_common.models import has_permission
|
||||
from common.djangoapps.student.roles import GlobalStaff
|
||||
from common.djangoapps.track import contexts
|
||||
from common.djangoapps.util.file import store_uploaded_file
|
||||
@@ -33,8 +34,7 @@ from lms.djangoapps.courseware.courses import get_course_overview_with_access, g
|
||||
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
|
||||
from lms.djangoapps.discussion.django_comment_client.permissions import (
|
||||
check_permissions_by_view,
|
||||
get_team,
|
||||
has_permission
|
||||
get_team
|
||||
)
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import (
|
||||
JsonError,
|
||||
|
||||
@@ -12,26 +12,11 @@ from lms.djangoapps.teams.models import CourseTeam
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client import Thread
|
||||
from openedx.core.djangoapps.django_comment_common.models import (
|
||||
CourseDiscussionSettings,
|
||||
all_permissions_for_user_in_course
|
||||
has_permission
|
||||
)
|
||||
from openedx.core.lib.cache_utils import request_cached
|
||||
|
||||
|
||||
def has_permission(user, permission, course_id=None): # lint-amnesty, pylint: disable=missing-function-docstring
|
||||
assert isinstance(course_id, (type(None), CourseKey))
|
||||
request_cache_dict = DEFAULT_REQUEST_CACHE.data
|
||||
cache_key = "django_comment_client.permissions.has_permission.all_permissions.{}.{}".format(
|
||||
user.id, course_id
|
||||
)
|
||||
if cache_key in request_cache_dict:
|
||||
all_permissions = request_cache_dict[cache_key]
|
||||
else:
|
||||
all_permissions = all_permissions_for_user_in_course(user, course_id)
|
||||
request_cache_dict[cache_key] = all_permissions
|
||||
|
||||
return permission in all_permissions
|
||||
|
||||
|
||||
CONDITIONS = ['is_open', 'is_author', 'is_question_author', 'is_team_member_if_applicable']
|
||||
|
||||
|
||||
|
||||
@@ -24,10 +24,10 @@ from lms.djangoapps.courseware.access import has_access
|
||||
from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY
|
||||
from lms.djangoapps.discussion.django_comment_client.permissions import (
|
||||
check_permissions_by_view,
|
||||
get_team,
|
||||
has_permission
|
||||
get_team
|
||||
)
|
||||
from lms.djangoapps.discussion.django_comment_client.settings import MAX_COMMENT_DEPTH
|
||||
from openedx.core.djangoapps.django_comment_common.models import has_permission
|
||||
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id
|
||||
from openedx.core.djangoapps.discussions.utils import (
|
||||
get_accessible_discussion_xblocks,
|
||||
|
||||
@@ -11,7 +11,7 @@ from django.utils.translation import gettext as _, ngettext
|
||||
from django.template.defaultfilters import escapejs
|
||||
from django.urls import reverse
|
||||
|
||||
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.djangolib.js_utils import dump_js_escaped_json, js_escaped_string
|
||||
%>
|
||||
|
||||
|
||||
@@ -28,6 +28,7 @@ from xmodule.modulestore.django import modulestore
|
||||
|
||||
import lms.djangoapps.discussion.django_comment_client.utils as utils
|
||||
import openedx.core.djangoapps.django_comment_common.comment_client as cc
|
||||
from openedx.core.djangoapps.django_comment_common.models import has_permission
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
|
||||
from common.djangoapps.util.json_request import JsonResponse, expect_json
|
||||
@@ -37,7 +38,6 @@ from lms.djangoapps.courseware.views.views import CourseTabView
|
||||
from lms.djangoapps.discussion.config.settings import is_forum_daily_digest_enabled
|
||||
from lms.djangoapps.discussion.django_comment_client.base.views import track_thread_viewed_event
|
||||
from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY
|
||||
from lms.djangoapps.discussion.django_comment_client.permissions import has_permission
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import (
|
||||
add_courseware_context,
|
||||
course_discussion_division_enabled,
|
||||
|
||||
1
mypy.ini
1
mypy.ini
@@ -15,6 +15,7 @@ files =
|
||||
# openedx/core/djangoapps/content/search,
|
||||
openedx/core/djangoapps/content_staging,
|
||||
openedx/core/djangoapps/content_libraries,
|
||||
openedx/core/djangoapps/discussions/services.py,
|
||||
openedx/core/djangoapps/programs/rest_api,
|
||||
openedx/core/djangoapps/xblock,
|
||||
openedx/core/lib/derived.py,
|
||||
|
||||
38
openedx/core/djangoapps/discussions/services.py
Normal file
38
openedx/core/djangoapps/discussions/services.py
Normal file
@@ -0,0 +1,38 @@
|
||||
"""
|
||||
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 django.contrib.auth.models import User # pylint: disable=imported-auth-user
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from openedx.core.djangoapps.django_comment_common.models import has_permission
|
||||
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
|
||||
|
||||
|
||||
class DiscussionConfigService:
|
||||
"""
|
||||
Service for providing discussion-related configuration and feature flags.
|
||||
"""
|
||||
|
||||
def has_permission(self, user: User, permission: str, course_id: CourseKey | None = None) -> bool:
|
||||
"""
|
||||
Return whether the user has the given discussion permission for a given course.
|
||||
"""
|
||||
return has_permission(user, permission, course_id)
|
||||
|
||||
def is_discussion_visible(self, course_key: CourseKey) -> bool:
|
||||
"""
|
||||
Discussion Xblock does not support new OPEN_EDX provider
|
||||
"""
|
||||
provider = DiscussionsConfiguration.get(course_key)
|
||||
return provider.provider_type == Provider.LEGACY
|
||||
|
||||
def is_discussion_enabled(self) -> bool:
|
||||
"""
|
||||
Return True if discussions are enabled; else False
|
||||
"""
|
||||
return settings.ENABLE_DISCUSSION_SERVICE
|
||||
@@ -14,6 +14,8 @@ from django.dispatch import receiver
|
||||
from django.utils.translation import gettext_noop
|
||||
from jsonfield.fields import JSONField
|
||||
from opaque_keys.edx.django.models import CourseKeyField
|
||||
from edx_django_utils.cache import DEFAULT_REQUEST_CACHE
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager
|
||||
from openedx.core.lib.cache_utils import request_cached
|
||||
@@ -193,6 +195,37 @@ def all_permissions_for_user_in_course(user, course_id):
|
||||
return permission_names
|
||||
|
||||
|
||||
def has_permission(user, permission, course_id=None):
|
||||
"""
|
||||
This function resolves all discussion-related permissions for the given
|
||||
user and course, caches them for the duration of the request, and verifies
|
||||
whether the requested permission is present.
|
||||
|
||||
Args:
|
||||
user (User): Django user whose permissions are being checked.
|
||||
permission (str): Discussion permission identifier
|
||||
(e.g., "create_comment", "create_thread").
|
||||
course_id (CourseKey): Course context in which to evaluate
|
||||
the permission
|
||||
|
||||
Returns:
|
||||
bool: True if the user has the specified permission in the given
|
||||
course context; False otherwise.
|
||||
"""
|
||||
assert isinstance(course_id, (type(None), CourseKey))
|
||||
request_cache_dict = DEFAULT_REQUEST_CACHE.data
|
||||
cache_key = "django_comment_client.permissions.has_permission.all_permissions.{}.{}".format(
|
||||
user.id, course_id
|
||||
)
|
||||
if cache_key in request_cache_dict:
|
||||
all_permissions = request_cache_dict[cache_key]
|
||||
else:
|
||||
all_permissions = all_permissions_for_user_in_course(user, course_id)
|
||||
request_cache_dict[cache_key] = all_permissions
|
||||
|
||||
return permission in all_permissions
|
||||
|
||||
|
||||
class ForumsConfig(ConfigurationModel):
|
||||
"""
|
||||
Config for the connection to the cs_comments_service forums backend.
|
||||
|
||||
@@ -347,6 +347,9 @@ class XBlockRuntime(RuntimeShim, Runtime):
|
||||
# Import here to avoid circular dependency
|
||||
from openedx.core.djangoapps.video_config.services import VideoConfigService
|
||||
return VideoConfigService()
|
||||
elif service_name == 'discussion_config_service':
|
||||
from openedx.core.djangoapps.discussions.services import DiscussionConfigService
|
||||
return DiscussionConfigService()
|
||||
|
||||
# Otherwise, fall back to the base implementation which loads services
|
||||
# defined in the constructor:
|
||||
|
||||
@@ -15,7 +15,7 @@ import json
|
||||
from django.utils.translation import gettext as _
|
||||
from django.template.defaultfilters import escapejs
|
||||
|
||||
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.djangolib.js_utils import dump_js_escaped_json, js_escaped_string
|
||||
from openedx.core.djangolib.markup import HTML
|
||||
from openedx.features.course_experience import course_home_page_title
|
||||
|
||||
@@ -17,8 +17,6 @@ from xblock.utils.resources import ResourceLoader
|
||||
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.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
|
||||
from xmodule.xml_block import XmlMixin
|
||||
@@ -37,6 +35,7 @@ def _(text):
|
||||
@XBlock.needs('user') # pylint: disable=abstract-method
|
||||
@XBlock.needs('i18n')
|
||||
@XBlock.needs('mako')
|
||||
@XBlock.wants('discussion_config_service')
|
||||
class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
XmlMixin): # lint-amnesty, pylint: disable=abstract-method
|
||||
"""
|
||||
@@ -76,6 +75,13 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
|
||||
has_author_view = True # Tells Studio to use author_view
|
||||
|
||||
@property
|
||||
def discussion_config_service(self):
|
||||
"""
|
||||
Returns discussion configuration service.
|
||||
"""
|
||||
return self.runtime.service(self, 'discussion_config_service')
|
||||
|
||||
@property
|
||||
def course_key(self):
|
||||
return getattr(self.scope_ids.usage_id, 'course_key', None)
|
||||
@@ -85,8 +91,18 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
"""
|
||||
Discussion Xblock does not support new OPEN_EDX provider
|
||||
"""
|
||||
provider = DiscussionsConfiguration.get(self.course_key)
|
||||
return provider.provider_type == Provider.LEGACY
|
||||
if self.discussion_config_service:
|
||||
return self.discussion_config_service.is_discussion_visible(self.course_key)
|
||||
return False
|
||||
|
||||
@property
|
||||
def is_discussion_enabled(self):
|
||||
"""
|
||||
Returns True if discussions are enabled; else False
|
||||
"""
|
||||
if self.discussion_config_service:
|
||||
return self.discussion_config_service.is_discussion_enabled()
|
||||
return False
|
||||
|
||||
@property
|
||||
def django_user(self):
|
||||
@@ -159,15 +175,14 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
:param str permission: Permission
|
||||
:rtype: bool
|
||||
"""
|
||||
return has_permission(self.django_user, permission, self.course_key)
|
||||
if self.discussion_config_service:
|
||||
return self.discussion_config_service.has_permission(self.django_user, permission, self.course_key)
|
||||
return False
|
||||
|
||||
def student_view(self, context=None):
|
||||
"""
|
||||
Renders student view for LMS.
|
||||
"""
|
||||
# to prevent a circular import issue
|
||||
import lms.djangoapps.discussion.django_comment_client.utils as utils
|
||||
|
||||
fragment = Fragment()
|
||||
|
||||
if not self.is_visible:
|
||||
@@ -193,7 +208,7 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
url='{}?{}'.format(reverse('register_user'), qs),
|
||||
),
|
||||
)
|
||||
if utils.is_discussion_enabled(self.course_key):
|
||||
if self.is_discussion_enabled:
|
||||
context = {
|
||||
'discussion_id': self.discussion_id,
|
||||
'display_name': self.display_name if self.display_name else _("Discussion"),
|
||||
@@ -282,8 +297,17 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin,
|
||||
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"
|
||||
|
||||
@@ -33,7 +33,7 @@ from xmodule.util.sandboxing import SandboxService
|
||||
from xmodule.x_module import DoNothingCache, XModuleMixin, ModuleStoreRuntime
|
||||
from openedx.core.djangoapps.video_config.services import VideoConfigService
|
||||
from openedx.core.lib.cache_utils import CacheService
|
||||
|
||||
from openedx.core.djangoapps.discussions.services import DiscussionConfigService
|
||||
|
||||
MODULE_DIR = path(__file__).dirname()
|
||||
# Location of common test DATA directory
|
||||
@@ -161,6 +161,7 @@ def get_test_system(
|
||||
'field-data': DictFieldData({}),
|
||||
'sandbox': SandboxService(contentstore, course_id),
|
||||
'video_config': VideoConfigService(),
|
||||
'discussion_config_service': DiscussionConfigService()
|
||||
}
|
||||
|
||||
descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
@@ -217,6 +218,7 @@ def prepare_block_runtime(
|
||||
'field-data': DictFieldData({}),
|
||||
'sandbox': SandboxService(contentstore, course_id),
|
||||
'video_config': VideoConfigService(),
|
||||
'discussion_config_service': DiscussionConfigService()
|
||||
}
|
||||
|
||||
if add_overrides:
|
||||
|
||||
Reference in New Issue
Block a user