Allow for multiple registered contexts when deciding if content is relevant #1574
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## redesign-2024 #1574 +/- ##
=================================================
+ Coverage 40.27% 40.29% +0.01%
=================================================
Files 509 509
Lines 22684 22688 +4
Branches 6743 7553 +810
=================================================
+ Hits 9137 9141 +4
Misses 13509 13509
Partials 38 38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -43,17 +43,15 @@ import {useContext} from "react"; | |||
| import {Immutable} from "immer"; | |||
|
|
|||
| export interface UseUserContextReturnType { | |||
| stage: STAGE; | |||
| contexts: UserContext[]; | |||
There was a problem hiding this comment.
This doesn't feel neat, but we can't just convert the existing stage/exam board properties to arrays because we need to keep stages and exam boards paired up correctly (e.g. if a user has AQA GCSE & OCR A-Level set). We also only need one explanation (etc), so having a list of contexts inside UseUserContextReturnType instead of having the hook return a list of UseUserContextReturnType seems sensible?
| if ((isPhy || explanation.examBoard === CONTEXT_SOURCE.DEFAULT) && explanation.stage === CONTEXT_SOURCE.DEFAULT && registeredContexts?.length) { | ||
| const registeredContextsWithExamBoards = isPhy ? registeredContexts.map(rc => ({stage: rc.stage, examBoard: EXAM_BOARD.ALL})) : registeredContexts; | ||
| contexts.push(...registeredContextsWithExamBoards); | ||
| explanation.stage = CONTEXT_SOURCE.REGISTERED; |
There was a problem hiding this comment.
Because we need to keep the stages/exam boards paired up for multiple registered contexts, we can't keep this in the original else if and push stages/exam boards as we encounter them.
|
@mwtrew - I've looked over your suggestion and agree it's a much nicer approach. There are a few test failures on that branch right now, but once that's sorted I'm happy for those changes to be merged into this. The only other thing I noticed is that that branch is hardcoding |
(Ada): If a teacher has multiple registered contexts set, they should see content highlighted as relevant to them if it is relevant to any of their contexts (we currently just check the first).