Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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
Expand Down Expand Up @@ -217,6 +218,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)
Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/courseware/block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

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
Expand Down Expand Up @@ -637,6 +638,7 @@ def inner_get_block(block: XBlock) -> XBlock | None:
'publish': EventPublishingService(user, course_id, track_function),
'enrollments': EnrollmentsService(),
'video_config': VideoConfigService(),
'discussion_config_service': DiscussionConfigService(),
}

runtime.get_block_for_descriptor = inner_get_block
Expand Down
42 changes: 28 additions & 14 deletions lms/djangoapps/courseware/tests/test_discussion_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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


Expand Down Expand Up @@ -193,6 +194,14 @@ def test_student_perms_are_correct(self, permissions):
'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()
Expand Down Expand Up @@ -223,13 +232,10 @@ def test_has_permission(self):
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."""
Expand All @@ -252,6 +258,14 @@ def test_student_perms_are_correct(self, permissions):
'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'
Expand Down Expand Up @@ -296,7 +310,7 @@ def get_root(self, block):
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.
Expand All @@ -317,7 +331,7 @@ def test_html_with_user(self):
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
Expand Down Expand Up @@ -393,11 +407,11 @@ def test_discussion_xblock_visibility(self):
"""
# Enable new OPEN_EDX provider for this course
course_key = self.course.location.course_key
DiscussionsConfiguration.objects.create(
context_key=course_key,
enabled=True,
provider_type=Provider.OPEN_EDX,
)
# DiscussionsConfiguration.objects.create(
# context_key=course_key,
# enabled=True,
# provider_type=Provider.OPEN_EDX,
# )

discussion_xblock = get_block_for_descriptor(
user=self.user,
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/discussion/django_comment_client/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

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
Expand All @@ -33,8 +34,7 @@
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,
Expand Down
17 changes: 1 addition & 16 deletions lms/djangoapps/discussion/django_comment_client/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,11 @@
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']


Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/discussion/django_comment_client/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
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
%>

Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/discussion/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

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
Expand All @@ -37,7 +38,6 @@
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,
Expand Down
38 changes: 38 additions & 0 deletions openedx/core/djangoapps/discussions/services.py
Original file line number Diff line number Diff line change
@@ -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:
Copy link
Member

Choose a reason for hiding this comment

The 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: User, permission: str, course_id: CourseKey = 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
33 changes: 33 additions & 0 deletions openedx/core/djangoapps/django_comment_common/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
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
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,9 @@ def service(self, block: XBlock, service_name: str):
# 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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
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
Expand Down
Loading
Loading