diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index e4f31db273d3..343ffe31ff71 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -2016,3 +2016,58 @@ def add_stats_for_users_with_null_values(course_stats, users_in_course): }) updated_course_stats = sorted(updated_course_stats, key=lambda d: len(d['username'])) return updated_course_stats + +def _get_user_label_function(course_id, course_staff_user_ids, moderator_user_ids, ta_user_ids): + """ + Create and return a function that determines user labels based on role. + + Args: + course_id: Course key/id used for discussion Role lookups + course_staff_user_ids: List of user IDs for course staff + moderator_user_ids: List of user IDs for moderators + ta_user_ids: List of user IDs for TAs + + Returns: + A function that takes a user_id and returns the appropriate label or None + """ + + # Pre-fetch discussion role names for all relevant users to avoid per-user queries. + relevant_user_ids = set(moderator_user_ids) | set(ta_user_ids) + role_names_by_user_id = {} + if relevant_user_ids: + for user_id_val, role_name in Role.objects.filter( + course_id=course_id, + users__id__in=relevant_user_ids, + name__in=[ + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, + ], + ).values_list('users__id', 'name'): + role_names_by_user_id.setdefault(int(user_id_val), set()).add(role_name) + + def get_user_label(user_id): + """Get role label for a user ID.""" + try: + user_id_int = int(user_id) + # Platform course roles (collapsed to "Course Staff" for deleted content lists) + if user_id_int in course_staff_user_ids: + return "Course Staff" + + # Discussion-specific roles (distinguish admin vs moderator) + role_names = role_names_by_user_id.get(user_id_int, set()) + if FORUM_ROLE_ADMINISTRATOR in role_names: + return "Discussion Administrator" + if FORUM_ROLE_MODERATOR in role_names: + return "Moderator" + if FORUM_ROLE_COMMUNITY_TA in role_names: + return "Community TA" + if FORUM_ROLE_GROUP_MODERATOR in role_names: + return "Group Moderator" + except (ValueError, TypeError): + # If user_id has any issues, there's no label to return + pass + return None + + return get_user_label \ No newline at end of file diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 0c3f5765493d..03a72b54207a 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -16,7 +16,7 @@ from rest_framework import serializers from common.djangoapps.student.models import get_user_by_username_or_email -from common.djangoapps.student.roles import GlobalStaff +from common.djangoapps.student.roles import GlobalStaff, CourseStaffRole, CourseInstructorRole from lms.djangoapps.discussion.django_comment_client.base.views import ( track_comment_edited_event, track_forum_response_mark_event, @@ -45,9 +45,20 @@ from openedx.core.djangoapps.discussions.utils import get_group_names_by_id from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread -from openedx.core.djangoapps.django_comment_common.comment_client.user import User as CommentClientUser -from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError -from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings +from openedx.core.djangoapps.django_comment_common.comment_client.user import ( + User as CommentClientUser, +) +from openedx.core.djangoapps.django_comment_common.comment_client.utils import ( + CommentClientRequestError, +) +from openedx.core.djangoapps.django_comment_common.models import ( + CourseDiscussionSettings, + Role, + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, +) from openedx.core.djangoapps.user_api.accounts.api import get_profile_images from openedx.core.lib.api.serializers import CourseKeyField @@ -185,6 +196,8 @@ class _ContentSerializer(serializers.Serializer): id = serializers.CharField(read_only=True) # pylint: disable=invalid-name author = serializers.SerializerMethodField() author_label = serializers.SerializerMethodField() + author_labels = serializers.SerializerMethodField() + learner_status = serializers.SerializerMethodField() created_at = serializers.CharField(read_only=True) updated_at = serializers.CharField(read_only=True) raw_body = serializers.CharField(source="body", validators=[validate_not_blank]) @@ -243,20 +256,119 @@ def get_author(self, obj): def _get_user_label(self, user_id): """ - Returns the role label (i.e. "Staff", "Moderator" or "Community TA") for the user - with the given id. + Returns a single legacy role label for the user. + Used by edit_by_label, closed_by_label, endorsed_by_label, deleted_by_label + to preserve backward compatibility. + Returns one of: "Staff", "Administrator", "Moderator", "Community TA", or None. """ is_staff = user_id in self.context["course_staff_user_ids"] is_moderator = user_id in self.context["moderator_user_ids"] is_ta = user_id in self.context["ta_user_ids"] + is_global_staff = False + if not (is_moderator or is_ta): + try: + user = User.objects.get(id=user_id) + is_global_staff = GlobalStaff().has_user(user) + except User.DoesNotExist: + pass + + is_administrator = False + if is_moderator: + course_id = self.context.get("course_id") + if course_id: + user_roles = Role.objects.filter( + users__id=user_id, + course_id=course_id, + name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR], + ).values_list("name", flat=True) + is_administrator = FORUM_ROLE_ADMINISTRATOR in user_roles + return ( - "Staff" if is_staff else - "Moderator" if is_moderator else - "Community TA" if is_ta else - None + "Staff" + if is_global_staff + else "Administrator" + if is_administrator + else "Moderator" if is_moderator else "Community TA" if is_ta else None ) + def _get_user_labels_all(self, user_id): + """ + Returns an array of ALL roles assigned to the user. + Used exclusively by get_author_labels to support multi-role display. + Examples: ["Global Staff", "Course Staff"], ["Administrator", "Community TA"] + """ + roles = [] + + # Check GlobalStaff (platform-wide) + try: + user = User.objects.get(id=user_id) + if GlobalStaff().has_user(user): + roles.append("Global Staff") + except User.DoesNotExist: + user = None + + # Check CourseStaff and CourseInstructor (platform course roles) + if user and user_id in self.context.get("course_staff_user_ids", []): + course_id = self.context.get("course_id") + if course_id: + if CourseInstructorRole(course_id).has_user(user): + roles.append("Course Instructor") + if CourseStaffRole(course_id).has_user(user): + roles.append("Course Staff") + + # Check discussion-specific moderator roles + if user_id in self.context.get("moderator_user_ids", []): + course_id = self.context.get("course_id") + if course_id: + user_roles = Role.objects.filter( + users__id=user_id, + course_id=course_id, + name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR] + ).values_list('name', flat=True) + + if FORUM_ROLE_ADMINISTRATOR in user_roles: + roles.append("Administrator") + if FORUM_ROLE_MODERATOR in user_roles: + roles.append("Moderator") + + # Check discussion-specific TA roles + if user_id in self.context.get("ta_user_ids", []): + course_id = self.context.get("course_id") + if course_id: + user_roles = Role.objects.filter( + users__id=user_id, + course_id=course_id, + name__in=[FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR] + ).values_list('name', flat=True) + + if FORUM_ROLE_COMMUNITY_TA in user_roles: + roles.append("Community TA") + if FORUM_ROLE_GROUP_MODERATOR in user_roles: + roles.append("Group Moderator") + + return roles if roles else None + + def get_learner_status(self, obj): + """ + Get the learner status for the discussion post author. + Returns one of: "anonymous", "staff", "new", "regular" + """ + # Skip for anonymous content + if self._is_anonymous(obj) or obj.get("user_id") is None: + return "anonymous" + + try: + user = User.objects.get(id=int(obj["user_id"])) + except (User.DoesNotExist, ValueError): + return "anonymous" + + course = self.context.get("course") + if not course: + return "anonymous" + + return get_user_learner_status(user, course.id) + def _get_user_label_from_username(self, username): """ Returns role label of user from username @@ -270,13 +382,25 @@ def _get_user_label_from_username(self, username): def get_author_label(self, obj): """ - Returns the role label for the content author. + Returns the primary role label for the content author as a string. + Returns None for posts that are anonymous to the viewer. + For anonymous_to_peers posts, staff/moderators/admins can see the label. """ if self._is_anonymous(obj) or obj["user_id"] is None: return None - else: - user_id = int(obj["user_id"]) - return self._get_user_label(user_id) + return self._get_user_label(int(obj["user_id"])) + + def get_author_labels(self, obj): + """ + Returns all role labels for the content author as an array. + New additive field for multi-role display in the frontend. + Existing legacy fields (edit_by_label, closed_by_label, endorsed_by_label, + deleted_by_label) are unaffected and continue to use _get_user_label. + Returns None for anonymous posts or users with no recognized roles. + """ + if self._is_anonymous(obj) or obj["user_id"] is None: + return None + return self._get_user_labels_all(int(obj["user_id"])) def get_rendered_body(self, obj): """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py index bc066e14f7bc..f023dbf8e19a 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py @@ -349,6 +349,7 @@ def test_basic_in_blackout_period_with_user_access(self, mock_emit): expected = self.expected_thread_data( { "author_label": "Moderator", + "author_labels": ["Moderator"], "id": "test_id", "course_id": str(self.course.id), "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_id", @@ -680,6 +681,8 @@ def test_success(self, parent_id, mock_emit): "parent_id": parent_id, "author": self.user.username, "author_label": None, + "author_labels": None, + "learner_status": "new", "created_at": "2015-05-27T00:00:00Z", "updated_at": "2015-05-27T00:00:00Z", "raw_body": "Test body", @@ -788,6 +791,8 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit): "parent_id": parent_id, "author": self.user.username, "author_label": "Moderator", + "author_labels": ["Moderator"], + "learner_status": "staff", "created_at": "2015-05-27T00:00:00Z", "updated_at": "2015-05-27T00:00:00Z", "raw_body": "Test body", @@ -1791,6 +1796,8 @@ def test_basic(self, parent_id): "parent_id": parent_id, "author": self.user.username, "author_label": None, + "author_labels": None, + "learner_status": "new", "created_at": "2015-06-03T00:00:00Z", "updated_at": "2015-06-03T00:00:00Z", "raw_body": "Edited body", @@ -3729,6 +3736,8 @@ def get_source_and_expected_comments(self): "parent_id": None, "author": self.author.username, "author_label": None, + "author_labels": None, + "learner_status": "new", "created_at": "2015-05-11T00:00:00Z", "updated_at": "2015-05-11T11:11:11Z", "raw_body": "Test body", @@ -3763,6 +3772,8 @@ def get_source_and_expected_comments(self): "parent_id": None, "author": None, "author_label": None, + "author_labels": None, + "learner_status": "anonymous", "created_at": "2015-05-11T22:22:22Z", "updated_at": "2015-05-11T33:33:33Z", "raw_body": "More content", diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py index 81b0dfe14187..e3680c91fec7 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py @@ -617,7 +617,7 @@ def test_anonymity(self, role_name, anonymous, anonymous_to_peers, expected_seri assert actual_serialized_anonymous == expected_serialized_anonymous @ddt.data( - (FORUM_ROLE_ADMINISTRATOR, False, "Moderator"), + (FORUM_ROLE_ADMINISTRATOR, False, "Administrator"), (FORUM_ROLE_ADMINISTRATOR, True, None), (FORUM_ROLE_MODERATOR, False, "Moderator"), (FORUM_ROLE_MODERATOR, True, None), @@ -631,20 +631,42 @@ def test_author_labels(self, role_name, anonymous, expected_label): """ Test correctness of the author_label field. - The label should be "Staff", "Moderator", or "Community TA" for the - Administrator, Moderator, and Community TA roles, respectively, but - the label should not be present if the content is anonymous. + The label should be "Administrator", "Moderator", or "Community TA" for + the respective roles, but None if the content is anonymous. + This field is a single string for backward compatibility. role_name is the name of the author's role. anonymous is the value of the anonymous field in the content. - expected_label is the expected value of the author_label field in the - API output. + expected_label is the expected string value of the author_label field. """ self.register_get_user_response(self.user) self.create_role(role_name, [self.author]) serialized = self.serialize(self.make_cs_content({"anonymous": anonymous})) assert serialized['author_label'] == expected_label + @ddt.data( + (FORUM_ROLE_ADMINISTRATOR, False, ["Administrator"]), + (FORUM_ROLE_ADMINISTRATOR, True, None), + (FORUM_ROLE_MODERATOR, False, ["Moderator"]), + (FORUM_ROLE_MODERATOR, True, None), + (FORUM_ROLE_COMMUNITY_TA, False, ["Community TA"]), + (FORUM_ROLE_COMMUNITY_TA, True, None), + (FORUM_ROLE_STUDENT, False, None), + (FORUM_ROLE_STUDENT, True, None), + ) + @ddt.unpack + def test_author_labels_multi(self, role_name, anonymous, expected_labels): + """ + Test correctness of the author_labels field (new multi-role array). + + author_labels is a list of all roles the author holds, or None when + anonymous or when the user has no recognized roles. + """ + self.register_get_user_response(self.user) + self.create_role(role_name, [self.author]) + serialized = self.serialize(self.make_cs_content({"anonymous": anonymous})) + assert serialized['author_labels'] == expected_labels + def test_abuse_flagged(self): self.register_get_user_response(self.user) serialized = self.serialize(self.make_cs_content({"abuse_flaggers": [str(self.user.id)]})) @@ -724,6 +746,7 @@ def test_basic(self): "parent_id": None, "author": self.author.username, "author_label": None, + "author_labels": None, "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", "raw_body": "Test body", @@ -737,6 +760,7 @@ def test_basic(self): "voted": False, "vote_count": 4, "children": [], + "learner_status": "new", "editable_fields": ["abuse_flagged", "voted"], "child_count": 0, "can_delete": False, @@ -786,7 +810,7 @@ def test_endorsed_by(self, endorser_role_name, thread_anonymous): assert actual_endorser_anonymous == expected_endorser_anonymous @ddt.data( - (FORUM_ROLE_ADMINISTRATOR, "Moderator"), + (FORUM_ROLE_ADMINISTRATOR, "Administrator"), (FORUM_ROLE_MODERATOR, "Moderator"), (FORUM_ROLE_COMMUNITY_TA, "Community TA"), (FORUM_ROLE_STUDENT, None), @@ -796,7 +820,7 @@ def test_endorsed_by_labels(self, role_name, expected_label): """ Test correctness of the endorsed_by_label field. - The label should be "Staff", "Moderator", or "Community TA" for the + The label should be "Administrator", "Moderator", or "Community TA" for the Administrator, Moderator, and Community TA roles, respectively. role_name is the name of the author's role. diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py index bcf6122b98fc..e4e2f8a484a1 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py @@ -374,6 +374,7 @@ def expected_response_data(self, overrides=None): "parent_id": None, "author": self.user.username, "author_label": None, + "author_labels": None, "created_at": "1970-01-01T00:00:00Z", "updated_at": "1970-01-01T00:00:00Z", "raw_body": "Original body", @@ -401,6 +402,7 @@ def expected_response_data(self, overrides=None): "image_url_medium": "http://testserver/static/default_50.png", "image_url_small": "http://testserver/static/default_30.png", }, + "learner_status": "new", } response_data.update(overrides or {}) return response_data @@ -1525,6 +1527,7 @@ def setUp(self): {"key": "author", "value": self.author.username}, {"key": "abuse_flagged", "value": False}, {"key": "author_label", "value": None}, + {"key": "author_labels", "value": None}, {"key": "can_delete", "value": True}, {"key": "close_reason", "value": None}, { @@ -1546,7 +1549,6 @@ def setUp(self): {"key": "non_endorsed_comment_list_url", "value": None}, {"key": "preview_body", "value": "Test body"}, {"key": "raw_body", "value": "Test body"}, - {"key": "rendered_body", "value": "

