From 0e1d4faff37082ab47cb894298be631459ab4575 Mon Sep 17 00:00:00 2001 From: naincy128 Date: Thu, 30 Oct 2025 13:38:20 +0000 Subject: [PATCH 1/5] feat: update new learner display and logic for improved visibility --- src/discussions/common/AlertBanner.jsx | 5 + src/discussions/common/AlertBar.jsx | 4 + src/discussions/common/AuthorLabel.jsx | 284 +++++++++++------- src/discussions/common/AuthorLabel.test.jsx | 52 +++- src/discussions/data/hooks/useIsNewLearner.js | 19 ++ .../data/hooks/useLearnerStatus.js | 52 ++++ src/discussions/messages.js | 20 ++ .../comments/comment/Comment.jsx | 1 + .../comments/comment/CommentHeader.jsx | 4 + .../post-comments/comments/comment/Reply.jsx | 5 +- .../posts/post-editor/PostEditor.jsx | 6 +- .../posts/post-editor/PostEditor.test.jsx | 2 +- src/discussions/posts/post/Post.jsx | 5 +- src/discussions/posts/post/PostHeader.jsx | 4 + src/discussions/posts/post/PostLink.jsx | 4 +- 15 files changed, 353 insertions(+), 114 deletions(-) create mode 100644 src/discussions/data/hooks/useIsNewLearner.js create mode 100644 src/discussions/data/hooks/useLearnerStatus.js diff --git a/src/discussions/common/AlertBanner.jsx b/src/discussions/common/AlertBanner.jsx index db6cde332..cff044c62 100644 --- a/src/discussions/common/AlertBanner.jsx +++ b/src/discussions/common/AlertBanner.jsx @@ -24,6 +24,7 @@ const AlertBanner = ({ closeReason, editByLabel, closedByLabel, + postData, }) => { const intl = useIntl(); const userHasModerationPrivileges = useSelector(selectUserHasModerationPrivileges); @@ -53,6 +54,7 @@ const AlertBanner = ({ authorLabel={editByLabel} labelColor={editByLabelColor && `text-${editByLabelColor}`} reason={lastEdit.reason} + postData={postData} /> )} {closed && ( @@ -62,6 +64,7 @@ const AlertBanner = ({ authorLabel={closedByLabel} labelColor={closedByLabelColor && `text-${closedByLabelColor}`} reason={closeReason} + postData={postData} /> )} @@ -82,6 +85,7 @@ AlertBanner.propTypes = { editorUsername: PropTypes.string, reason: PropTypes.string, }), + postData: PropTypes.shape({}), }; AlertBanner.defaultProps = { @@ -92,6 +96,7 @@ AlertBanner.defaultProps = { closeReason: undefined, editByLabel: undefined, lastEdit: {}, + postData: null, }; export default React.memo(AlertBanner); diff --git a/src/discussions/common/AlertBar.jsx b/src/discussions/common/AlertBar.jsx index 3464860b4..d5c1f6f73 100644 --- a/src/discussions/common/AlertBar.jsx +++ b/src/discussions/common/AlertBar.jsx @@ -14,6 +14,7 @@ const AlertBar = ({ authorLabel, labelColor, reason, + postData, }) => { const intl = useIntl(); @@ -28,6 +29,7 @@ const AlertBar = ({ labelColor={labelColor} linkToProfile postOrComment + postData={postData} /> { timeago.register('time-locale', timeLocale); const intl = useIntl(); const { courseId, enableInContextSidebar } = useContext(DiscussionContext); - const { icon, authorLabelMessage } = useMemo(() => getAuthorLabel(intl, authorLabel), [authorLabel]); + const { icon, authorLabelMessage } = useMemo( + () => getAuthorLabel(intl, authorLabel), + [authorLabel], + ); + const { isNewLearner, isRegularLearner } = useLearnerStatus( + postData, + author, + authorLabel, + ); const isRetiredUser = author ? author.startsWith('retired__user') : false; const showTextPrimary = !authorLabelMessage && !isRetiredUser && !alert; - const className = classNames('d-flex align-items-center', { 'mb-0.5': !postOrComment }, labelColor); + const className = classNames( + 'd-flex align-items-center', + { 'mb-0.5': !postOrComment }, + labelColor, + ); - const showUserNameAsLink = linkToProfile && author && author !== intl.formatMessage(messages.anonymous) - && !enableInContextSidebar; + const showUserNameAsLink = ( + linkToProfile + && author + && author !== intl.formatMessage(messages.anonymous) + && !enableInContextSidebar + ); - const authorName = useMemo(() => ( - - {isRetiredUser ? '[Deactivated]' : author} - - ), [author, authorLabelMessage, isRetiredUser]); + const authorName = useMemo( + () => ( + + {isRetiredUser ? '[Deactivated]' : author} + + ), + [author, authorLabelMessage, isRetiredUser], + ); - const labelContents = useMemo(() => ( - <> - - <> - {authorToolTip ? author : authorLabel} -
- {intl.formatMessage(messages.authorAdminDescription)} - - - )} - trigger={['hover', 'focus']} + const createLearnerMessage = useCallback( + (messageKey) => ( + -
- - {authorLabelMessage && ( - + ), + [intl], + ); + + const learnerMessageComponent = useMemo(() => { + if (isNewLearner) { + return createLearnerMessage('newLearnerMessage'); + } + if (isRegularLearner) { + return createLearnerMessage('learnerMessage'); + } + return null; + }, [isNewLearner, isRegularLearner, createLearnerMessage]); + + const labelContents = useMemo( + () => ( + <> + - {authorLabelMessage} - + <> + {authorToolTip ? author : authorLabel} +
+ {intl.formatMessage(messages.authorAdminDescription)} + + )} -
- - {postCreatedAt && ( - - {timeago.format(postCreatedAt, 'time-locale')} - - )} - - ), [author, authorLabelMessage, authorToolTip, icon, isRetiredUser, postCreatedAt, showTextPrimary, alert]); +
+ + {authorLabelMessage && ( + + {authorLabelMessage} + + )} +
+ + {postCreatedAt && ( + + {timeago.format(postCreatedAt, 'time-locale')} + + )} + + ), + [ + author, + authorLabelMessage, + authorToolTip, + icon, + isRetiredUser, + postCreatedAt, + showTextPrimary, + alert, + ], + ); const learnerPostsLink = ( ); - return showUserNameAsLink - ? ( -
- {!authorLabel ? ( - - <> - {intl.formatMessage(messages.authorLearnerTitle)} -
- {intl.formatMessage(messages.authorLearnerDescription)} - - - )} - trigger={['hover', 'focus']} - > - {learnerPostsLink} - - ) : learnerPostsLink } - {labelContents} + return showUserNameAsLink ? ( +
+
+
+ {!authorLabel ? ( + + <> + {intl.formatMessage(messages.authorLearnerTitle)} +
+ {intl.formatMessage(messages.authorLearnerDescription)} + + + )} + trigger={['hover', 'focus']} + > + {learnerPostsLink} + + ) : ( + learnerPostsLink + )} + {labelContents} +
+ {learnerMessageComponent} +
+
+ ) : ( +
+
+
+ {authorName} + {labelContents} +
+ {learnerMessageComponent}
- ) - :
{authorName}{labelContents}
; +
+ ); }; AuthorLabel.propTypes = { @@ -148,16 +230,10 @@ AuthorLabel.propTypes = { postCreatedAt: PropTypes.string, authorToolTip: PropTypes.bool, postOrComment: PropTypes.bool, -}; - -AuthorLabel.defaultProps = { - linkToProfile: false, - authorLabel: null, - labelColor: '', - alert: false, - postCreatedAt: null, - authorToolTip: false, - postOrComment: false, + postData: PropTypes.shape({ + is_new_learner: PropTypes.bool, + is_regular_learner: PropTypes.bool, + }), // Thread or comment data from API }; export default React.memo(AuthorLabel); diff --git a/src/discussions/common/AuthorLabel.test.jsx b/src/discussions/common/AuthorLabel.test.jsx index bd0dfe456..b1d9079f4 100644 --- a/src/discussions/common/AuthorLabel.test.jsx +++ b/src/discussions/common/AuthorLabel.test.jsx @@ -21,7 +21,7 @@ let store; let axiosMock; let container; -function renderComponent(author, authorLabel, linkToProfile, labelColor, enableInContextSidebar) { +function renderComponent(author, authorLabel, linkToProfile, labelColor, enableInContextSidebar, postData = null) { const wrapper = render( @@ -31,6 +31,7 @@ function renderComponent(author, authorLabel, linkToProfile, labelColor, enableI authorLabel={authorLabel} linkToProfile={linkToProfile} labelColor={labelColor} + postData={postData} /> @@ -119,4 +120,53 @@ describe('Author label', () => { }, ); }); + + describe('New learner message', () => { + it('should display new learner message when backend provides is_new_learner=true', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.getByText('👋 Hi, I am a new learner')).toBeInTheDocument(); + }); + + it('should not display new learner message when backend provides is_new_learner=false', () => { + const postData = { is_new_learner: false, is_regular_learner: false }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message when postData is not provided', () => { + renderComponent('testuser', null, false, '', false, null); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for staff users even if backend says new learner', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Staff', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for moderators', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Moderator', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for Community TAs', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Community TA', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for anonymous users', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('anonymous', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for retired users', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('retired__user_123', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + }); }); diff --git a/src/discussions/data/hooks/useIsNewLearner.js b/src/discussions/data/hooks/useIsNewLearner.js new file mode 100644 index 000000000..9b68b1eca --- /dev/null +++ b/src/discussions/data/hooks/useIsNewLearner.js @@ -0,0 +1,19 @@ +import { useLearnerStatus } from './useLearnerStatus'; + +/** + * Custom hook to determine if a user is a new learner. + * + * This hook now consumes the is_new_learner field from the backend API + * and is a convenience wrapper around useLearnerStatus for backward compatibility. + * + * @param {Object} postData - The thread or comment data from the API that includes is_new_learner field + * @param {string} author - The username of the author (fallback for legacy usage) + * @param {string} authorLabel - The author's role label (Staff, Moderator, etc.) + * @returns {boolean} - True if the user is a new learner, false otherwise + */ +export const useIsNewLearner = (postData, author, authorLabel) => { + const { isNewLearner } = useLearnerStatus(postData, author, authorLabel); + return isNewLearner; +}; + +export default useIsNewLearner; diff --git a/src/discussions/data/hooks/useLearnerStatus.js b/src/discussions/data/hooks/useLearnerStatus.js new file mode 100644 index 000000000..4741adba8 --- /dev/null +++ b/src/discussions/data/hooks/useLearnerStatus.js @@ -0,0 +1,52 @@ +import { useMemo } from 'react'; + +/** + * Custom hook to determine learner status (new learner, regular learner, or neither). + * + * This hook consumes both is_new_learner and is_regular_learner fields from the backend API. + * The backend determines learner status based on actual engagement metrics and enrollment data. + * + * @param {Object} postData - The thread or comment data from the API that includes learner fields + * @param {string} author - The username of the author; used to check for anonymous and retired users to suppress learner messages + * @param {string} authorLabel - The author's role label (Staff, Moderator, etc.) + * @returns {Object} - { isNewLearner: boolean, isRegularLearner: boolean } + */ +export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => { + // Users with special roles (Staff, Moderator, Community TA) should not display learner messages + // Anonymous and retired users should also not display learner messages + if ( + authorLabel + || author === 'anonymous' + || (author && author.startsWith('retired__user')) + ) { + return { + isNewLearner: false, + isRegularLearner: false, + }; + } + + // Always rely on backend-provided fields + // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase + if (postData && typeof postData === 'object') { + const isNewLearner = postData.isNewLearner + || postData.is_new_learner + || false; + const isRegularLearner = postData.isRegularLearner + || postData.is_regular_learner + || false; + + return { + isNewLearner, + isRegularLearner, + }; + } + + // If postData is not available, return false for both + // Do not attempt client-side detection as it would produce false positives + return { + isNewLearner: false, + isRegularLearner: false, + }; +}, [postData, author, authorLabel]); + +export default useLearnerStatus; diff --git a/src/discussions/messages.js b/src/discussions/messages.js index 38122a482..3af911e76 100644 --- a/src/discussions/messages.js +++ b/src/discussions/messages.js @@ -223,6 +223,26 @@ const messages = defineMessages({ defaultMessage: 'Taking the course just like you', description: 'tooltip for course learners', }, + newLearnerMessage: { + id: 'discussions.author.newLearner.message', + defaultMessage: '👋 Hi, I am a new learner', + description: 'Message displayed below username for new learners who have only viewed the course outline', + }, + learnerMessage: { + id: 'discussions.author.learner.message', + defaultMessage: 'Learner', + description: 'Message displayed below username for regular learners', + }, + spamWarningHeading: { + id: 'discussions.spamWarning.heading', + defaultMessage: 'Reminder', + description: 'Heading for the spam warning banner', + }, + spamWarningMessage: { + id: 'discussions.spamWarning.message', + defaultMessage: 'Faculty and staff will never invite you to join external groups or ask for personal or financial information in the discussions. Stay safe, and if you see suspicious activity, please report it.', + description: 'Warning message about spam and impersonation in discussion forums', + }, }); export default messages; diff --git a/src/discussions/post-comments/comments/comment/Comment.jsx b/src/discussions/post-comments/comments/comment/Comment.jsx index 7a0820744..966cf492b 100644 --- a/src/discussions/post-comments/comments/comment/Comment.jsx +++ b/src/discussions/post-comments/comments/comment/Comment.jsx @@ -210,6 +210,7 @@ const Comment = ({ createdAt={createdAt} lastEdit={lastEdit} postUsers={postUsers} + postData={comment} /> {isEditing ? ( { const colorClass = AvatarOutlineAndLabelColors[authorLabel]; const hasAnyAlert = useAlertBannerVisible({ @@ -56,6 +57,7 @@ const CommentHeader = ({ linkToProfile postCreatedAt={createdAt} postOrComment + postData={postData} />
@@ -73,12 +75,14 @@ CommentHeader.propTypes = { reason: PropTypes.string, }), postUsers: PropTypes.shape({}).isRequired, + postData: PropTypes.shape({}), }; CommentHeader.defaultProps = { authorLabel: null, closed: undefined, lastEdit: null, + postData: null, }; export default React.memo(CommentHeader); diff --git a/src/discussions/post-comments/comments/comment/Reply.jsx b/src/discussions/post-comments/comments/comment/Reply.jsx index 91ecaa9fc..ba0f054c0 100644 --- a/src/discussions/post-comments/comments/comment/Reply.jsx +++ b/src/discussions/post-comments/comments/comment/Reply.jsx @@ -23,10 +23,11 @@ import CommentEditor from './CommentEditor'; const Reply = ({ responseId }) => { timeago.register('time-locale', timeLocale); + const commentData = useSelector(selectCommentOrResponseById(responseId)); const { id, abuseFlagged, author, authorLabel, endorsed, lastEdit, closed, closedBy, closeReason, createdAt, threadId, parentId, rawBody, renderedBody, editByLabel, closedByLabel, - } = useSelector(selectCommentOrResponseById(responseId)); + } = commentData; const intl = useIntl(); const dispatch = useDispatch(); const [isEditing, setEditing] = useState(false); @@ -114,6 +115,7 @@ const Reply = ({ responseId }) => { lastEdit={lastEdit} editByLabel={editByLabel} closedByLabel={closedByLabel} + postData={commentData} /> @@ -142,6 +144,7 @@ const Reply = ({ responseId }) => { linkToProfile postCreatedAt={createdAt} postOrComment + postData={commentData} />
{ const intl = useIntl(); const navigate = useNavigate(); @@ -556,8 +556,4 @@ PostEditor.propTypes = { openRestrictionDialogue: PropTypes.func.isRequired, }; -PostEditor.defaultProps = { - editExisting: false, -}; - export default React.memo(withPostingRestrictions(PostEditor)); diff --git a/src/discussions/posts/post-editor/PostEditor.test.jsx b/src/discussions/posts/post-editor/PostEditor.test.jsx index e9c0c5040..17a62887e 100644 --- a/src/discussions/posts/post-editor/PostEditor.test.jsx +++ b/src/discussions/posts/post-editor/PostEditor.test.jsx @@ -61,7 +61,7 @@ async function renderComponent(editExisting = false, location = `/${courseId}/po {paths.map((path) => ( - } /> + } /> ))} diff --git a/src/discussions/posts/post/Post.jsx b/src/discussions/posts/post/Post.jsx index dd51d1445..5fed345c0 100644 --- a/src/discussions/posts/post/Post.jsx +++ b/src/discussions/posts/post/Post.jsx @@ -30,11 +30,12 @@ import PostHeader from './PostHeader'; const Post = ({ handleAddResponseButton, openRestrictionDialogue }) => { const { enableInContextSidebar, postId } = useContext(DiscussionContext); + const threadData = useSelector(selectThread(postId)); const { topicId, abuseFlagged, closed, pinned, voted, hasEndorsed, following, closedBy, voteCount, groupId, groupName, closeReason, authorLabel, type: postType, author, title, createdAt, renderedBody, lastEdit, editByLabel, closedByLabel, users: postUsers, - } = useSelector(selectThread(postId)); + } = threadData; const intl = useIntl(); const location = useLocation(); const navigate = useNavigate(); @@ -176,6 +177,7 @@ const Post = ({ handleAddResponseButton, openRestrictionDialogue }) => { closeReason={closeReason} editByLabel={editByLabel} closedByLabel={closedByLabel} + postData={threadData} /> { postType={postType} title={title} postUsers={postUsers} + postData={threadData} />
diff --git a/src/discussions/posts/post/PostHeader.jsx b/src/discussions/posts/post/PostHeader.jsx index 5f40985dd..6725a0b2c 100644 --- a/src/discussions/posts/post/PostHeader.jsx +++ b/src/discussions/posts/post/PostHeader.jsx @@ -101,6 +101,7 @@ const PostHeader = ({ postType, preview, postUsers, + postData, }) => { const intl = useIntl(); const showAnsweredBadge = preview && hasEndorsed && postType === ThreadType.QUESTION; @@ -142,6 +143,7 @@ const PostHeader = ({ linkToProfile postCreatedAt={createdAt} postOrComment + postData={postData} />
@@ -163,6 +165,7 @@ PostHeader.propTypes = { }), closed: PropTypes.bool, postUsers: PropTypes.shape({}).isRequired, + postData: PropTypes.shape({}), }; PostHeader.defaultProps = { @@ -171,6 +174,7 @@ PostHeader.defaultProps = { abuseFlagged: false, lastEdit: {}, closed: false, + postData: null, }; export default React.memo(PostHeader); diff --git a/src/discussions/posts/post/PostLink.jsx b/src/discussions/posts/post/PostLink.jsx index 5501fba1c..3693d0cab 100644 --- a/src/discussions/posts/post/PostLink.jsx +++ b/src/discussions/posts/post/PostLink.jsx @@ -33,11 +33,12 @@ const PostLink = ({ category, learnerUsername, } = useContext(DiscussionContext); + const threadData = useSelector(selectThread(postId)); const { topicId, hasEndorsed, type, author, authorLabel, abuseFlagged, abuseFlaggedCount, read, commentCount, unreadCommentCount, id, pinned, previewBody, title, voted, voteCount, following, groupId, groupName, createdAt, users: postUsers, - } = useSelector(selectThread(postId)); + } = threadData; const { pathname } = discussionsPath(Routes.COMMENTS.PAGES[page], { 0: enableInContextSidebar ? 'in-context' : undefined, courseId, @@ -135,6 +136,7 @@ const PostLink = ({ author={author || intl.formatMessage(messages.anonymous)} authorLabel={authorLabel} labelColor={authorLabelColor && `text-${authorLabelColor}`} + postData={threadData} /> Date: Thu, 30 Oct 2025 16:31:11 +0000 Subject: [PATCH 2/5] feat: update new learner display and logic for improved visibility --- .../data/hooks/useLearnerStatus.js | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/discussions/data/hooks/useLearnerStatus.js b/src/discussions/data/hooks/useLearnerStatus.js index 4741adba8..465f1a1cb 100644 --- a/src/discussions/data/hooks/useLearnerStatus.js +++ b/src/discussions/data/hooks/useLearnerStatus.js @@ -7,46 +7,46 @@ import { useMemo } from 'react'; * The backend determines learner status based on actual engagement metrics and enrollment data. * * @param {Object} postData - The thread or comment data from the API that includes learner fields - * @param {string} author - The username of the author; used to check for anonymous and retired users to suppress learner messages + * @param {string} author - The username of the author; used to check for anonymous and retired users + * to suppress learner messages * @param {string} authorLabel - The author's role label (Staff, Moderator, etc.) * @returns {Object} - { isNewLearner: boolean, isRegularLearner: boolean } */ -export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => { - // Users with special roles (Staff, Moderator, Community TA) should not display learner messages - // Anonymous and retired users should also not display learner messages - if ( - authorLabel - || author === 'anonymous' - || (author && author.startsWith('retired__user')) - ) { - return { - isNewLearner: false, - isRegularLearner: false, - }; - } +export const useLearnerStatus = (postData, author, authorLabel) => + useMemo(() => { + // Users with special roles (Staff, Moderator, Community TA) should not display learner messages + // Anonymous and retired users should also not display learner messages + if ( + authorLabel || + author === 'anonymous' || + (author && author.startsWith('retired__user')) + ) { + return { + isNewLearner: false, + isRegularLearner: false, + }; + } - // Always rely on backend-provided fields - // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase - if (postData && typeof postData === 'object') { - const isNewLearner = postData.isNewLearner - || postData.is_new_learner - || false; - const isRegularLearner = postData.isRegularLearner - || postData.is_regular_learner - || false; + // Always rely on backend-provided fields + // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase + if (postData && typeof postData === 'object') { + const isNewLearner = + postData.isNewLearner || postData.is_new_learner || false; + const isRegularLearner = + postData.isRegularLearner || postData.is_regular_learner || false; + return { + isNewLearner, + isRegularLearner, + }; + } + + // If postData is not available, return false for both + // Do not attempt client-side detection as it would produce false positives return { - isNewLearner, - isRegularLearner, + isNewLearner: false, + isRegularLearner: false, }; - } - - // If postData is not available, return false for both - // Do not attempt client-side detection as it would produce false positives - return { - isNewLearner: false, - isRegularLearner: false, - }; -}, [postData, author, authorLabel]); + }, [postData, author, authorLabel]); export default useLearnerStatus; From 9f8a5707da0f5244cd21630eb4f6092ec62dccf9 Mon Sep 17 00:00:00 2001 From: naincy128 Date: Thu, 30 Oct 2025 16:36:05 +0000 Subject: [PATCH 3/5] feat: update new learner display and logic for improved visibility --- .../data/hooks/useLearnerStatus.js | 65 ++++++++++--------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/discussions/data/hooks/useLearnerStatus.js b/src/discussions/data/hooks/useLearnerStatus.js index 465f1a1cb..6c6c5da22 100644 --- a/src/discussions/data/hooks/useLearnerStatus.js +++ b/src/discussions/data/hooks/useLearnerStatus.js @@ -12,41 +12,42 @@ import { useMemo } from 'react'; * @param {string} authorLabel - The author's role label (Staff, Moderator, etc.) * @returns {Object} - { isNewLearner: boolean, isRegularLearner: boolean } */ -export const useLearnerStatus = (postData, author, authorLabel) => - useMemo(() => { - // Users with special roles (Staff, Moderator, Community TA) should not display learner messages - // Anonymous and retired users should also not display learner messages - if ( - authorLabel || - author === 'anonymous' || - (author && author.startsWith('retired__user')) - ) { - return { - isNewLearner: false, - isRegularLearner: false, - }; - } - - // Always rely on backend-provided fields - // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase - if (postData && typeof postData === 'object') { - const isNewLearner = - postData.isNewLearner || postData.is_new_learner || false; - const isRegularLearner = - postData.isRegularLearner || postData.is_regular_learner || false; - - return { - isNewLearner, - isRegularLearner, - }; - } - - // If postData is not available, return false for both - // Do not attempt client-side detection as it would produce false positives +export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => { + // Users with special roles (Staff, Moderator, Community TA) should not display learner messages + // Anonymous and retired users should also not display learner messages + if ( + authorLabel + || author === 'anonymous' + || (author && author.startsWith('retired__user')) + ) { return { isNewLearner: false, isRegularLearner: false, }; - }, [postData, author, authorLabel]); + } + + // Always rely on backend-provided fields + // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase + if (postData && typeof postData === 'object') { + const isNewLearner = postData.isNewLearner + || postData.is_new_learner + || false; + const isRegularLearner = postData.isRegularLearner + || postData.is_regular_learner + || false; + + return { + isNewLearner, + isRegularLearner, + }; + } + + // If postData is not available, return false for both + // Do not attempt client-side detection as it would produce false positives + return { + isNewLearner: false, + isRegularLearner: false, + }; +}, [postData, author, authorLabel]); export default useLearnerStatus; From 23bb5bc1a65b895ffd2dbdc8d0440105706e49d8 Mon Sep 17 00:00:00 2001 From: naincy128 Date: Tue, 4 Nov 2025 11:22:08 +0000 Subject: [PATCH 4/5] feat: update new learner display and logic for improved visibility --- src/discussions/common/AuthorLabel.test.jsx | 130 ++++++++++++------ .../data/hooks/useLearnerStatus.js | 34 +++-- .../data/hooks/useLearnerStatus.test.js | 119 ++++++++++++++++ 3 files changed, 228 insertions(+), 55 deletions(-) create mode 100644 src/discussions/data/hooks/useLearnerStatus.test.js diff --git a/src/discussions/common/AuthorLabel.test.jsx b/src/discussions/common/AuthorLabel.test.jsx index b1d9079f4..4ecf54206 100644 --- a/src/discussions/common/AuthorLabel.test.jsx +++ b/src/discussions/common/AuthorLabel.test.jsx @@ -121,52 +121,100 @@ describe('Author label', () => { ); }); - describe('New learner message', () => { - it('should display new learner message when backend provides is_new_learner=true', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('testuser', null, false, '', false, postData); - expect(screen.getByText('👋 Hi, I am a new learner')).toBeInTheDocument(); + describe('Learner status messages', () => { + describe('with new learner_status API field', () => { + it('should display new learner message when backend provides learner_status="new"', () => { + const postData = { learner_status: 'new' }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.getByText('👋 Hi, I am a new learner')).toBeInTheDocument(); + }); + + it('should not display new learner message when backend provides learner_status="regular"', () => { + const postData = { learner_status: 'regular' }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message when backend provides learner_status="staff"', () => { + const postData = { learner_status: 'staff' }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message when backend provides learner_status="anonymous"', () => { + const postData = { learner_status: 'anonymous' }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for staff users even if backend says learner_status="new"', () => { + const postData = { learner_status: 'new' }; + renderComponent('testuser', 'Staff', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); }); - it('should not display new learner message when backend provides is_new_learner=false', () => { - const postData = { is_new_learner: false, is_regular_learner: false }; - renderComponent('testuser', null, false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + describe('with legacy boolean API fields (backward compatibility)', () => { + it('should display new learner message when backend provides is_new_learner=true', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.getByText('👋 Hi, I am a new learner')).toBeInTheDocument(); + }); + + it('should not display new learner message when backend provides is_new_learner=false', () => { + const postData = { is_new_learner: false, is_regular_learner: false }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for staff users even if backend says new learner', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Staff', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for moderators', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Moderator', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for Community TAs', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('testuser', 'Community TA', false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for anonymous users', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('anonymous', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should not display new learner message for retired users', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + renderComponent('retired__user_123', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); + + it('should prioritize new learner_status field over legacy boolean fields', () => { + // If both are present, learner_status should take precedence + const postData = { learner_status: 'regular', is_new_learner: true }; + renderComponent('testuser', null, false, '', false, postData); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); }); - it('should not display new learner message when postData is not provided', () => { - renderComponent('testuser', null, false, '', false, null); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); - }); - - it('should not display new learner message for staff users even if backend says new learner', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('testuser', 'Staff', false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); - }); - - it('should not display new learner message for moderators', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('testuser', 'Moderator', false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); - }); - - it('should not display new learner message for Community TAs', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('testuser', 'Community TA', false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); - }); - - it('should not display new learner message for anonymous users', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('anonymous', null, false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); - }); + describe('general cases', () => { + it('should not display new learner message when postData is not provided', () => { + renderComponent('testuser', null, false, '', false, null); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); - it('should not display new learner message for retired users', () => { - const postData = { is_new_learner: true, is_regular_learner: false }; - renderComponent('retired__user_123', null, false, '', false, postData); - expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + it('should not display new learner message when postData is empty object', () => { + renderComponent('testuser', null, false, '', false, {}); + expect(screen.queryByText('👋 Hi, I am a new learner')).not.toBeInTheDocument(); + }); }); }); }); diff --git a/src/discussions/data/hooks/useLearnerStatus.js b/src/discussions/data/hooks/useLearnerStatus.js index 6c6c5da22..ad8dacc21 100644 --- a/src/discussions/data/hooks/useLearnerStatus.js +++ b/src/discussions/data/hooks/useLearnerStatus.js @@ -3,10 +3,10 @@ import { useMemo } from 'react'; /** * Custom hook to determine learner status (new learner, regular learner, or neither). * - * This hook consumes both is_new_learner and is_regular_learner fields from the backend API. + * This hook consumes the learner_status field from the backend API. * The backend determines learner status based on actual engagement metrics and enrollment data. * - * @param {Object} postData - The thread or comment data from the API that includes learner fields + * @param {Object} postData - The thread or comment data from the API that includes learner_status field * @param {string} author - The username of the author; used to check for anonymous and retired users * to suppress learner messages * @param {string} authorLabel - The author's role label (Staff, Moderator, etc.) @@ -26,20 +26,26 @@ export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => }; } - // Always rely on backend-provided fields - // Note: Backend sends 'is_new_learner'/'is_regular_learner' but frontend may transform to camelCase + // Always rely on backend-provided learner_status field if (postData && typeof postData === 'object') { - const isNewLearner = postData.isNewLearner - || postData.is_new_learner - || false; - const isRegularLearner = postData.isRegularLearner - || postData.is_regular_learner - || false; + const learnerStatus = postData.learnerStatus || postData.learner_status; + + // Also check for legacy boolean fields for backward compatibility + const legacyIsNewLearner = postData.isNewLearner || postData.is_new_learner; + const legacyIsRegularLearner = postData.isRegularLearner || postData.is_regular_learner; - return { - isNewLearner, - isRegularLearner, - }; + // Use new learner_status field if available, otherwise fall back to legacy boolean fields + if (learnerStatus) { + return { + isNewLearner: learnerStatus === 'new', + isRegularLearner: learnerStatus === 'regular', + }; + } else if (legacyIsNewLearner !== undefined || legacyIsRegularLearner !== undefined) { + return { + isNewLearner: Boolean(legacyIsNewLearner), + isRegularLearner: Boolean(legacyIsRegularLearner), + }; + } } // If postData is not available, return false for both diff --git a/src/discussions/data/hooks/useLearnerStatus.test.js b/src/discussions/data/hooks/useLearnerStatus.test.js new file mode 100644 index 000000000..a20183ef9 --- /dev/null +++ b/src/discussions/data/hooks/useLearnerStatus.test.js @@ -0,0 +1,119 @@ +import { renderHook } from '@testing-library/react'; +import { useLearnerStatus } from './useLearnerStatus'; + +describe('useLearnerStatus hook', () => { + describe('with new learner_status field', () => { + it('should return isNewLearner=true for learner_status="new"', () => { + const postData = { learner_status: 'new' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(true); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return isRegularLearner=true for learner_status="regular"', () => { + const postData = { learner_status: 'regular' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(true); + }); + + it('should return both false for learner_status="staff"', () => { + const postData = { learner_status: 'staff' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return both false for learner_status="anonymous"', () => { + const postData = { learner_status: 'anonymous' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return both false for learner_status="unenrolled"', () => { + const postData = { learner_status: 'unenrolled' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + }); + + describe('with legacy boolean fields', () => { + it('should work with legacy is_new_learner=true', () => { + const postData = { is_new_learner: true, is_regular_learner: false }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(true); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should work with legacy is_regular_learner=true', () => { + const postData = { is_new_learner: false, is_regular_learner: true }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(true); + }); + + it('should prioritize learner_status over legacy fields', () => { + // If both new and legacy fields are present, learner_status should take precedence + const postData = { + learner_status: 'regular', + is_new_learner: true, + is_regular_learner: false + }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(true); + }); + }); + + describe('special user types', () => { + it('should return false for staff users regardless of learner_status', () => { + const postData = { learner_status: 'new' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', 'Staff')); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return false for anonymous users', () => { + const postData = { learner_status: 'new' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'anonymous', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return false for retired users', () => { + const postData = { learner_status: 'new' }; + const { result } = renderHook(() => useLearnerStatus(postData, 'retired__user_123', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + }); + + describe('edge cases', () => { + it('should return false when postData is null', () => { + const { result } = renderHook(() => useLearnerStatus(null, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + + it('should return false when postData is empty object', () => { + const { result } = renderHook(() => useLearnerStatus({}, 'testuser', null)); + + expect(result.current.isNewLearner).toBe(false); + expect(result.current.isRegularLearner).toBe(false); + }); + }); +}); \ No newline at end of file From 73626e895d8ef6b09408602a80141487c68392c4 Mon Sep 17 00:00:00 2001 From: naincy128 Date: Tue, 4 Nov 2025 11:26:11 +0000 Subject: [PATCH 5/5] feat: update new learner display and logic for improved visibility --- .../data/hooks/useLearnerStatus.js | 4 +- .../data/hooks/useLearnerStatus.test.js | 37 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/discussions/data/hooks/useLearnerStatus.js b/src/discussions/data/hooks/useLearnerStatus.js index ad8dacc21..a039635d3 100644 --- a/src/discussions/data/hooks/useLearnerStatus.js +++ b/src/discussions/data/hooks/useLearnerStatus.js @@ -29,7 +29,7 @@ export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => // Always rely on backend-provided learner_status field if (postData && typeof postData === 'object') { const learnerStatus = postData.learnerStatus || postData.learner_status; - + // Also check for legacy boolean fields for backward compatibility const legacyIsNewLearner = postData.isNewLearner || postData.is_new_learner; const legacyIsRegularLearner = postData.isRegularLearner || postData.is_regular_learner; @@ -40,7 +40,7 @@ export const useLearnerStatus = (postData, author, authorLabel) => useMemo(() => isNewLearner: learnerStatus === 'new', isRegularLearner: learnerStatus === 'regular', }; - } else if (legacyIsNewLearner !== undefined || legacyIsRegularLearner !== undefined) { + } if (legacyIsNewLearner !== undefined || legacyIsRegularLearner !== undefined) { return { isNewLearner: Boolean(legacyIsNewLearner), isRegularLearner: Boolean(legacyIsRegularLearner), diff --git a/src/discussions/data/hooks/useLearnerStatus.test.js b/src/discussions/data/hooks/useLearnerStatus.test.js index a20183ef9..cf271e632 100644 --- a/src/discussions/data/hooks/useLearnerStatus.test.js +++ b/src/discussions/data/hooks/useLearnerStatus.test.js @@ -1,4 +1,5 @@ import { renderHook } from '@testing-library/react'; + import { useLearnerStatus } from './useLearnerStatus'; describe('useLearnerStatus hook', () => { @@ -6,7 +7,7 @@ describe('useLearnerStatus hook', () => { it('should return isNewLearner=true for learner_status="new"', () => { const postData = { learner_status: 'new' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(true); expect(result.current.isRegularLearner).toBe(false); }); @@ -14,7 +15,7 @@ describe('useLearnerStatus hook', () => { it('should return isRegularLearner=true for learner_status="regular"', () => { const postData = { learner_status: 'regular' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(true); }); @@ -22,7 +23,7 @@ describe('useLearnerStatus hook', () => { it('should return both false for learner_status="staff"', () => { const postData = { learner_status: 'staff' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -30,7 +31,7 @@ describe('useLearnerStatus hook', () => { it('should return both false for learner_status="anonymous"', () => { const postData = { learner_status: 'anonymous' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -38,7 +39,7 @@ describe('useLearnerStatus hook', () => { it('should return both false for learner_status="unenrolled"', () => { const postData = { learner_status: 'unenrolled' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -48,7 +49,7 @@ describe('useLearnerStatus hook', () => { it('should work with legacy is_new_learner=true', () => { const postData = { is_new_learner: true, is_regular_learner: false }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(true); expect(result.current.isRegularLearner).toBe(false); }); @@ -56,20 +57,20 @@ describe('useLearnerStatus hook', () => { it('should work with legacy is_regular_learner=true', () => { const postData = { is_new_learner: false, is_regular_learner: true }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(true); }); it('should prioritize learner_status over legacy fields', () => { // If both new and legacy fields are present, learner_status should take precedence - const postData = { - learner_status: 'regular', - is_new_learner: true, - is_regular_learner: false + const postData = { + learner_status: 'regular', + is_new_learner: true, + is_regular_learner: false, }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(true); }); @@ -79,7 +80,7 @@ describe('useLearnerStatus hook', () => { it('should return false for staff users regardless of learner_status', () => { const postData = { learner_status: 'new' }; const { result } = renderHook(() => useLearnerStatus(postData, 'testuser', 'Staff')); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -87,7 +88,7 @@ describe('useLearnerStatus hook', () => { it('should return false for anonymous users', () => { const postData = { learner_status: 'new' }; const { result } = renderHook(() => useLearnerStatus(postData, 'anonymous', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -95,7 +96,7 @@ describe('useLearnerStatus hook', () => { it('should return false for retired users', () => { const postData = { learner_status: 'new' }; const { result } = renderHook(() => useLearnerStatus(postData, 'retired__user_123', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); @@ -104,16 +105,16 @@ describe('useLearnerStatus hook', () => { describe('edge cases', () => { it('should return false when postData is null', () => { const { result } = renderHook(() => useLearnerStatus(null, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); it('should return false when postData is empty object', () => { const { result } = renderHook(() => useLearnerStatus({}, 'testuser', null)); - + expect(result.current.isNewLearner).toBe(false); expect(result.current.isRegularLearner).toBe(false); }); }); -}); \ No newline at end of file +});