Skip to content
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.

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

Ada core and advanced tagging #1014

Merged
merged 13 commits into from
Jul 23, 2024
Merged

Conversation

skyepurchase
Copy link
Contributor

If a topic or concept is not on a users specification display the tag as "core" or "advanced" based on prior tagging. This prefers "core" over "advanced" and only shows "advanced" if "core" is not present.

If a user has not set their preferences every tag is displayed as "core" or "advanced". To accomodate this, if a user has not set their preferences , on Ada, they have a new "none" stage and examboard to distinguish from deliberately selected "all".

If a topic or concept is not on a users specification display the tag as
"core" or "advanced" based on prior tagging. This prefers "core" over
"advanced" and only shows "advanced" if "core" is not present.

If a user has not set their preferences every tag is displayed as "core"
or "advanced". To accomodate this, if a user has not set their preferences
, on Ada, they have a new "none" stage and examboard to distinguish from
deliberately selected "all".
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 25.80645% with 46 lines in your changes missing coverage. Please review.

Project coverage is 37.12%. Comparing base (4c6a300) to head (efd13c4).
Report is 7 commits behind head on master.

Files Patch % Lines
src/app/services/userViewingContext.ts 22.22% 13 Missing and 1 partial ⚠️
...onents/elements/inputs/UserContextAccountInput.tsx 43.75% 9 Missing ⚠️
src/app/components/content/IsaacAccordion.tsx 0.00% 4 Missing ⚠️
...ponents/elements/list-groups/TopicSummaryLinks.tsx 0.00% 4 Missing ⚠️
src/app/components/pages/RegistrationAgeCheck.tsx 0.00% 3 Missing ⚠️
...rc/app/components/pages/RegistrationSetDetails.tsx 0.00% 3 Missing ⚠️
...pp/components/pages/RegistrationTeacherConnect.tsx 25.00% 3 Missing ⚠️
src/app/components/elements/Accordion.tsx 0.00% 1 Missing ⚠️
...p/components/elements/inputs/UserContextPicker.tsx 0.00% 1 Missing ⚠️
...ements/list-groups/ContentSummaryListGroupItem.tsx 0.00% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1014      +/-   ##
==========================================
- Coverage   37.16%   37.12%   -0.05%     
==========================================
  Files         420      420              
  Lines       18486    18532      +46     
  Branches     5424     6085     +661     
==========================================
+ Hits         6871     6880       +9     
+ Misses      11576    11033     -543     
- Partials       39      619     +580     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Discrepancies between accordions and topic summary links meant the two
behaved differently when they should behave the same.

The logic for deciding colour and label has been pushed in
`stringifyAudience` to keep behaviour consistent. Additionally, support
for intentionally chosen Core and Advanced stages has been added.
# Conflicts:
#	src/app/components/elements/inputs/UserContextAccountInput.tsx
@@ -53,7 +54,7 @@ function UserContextRow({
const onlyOneAtThisStage = existingUserContexts.length === 0 || existingUserContexts.filter(uc => uc.stage === e.target.value).length === 1;
const possibleExamBoards = getFilteredExamBoardOptions(
{byStages: [stage || STAGE.ALL], byUserContexts: existingUserContexts, includeNullOptions: onlyOneAtThisStage
}) || [EXAM_BOARD.ALL];
}) || [EXAM_BOARD.ADA];
Copy link
Member

Choose a reason for hiding this comment

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

The method getFilteredExamBoardOptions returns an array of {label: string, value: EXAM_BOARD} objects, but then we have this array that just contains EXAM_BOARD.ADA and is thus a completely different type. This isn't a new problem, but I do think we should fix it.

@jsharkey13
Copy link
Member

I merged the Bootstrap changes into this, and I think I resolved the messy conflict correctly. However, there are type warnings in my IDE, which I think are valid and I have commented about above.

@skyepurchase skyepurchase force-pushed the feature/core_and_advanced_support branch from 53dc0f2 to a351e57 Compare July 23, 2024 11:01
@mwtrew mwtrew merged commit 78e1df2 into master Jul 23, 2024
4 checks passed
@mwtrew mwtrew deleted the feature/core_and_advanced_support branch July 23, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants