feat: update chat component to use PluginSlot #1810
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1810 +/- ##
==========================================
- Coverage 90.84% 90.82% -0.03%
==========================================
Files 345 345
Lines 5800 5787 -13
Branches 1376 1324 -52
==========================================
- Hits 5269 5256 -13
Misses 514 514
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mraman-2U
changed the title
There was a problem hiding this comment.
I left some comments.
Something we should think about is the learning assistant Redux store, which is included in the learning MFE store definition. Can it be removed easily? It references the learning assistant, so it should be.
There are some references to it in the repository still.
| PRIVACY_POLICY_URL='http://localhost:18000/privacy' | ||
| OPTIMIZELY_FULL_STACK_SDK_KEY='' | ||
| SHOW_UNGRADED_ASSIGNMENT_PROGRESS='' | ||
| ENABLE_XPERT_AUDIT='true' |
There was a problem hiding this comment.
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.
| contentToolsEnabled={contentToolsEnabled} | ||
| unitId={unitId} | ||
| isUpgradeEligible={auditMode} | ||
| <PluginSlot |
There was a problem hiding this comment.
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"?
| ACCESS_TOKEN_COOKIE_NAME='edx-jwt-cookie-header-payload' | ||
| APP_ID='learning' | ||
| BASE_URL='http://localhost:2000' | ||
| BASE_URL='http://localhost:2010' |
There was a problem hiding this comment.
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.
| // If chat is disabled, don't show anything | ||
| if (!enabled) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
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.
| return null; | ||
| } | ||
|
|
||
| // Provide minimal, generic context - no feature-specific flags |
At present it is difficult to remove this learning assistant Redux store usage from MFE. Though it defeats the purpose of having the library independent of the MFE. This may be acceptable for cross component coordination and not related to MFE business logic |
2 participants
This pull request refactors the
Chatcomponent in the courseware to use a generic plugin slot system instead of a feature-specific implementation, simplifying the logic and making it more extensible for future tools. Additionally, it updates environment configuration files and modernizes the related test suite for the new architecture.Major changes include:
Chat Component Refactor
Chat.jsxto remove all feature-specific logic (such as Xpert and enrollment mode checks) and instead render a genericPluginSlotwith minimal context, allowing plugins to handle their own requirements. This makes the component more maintainable and extensible. [1] [2]Chatto remove unused props and reflect the simplified interface.Test Suite Overhaul
Chat.test.jsxto mock the newPluginSlotand test the new, simplified rendering logic, focusing on correct context propagation and conditional rendering based solely on theenabledprop. The tests no longer depend on Redux or feature-specific logic.Environment Configuration Updates
.env.developmentfrom2000to2010to align with updated local development practices. [1] [2]ENABLE_XPERT_AUDIT='true'to.env.developmentand removed the unusedFEATURE_ENABLE_CHAT_V2_ENDPOINTflag from both.envand.env.development, cleaning up obsolete configuration. [1] [2]These changes collectively make the chat integration more modular, easier to maintain, and ready for future plugin-based learner tools.