Refine Radio's Rubric and UserInput types #1758

Myranae · 2024-10-16T21:08:12Z

Summary:

As part of the Server Side Scoring project, this refactors Radio's Rubric type to only contain what is necessary for scoring and UserInput to only contain info about the user's input. Also adds the types to Radio's validator tests.

Issue: LEMS-2471

Test plan:

Confirm all checks pass
Confirm Radio still works as expected via Storybook

And remove one item that isn't needed

github-actions · 2024-10-16T21:10:36Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (277954c) and published it to npm. You
can install it using the tag PR1758.

Example:

yarn add @khanacademy/perseus@PR1758

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1758

github-actions · 2024-10-16T21:10:42Z

Size Change: -13 B (0%)

Total Size: 866 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	419 kB	-13 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.8 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.8 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-editor/dist/es/index.js`	280 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.4 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

khan-actions-bot · 2024-10-16T21:19:29Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/odd-dancers-relax.md, packages/perseus/src/validation.types.ts, packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx, packages/perseus/src/widgets/group/group.test.tsx, packages/perseus/src/widgets/radio/radio-component.tsx, packages/perseus/src/widgets/radio/radio-validator.test.ts, packages/perseus/src/widgets/radio/radio-validator.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

handeyeco · 2024-10-16T22:03:49Z

packages/perseus/src/validation.types.ts


 export type PerseusRadioUserInput = {
+    /*TODO(LEMS-2541): countChoices seems to be necessary for rendering the
+       question, not scoring. Maybe move to renderProps?*/
    countChoices?: boolean;


So one way to look at the overall problem of RenderProps vs UserInput vs Rubric: is the thing in question used in the validator? Since countChoices isn't in the validator (or any of the functionality that the validator uses), it can't be in the Rubric (which is stuff we need from KA to validate the answer) or UserInput (which is stuff we need from the leaner to validate the answer). It's rendering logic, so RenderProps.

Then like numCorrect is a weird one:

Is there anything the learner can do to affect numCorrect and if so would that affect their score? No, so it's not UserInput.

Is numCorrect something KA determines and is it used to score? Yes, so it's part of the Rubric.

Is numCorrect something that determines how we render the component? Yes, so it needs to be part of RenderProps.

There's some overlap between all of this, but that's okay. What's important (for this leg of the work) is that we know exactly what shape the Rubric and UserInput is.

Totally makes sense. I wanted to fix the UserInput so countChoices is not included, but that requires some logic refactoring, which is why I created a separate ticket for it. Should I just do the refactoring as part of the type update here?

I went through the code to see where removing countChoices from Radio's user input would require a refactor, but didn't see it. I asked Jeremy to follow-up on this today so you have coverage while I'm gone.

I think I was getting some weird errors the first time, but I was able to follow through with removing countChoices from the UserInput type, so that's been added here.

catandthemachines

Looks good to me!

catandthemachines · 2024-10-17T10:08:30Z

packages/perseus/src/widgets/radio/radio-validator.test.ts

            choicesSelected: [false, false, false, false, true],
            noneOfTheAboveSelected: true,
            noneOfTheAboveIndex: 4,
        };

-        const rubric = {
+        const rubric: PerseusRadioRubric = {


I love all this typing. Thank you!

handeyeco

Thank you

handeyeco · 2024-10-28T14:42:09Z

packages/perseus/src/widgets/radio/radio-validator.ts

@@ -21,7 +21,7 @@ function radioValidator(
        };
    }

-    // TODO: should numCorrect actually be on the rubric
+    // TODO(LEMS-2541): should numCorrect actually be on the rubric


Thanks for filing this ticket. I think numCorrect, noneOfTheAboveIndex, and noneOfTheAboveSelected are all problematic, but I don't think we have to address it in this PR. It should probably be a fast follow though.

# Conflicts: # packages/perseus/src/validation.types.ts

@jeremywiebe

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Patch Changes - [#1799](#1799) [`f3139edfe`](f3139ed) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change functional components to use default parameters instead of deprecated 'defaultProps' ## @khanacademy/[email protected] ### Patch Changes - [#1766](#1766) [`39e1292a9`](39e1292) Thanks [@Myranae](https://github.com/Myranae)! - Refine iFrame's Rubric type - [#1799](#1799) [`f3139edfe`](f3139ed) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change functional components to use default parameters instead of deprecated 'defaultProps' - [#1765](#1765) [`5cf8d975b`](5cf8d97) Thanks [@Myranae](https://github.com/Myranae)! - Refine Sorter's Rubric type - [#1758](#1758) [`d6edf18ef`](d6edf18) Thanks [@Myranae](https://github.com/Myranae)! - Refine Radio's Rubric and UserInput types - [#1743](#1743) [`5ea5d5927`](5ea5d59) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - BUGFIX - [Numeric Input] - Check for wrong answers when scoring - [#1760](#1760) [`9426509cd`](9426509) Thanks [@Myranae](https://github.com/Myranae)! - Refine Matrix's Rubric and UserInput types - [#1761](#1761) [`dbe17d1ee`](dbe17d1) Thanks [@Myranae](https://github.com/Myranae)! - Refine NumericInput's Rubric type - Updated dependencies \[[`f3139edfe`](f3139ed)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - [#1794](#1794) [`9dd0f8c56`](9dd0f8c) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove unused prop, optionRenderer, from dropdown-option component - Updated dependencies \[[`39e1292a9`](39e1292), [`f3139edfe`](f3139ed), [`5cf8d975b`](5cf8d97), [`d6edf18ef`](d6edf18), [`5ea5d5927`](5ea5d59), [`9426509cd`](9426509), [`dbe17d1ee`](dbe17d1)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`f3139edfe`](f3139ed)]: - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ⏭️ Publish npm snapshot, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1800

Myranae added 4 commits October 16, 2024 15:55

Add types to test

fba00dc

And remove one item that isn't needed

Refine Rubric type to what is needed to score

2c2df85

Update TODOs with ticket reference

7d37ff7

Add changeset

6950cc8

Myranae self-assigned this Oct 16, 2024

Myranae marked this pull request as ready for review October 16, 2024 21:19

Myranae requested a review from handeyeco October 16, 2024 21:19

khan-actions-bot requested a review from a team October 16, 2024 21:19

handeyeco reviewed Oct 16, 2024

View reviewed changes

catandthemachines approved these changes Oct 17, 2024

View reviewed changes

Remove countChoices from userInput

4ede900

khan-actions-bot requested a review from a team October 17, 2024 16:46

Myranae requested a review from handeyeco October 17, 2024 16:52

Update changeset

480e653

Myranae changed the title ~~Refine Radio's Rubric type~~ Refine Radio's Rubric and UserInput types Oct 17, 2024

handeyeco approved these changes Oct 28, 2024

View reviewed changes

Myranae added 2 commits October 28, 2024 10:29

Merge branch 'main' into tb/LEMS-2471/radio-refine-types

1652070

Merge branch 'main' into tb/LEMS-2471/radio-refine-types

277954c

# Conflicts: # packages/perseus/src/validation.types.ts

Myranae merged commit d6edf18 into main Oct 28, 2024
9 checks passed

Myranae deleted the tb/LEMS-2471/radio-refine-types branch October 28, 2024 15:53

khan-actions-bot mentioned this pull request Oct 28, 2024

Version Packages #1800

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refine Radio's Rubric and UserInput types #1758

Refine Radio's Rubric and UserInput types #1758

Myranae commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

khan-actions-bot commented Oct 16, 2024 •

edited

Loading

handeyeco Oct 16, 2024 •

edited

Loading

Myranae Oct 17, 2024

handeyeco Oct 17, 2024

Myranae Oct 17, 2024

catandthemachines left a comment

catandthemachines Oct 17, 2024

handeyeco left a comment

handeyeco Oct 28, 2024

Refine Radio's Rubric and UserInput types #1758

Refine Radio's Rubric and UserInput types #1758

Conversation

Myranae commented Oct 16, 2024 • edited Loading

Summary:

Test plan:

github-actions bot commented Oct 16, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Oct 16, 2024 • edited Loading

khan-actions-bot commented Oct 16, 2024 • edited Loading

Gerald

handeyeco Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Myranae Oct 17, 2024

Choose a reason for hiding this comment

handeyeco Oct 17, 2024

Choose a reason for hiding this comment

Myranae Oct 17, 2024

Choose a reason for hiding this comment

catandthemachines left a comment

Choose a reason for hiding this comment

catandthemachines Oct 17, 2024

Choose a reason for hiding this comment

handeyeco left a comment

Choose a reason for hiding this comment

handeyeco Oct 28, 2024

Choose a reason for hiding this comment

Myranae commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

khan-actions-bot commented Oct 16, 2024 •

edited

Loading

handeyeco Oct 16, 2024 •

edited

Loading