Test body

"}, {"key": "response_count", "value": 0}, {"key": "topic_id", "value": "test_topic"}, @@ -1566,7 +1568,7 @@ def setUp(self): }}, {"key": "vote_count", "value": 4}, {"key": "voted", "value": False}, - + {"key": "learner_status", "value": "new"}, ] self.url = reverse("discussion_learner_threads", kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index d4dda7ea4cec..050bcc277b89 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -543,6 +543,7 @@ def expected_thread_data(self, overrides=None): "anonymous_to_peers": False, "author": self.user.username, "author_label": None, + "author_labels": None, "created_at": "1970-01-01T00:00:00Z", "updated_at": "1970-01-01T00:00:00Z", "raw_body": "Test body", @@ -588,6 +589,7 @@ def expected_thread_data(self, overrides=None): "closed_by_label": None, "close_reason": None, "close_reason_code": None, + "learner_status": "new", } response_data.update(overrides or {}) return response_data @@ -780,6 +782,7 @@ def expected_thread_data(self, overrides=None): "anonymous_to_peers": False, "author": self.user.username, "author_label": None, + "author_labels": None, "created_at": "1970-01-01T00:00:00Z", "updated_at": "1970-01-01T00:00:00Z", "raw_body": "Test body", @@ -821,6 +824,7 @@ def expected_thread_data(self, overrides=None): "response_count": 0, "last_edit": None, "edit_by_label": None, + "learner_status": "new", "closed_by": None, "closed_by_label": None, "close_reason": None,