feat: update chat visibility #1297
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1297 +/- ##
==========================================
+ Coverage 88.36% 88.37% +0.01%
==========================================
Files 291 291
Lines 4941 4948 +7
Branches 1245 1251 +6
==========================================
+ Hits 4366 4373 +7
Misses 561 561
Partials 14 14 ☔ View full report in Codecov by Sentry. |
| @@ -70,6 +70,23 @@ const Course = ({ | |||
|
|
|||
| const SidebarProviderComponent = enableNewSidebar === 'true' ? NewSidebarProvider : SidebarProvider; | |||
|
|
|||
| const chatValidDates = () => { | |||
There was a problem hiding this comment.
Since course comes from the Redux store, I think we could read in the course with the useModel hook and compute this data within the Chat component to avoid having Chat specific logic in the Course component. What do you think?
There was a problem hiding this comment.
Also, technically, this bit of code is untested. If you move this into Chat, you'll have to add this data to the mock Redux store for it to work properly in your test. That would get you better test coverage, but would be more work.
If you want to get this out and fast-follow with the refactor/testing changes, I'm good with that. But let me know if you don't think this is necessary.
There was a problem hiding this comment.
I think I'd like to get this out and then work on updating the tests/refactor afterwards. Once I merge this I'll start that work.
| @@ -182,7 +181,7 @@ describe('Chat', () => { | |||
| enabled | |||
| courseId={courseId} | |||
| contentToolsEnabled={false} | |||
| endDate={new Date(currentTime.getTime() - 10 * 60000).toISOString()} | |||
| validDates={false} | |||
There was a problem hiding this comment.
Nit: I'd rename this test, because it's testing something less specific now.
The backend for the learning assistant will not return 200 responses for courses that don't pass certain date conditions. We should replicate that behavior on the frontend to ensure that users do not see 400 errors.