Create `/test/view` pages #1396

barna-isaac · 2025-04-11T14:04:41Z

This extends the existing /test/* family of pages. Unlike /test/attempt, this just shows a description of the test, without adding the test to "My Tests". Unlike /test/preview, this is available to all logged in users, not just teachers. (We still don't allow anonymous users to view tests -- this is how the API endpoint currently works). I think it'd be fine to show this page to anonymous users as well in the future.

This PR also includes a change to the way these components handle navigation. Prior to this PR, when navigating around tests, target URL's would replace the current url in the user's browsing history. So, a user arriving from Bluesky, who then clicks Take Test, (they're on Sections) then clicks Continue, (they're on Page 1), then clicks Next (they're on Page 2), and then uses the browser's navigate backwords functionality (either by using the button, a keyboard shorcut, a mouse gesture or a touch gesture on a phone), would then be taken back to Bluesky instead of Page 1. I think most users would expect to be taken to Page 1 from Page 2. After this PR, the user would be taken to Page 1 from page 2.

This PR also includes new component test for all three pages, although the tests for /preview and /attempt are far from compehensive. I've also started using page-specific helpers, and I welcome any comments regarding where they should be, or indeed whether we should have them at all. I'm also interested in takes regarding the new component tests, the code style I employed, as well as whether some of them may be redundant. I think they're quite fast, take up just a few lines, and are written to only break when there's an actual change to functionality -- in other words, I'm hoping they'll only give us trouble when we break something.

On this page, users will be able to view a test's short description (rubric). We have two similar pages, but - `/preview` is only available to teachers - `/attempt` commits the user to completing the test

This could affect the `/view` and `/attempt` pages as well, but I think each quiz has at least one section. Even if there were tests without sections, it'd make no sense to show a header and then not list any sections. The `QuizView` component always passes an empty array for sections, so the outcome is that sections are hidden from that component only.

- remove "Click button when you are ready" message - "Start" button should always just say continue. It was misleading that this ever said "Start", because by the time the user saw this page, the test has already been added to "My Practice Tests"

What used to be two buttons is now just one. From "View Test", teachers can decide whether to take or preview the test. Students only ever had the opportunity to take a test.

this ensures there's a margin between the "You are viewing..." text and the "Instructions" heading. Without it, the margin is enforced by the "Set Test" button, which we only show for teachers. The margin was missing for students.

none if these components are memoized, so it doesn't matter that we pass a different function each time

I'm hoping, in the future, we can extract similar helpers from other tests as well.

codecov · 2025-04-11T14:17:43Z

Codecov Report

Attention: Patch coverage is 95.20548% with 7 lines in your changes missing coverage. Please review.

Project coverage is 40.81%. Comparing base (c7a7f43) to head (2fdd92a).
Report is 33 commits behind head on master.

Files with missing lines	Patch %	Lines
...components/elements/quiz/QuizContentsComponent.tsx	92.68%	3 Missing ⚠️
src/test/testUtils.tsx	88.00%	3 Missing ⚠️
src/app/services/tagsPhy.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1396      +/-   ##
==========================================
+ Coverage   38.87%   40.81%   +1.93%     
==========================================
  Files         456      460       +4     
  Lines       19916    20017     +101     
  Branches     6514     6544      +30     
==========================================
+ Hits         7743     8169     +426     
+ Misses      11549    11241     -308     
+ Partials      624      607      -17

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jacbn

Few small issues, but the main point for me is keeping the API types consistent with how they actually are in the API. Have made some comments about the redesign too here, only because I can comment in the right place, but they ought to refer to #1400. Works really well though!

src/IsaacApiTypes.tsx

src/app/components/elements/quiz/builErrorComponent.tsx

src/test/pages/QuizView.test.tsx

src/test/testUtils.tsx

src/app/components/elements/quiz/QuizAttemptComponent.tsx

src/app/components/pages/quizzes/QuizView.tsx

src/app/components/elements/quiz/builErrorComponent.tsx

src/app/components/pages/quizzes/QuizView.tsx

, because `DetailedQuizSummaryDTO` is an actual type that exists on the API, whereas `IsaacRubricDTO` was a type I just made up. This became a larger refactor because DetailedQuizSummaryDTO highlighted a lot of type incompatibilites that I didn't encounter using `IsaacRubricDTO`.

API only specifies that `ContentDTO['children']` be `ContentBaseDTO`, so I chose not to assume more (following a code review comment about this by Jaycie). The specific test objects are narrower (they have more properties than specified by these types). I've introduced a new `recordOf` function that I use here to assert that the mocks at least implement the interface. I chose this over introducing new narrower types specifically for just the test fixtures.

it's now also used for `QuizView`

`QuizDetails` is only ever shown for attempts, which always have sections.

barna-isaac · 2025-04-16T16:25:35Z

Same as I wrote on #1400, I think this is ready for a re-review!

jacbn

This looks good! One thing I forgot to mention in the original review was that it's a little weird to me that the buttons at the bottom of the /view page redirect to the equivalent /preview or /attempt pages, which are pretty much identical pages bar the sentence at the top, rather than the first page of the test. I would suppose this is because the /attempt page has some API request attached that the test pages don't, so if we did want to change it I would suggest doing it as a separate piece of work. Regardless I think this is great for now!

jacbn · 2025-04-17T14:21:37Z

src/mocks/utils.ts

@@ -0,0 +1,2 @@
+// this makes sure `elem` implements an interface without widening its type
+export const recordOf = <T extends string | number | symbol, U>() => <V extends U>(elem: Record<T, V>) => elem;


I've had a play but can't find an equivalent type for this. You need an assertion that the inferred type extends the type parameter, and you can do the opposite with infer but not this way around. I like this for now so let's keep it :)

src/app/components/elements/quiz/QuizAttemptComponent.tsx

barna-isaac · 2025-04-17T15:55:14Z

This looks good! One thing I forgot to mention in the original review was that it's a little weird to me that the buttons at the bottom of the /view page redirect to the equivalent /preview or /attempt pages, which are pretty much identical pages bar the sentence at the top, rather than the first page of the test. I would suppose this is because the /attempt page has some API request attached that the test pages don't, so if we did want to change it I would suggest doing it as a separate piece of work. Regardless I think this is great for now!

They're similar, but there are more differences: both have the sections, and for an attempted question, these also show the student's progress across sections -- if we'd like to skip the /attempts page, maybe we should also implement this on /view. And yeah it's also what you said about needing to start the attempt.

barna-isaac added 21 commits April 9, 2025 15:24

add QuizView page

9971fb1

On this page, users will be able to view a test's short description (rubric). We have two similar pages, but - `/preview` is only available to teachers - `/attempt` commits the user to completing the test

change breadcrumbs, actionMessage on QuizView

69dc490

clean up redundant tests

0c35462

show "Take Test" button

eaf12a2

allow navigation between the view and attempt pages

e61fad5

just a style fix

cb69193

show Preview button for teachers and above

ab5b131

simplify QuizAttemptFooter

900b1e8

- remove "Click button when you are ready" message - "Start" button should always just say continue. It was misleading that this ever said "Start", because by the time the user saw this page, the test has already been added to "My Practice Tests"

take user to "View Test" from "Practice Tests"

dc8c6c1

What used to be two buttons is now just one. From "View Test", teachers can decide whether to take or preview the test. Students only ever had the opportunity to take a test.

undo change I comitted by accident

2671a18

refactor: extract error response from handlers

ea81ef8

prevent overflow on Take Test button

0fcd569

QuizView works on Ada

a9bc488

fix spacing

ec8a23f

this ensures there's a margin between the "You are viewing..." text and the "Instructions" heading. Without it, the margin is enforced by the "Set Test" button, which we only show for teachers. The margin was missing for students.

a minor refactor

f5eff81

none if these components are memoized, so it doesn't matter that we pass a different function each time

Continue on QuizDoFreeAttempt allows going back

44d9f22

quiz attempt pages always allow navigating back

f61741f

allow backward navigation when previewing

c9ed9b1

extract Quiz page helpers

8786913

I'm hoping, in the future, we can extract similar helpers from other tests as well.

apply Ada-specific casing differences

b0b62c9

barna-isaac mentioned this pull request Apr 14, 2025

Create /test/view pages on redesign #1400

Merged

jacbn requested changes Apr 15, 2025

View reviewed changes

barna-isaac added 6 commits April 15, 2025 14:45

refactor: fix typo in filename

f7c7118

fix editing mistake in tooltip text

2710697

simplify QuizAttemptComponent

a12c43d

rename QuizAttemptComponent

93b66da

it's now also used for `QuizView`

remove anySections check in QuizContents

2fdd92a

`QuizDetails` is only ever shown for attempts, which always have sections.

barna-isaac requested a review from jacbn April 16, 2025 15:49

jacbn approved these changes Apr 17, 2025

View reviewed changes

jacbn merged commit dbc5ae9 into master Apr 17, 2025
8 checks passed

jacbn deleted the feature/quiz-view branch April 17, 2025 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Create `/test/view` pages #1396

Create `/test/view` pages #1396

Uh oh!

barna-isaac commented Apr 11, 2025 •

edited

Loading

Uh oh!

codecov bot commented Apr 11, 2025 •

edited

Loading

Uh oh!

jacbn left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

barna-isaac commented Apr 16, 2025

Uh oh!

jacbn left a comment

Uh oh!

jacbn Apr 17, 2025

Uh oh!

Uh oh!

Uh oh!

barna-isaac commented Apr 17, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,2 @@
		// this makes sure `elem` implements an interface without widening its type
		export const recordOf = <T extends string \| number \| symbol, U>() => <V extends U>(elem: Record<T, V>) => elem;

Create /test/view pages #1396

Create /test/view pages #1396

Uh oh!

Conversation

barna-isaac commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jacbn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

barna-isaac commented Apr 16, 2025

Uh oh!

jacbn left a comment

Choose a reason for hiding this comment

Uh oh!

jacbn Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

barna-isaac commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Create `/test/view` pages #1396

Create `/test/view` pages #1396

barna-isaac commented Apr 11, 2025 •

edited

Loading

codecov bot commented Apr 11, 2025 •

edited

Loading

barna-isaac commented Apr 17, 2025 •

edited

Loading