Skip to content

feat: update chat component to use PluginSlot #1810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: update chat component to use PluginSlot #1810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion .env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,3 @@ OPTIMIZELY_FULL_STACK_SDK_KEY=''
SHOW_UNGRADED_ASSIGNMENT_PROGRESS=''
# Fallback in local style files
PARAGON_THEME_URLS={}
FEATURE_ENABLE_CHAT_V2_ENDPOINT=''
6 changes: 3 additions & 3 deletions .env.development
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ NODE_ENV='development'

ACCESS_TOKEN_COOKIE_NAME='edx-jwt-cookie-header-payload'
APP_ID='learning'
BASE_URL='http://localhost:2000'
BASE_URL='http://localhost:2010'
Copy link
Contributor

@michaelroytman michaelroytman Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to use port 2010 was a 2U only change, so this should remain 2000, as this is what the community continues to use.

CONTACT_URL='http://localhost:18000/contact'
CREDENTIALS_BASE_URL='http://localhost:18150'
CREDIT_HELP_LINK_URL='https://help.edx.org/edxlearner/s/article/Can-I-receive-college-credit-or-credit-hours-for-my-course'
Expand All @@ -31,7 +31,7 @@ LOGO_WHITE_URL=https://edx-cdn.org/v3/default/logo-white.svg
LEGACY_THEME_NAME=''
MARKETING_SITE_BASE_URL='http://localhost:18000'
ORDER_HISTORY_URL='http://localhost:1996/orders'
PORT=2000
PORT=2010
PROCTORED_EXAM_FAQ_URL=''
PROCTORED_EXAM_RULES_URL=''
REFRESH_ACCESS_TOKEN_ENDPOINT='http://localhost:18000/login_refresh'
Expand All @@ -53,6 +53,6 @@ CHAT_RESPONSE_URL='http://localhost:18000/api/learning_assistant/v1/course_id'
PRIVACY_POLICY_URL='http://localhost:18000/privacy'
OPTIMIZELY_FULL_STACK_SDK_KEY=''
SHOW_UNGRADED_ASSIGNMENT_PROGRESS=''
ENABLE_XPERT_AUDIT='true'
Copy link
Contributor

@michaelroytman michaelroytman Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add this here, because its use is being removed in this PR.

I think we should just provide instructions in the component README to add this environment variable to a developer's environment.

# Fallback in local style files
PARAGON_THEME_URLS={}
FEATURE_ENABLE_CHAT_V2_ENDPOINT='false'
76 changes: 23 additions & 53 deletions src/courseware/course/chat/Chat.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,66 +1,37 @@
import { createPortal } from 'react-dom';
import { useSelector } from 'react-redux';
import PropTypes from 'prop-types';

import { Xpert } from '@edx/frontend-lib-learning-assistant';
import { getConfig } from '@edx/frontend-platform';

import { ALLOW_UPSELL_MODES, VERIFIED_MODES } from '@src/constants';
import { useModel } from '../../../generic/model-store';
import { PluginSlot } from '@openedx/frontend-plugin-framework';
import { getAuthenticatedUser } from '@edx/frontend-platform/auth';

const Chat = ({
enabled,
enrollmentMode,
isStaff,
courseId,
contentToolsEnabled,
unitId,
}) => {
const {
activeAttempt, exam,
} = useSelector(state => state.specialExams);
const course = useModel('coursewareMeta', courseId);

// If is disabled or taking an exam, we don't show the chat.
if (!enabled || activeAttempt?.attempt_id || exam?.id) { return null; }

// If is not staff and doesn't have an enrollment, we don't show the chat.
if (!isStaff && !enrollmentMode) { return null; }

const verifiedMode = VERIFIED_MODES.includes(enrollmentMode); // Enrollment verified
const auditMode = (
!isStaff
&& !verifiedMode
&& ALLOW_UPSELL_MODES.includes(enrollmentMode) // Can upgrade course
&& getConfig().ENABLE_XPERT_AUDIT
);
// If user has no access, we don't show the chat.
if (!isStaff && !(verifiedMode || auditMode)) { return null; }

// Date validation
const {
accessExpiration,
start,
end,
} = course;

const utcDate = (new Date()).toISOString();
const expiration = accessExpiration?.expirationDate || utcDate;
const validDate = (
(start ? start <= utcDate : true)
&& (end ? end >= utcDate : true)
&& (auditMode ? expiration >= utcDate : true)
);
// If date is invalid, we don't show the chat.
if (!validDate) { return null; }

// Use a portal to ensure that component overlay does not compete with learning MFE styles.
const { userId } = getAuthenticatedUser();

// If chat is disabled, don't show anything
if (!enabled) {
return null;
}
Comment on lines +15 to +18
Copy link
Contributor

@michaelroytman michaelroytman Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this condition. The plugin framework allows developers to programmatically insert plugins into the plugin slot. Either the plugin can not be inserted into the plugin slot, or the plugin can determine its own rendering logic.

Also, we should probably remove the reference to course.learningAssistantEnabled here, as it's Xpert specific.


// Provide minimal, generic context - no feature-specific flags
Copy link
Contributor

@michaelroytman michaelroytman Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

const pluginContext = {
courseId,
unitId,
userId,
isStaff,
enrollmentMode,
};

// Use generic plugin slot ID (location-based, not feature-specific)
// Plugins will query their own requirements from Redux/config
return createPortal(
<Xpert
courseId={courseId}
contentToolsEnabled={contentToolsEnabled}
unitId={unitId}
isUpgradeEligible={auditMode}
<PluginSlot
Copy link
Contributor

@michaelroytman michaelroytman Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there's a conflict between the name of this component and its new purpose. Is this plugin slot meant to be used for an implementation of a chat bot? Or is it, as the ID suggests, a plugin slot for learner tools? I believe it should be the latter. Can we update the name of this component and references to "chat"?

id="learner_tools_slot"
pluginProps={pluginContext}
/>,
document.body,
);
Expand All @@ -71,7 +42,6 @@ Chat.propTypes = {
enabled: PropTypes.bool.isRequired,
enrollmentMode: PropTypes.string,
courseId: PropTypes.string.isRequired,
contentToolsEnabled: PropTypes.bool.isRequired,
unitId: PropTypes.string.isRequired,
};

Expand Down
Loading