Dropdown: Extract validation out of scoring #1898

Myranae · 2024-11-21T16:13:45Z

Summary:

To complete server-side scoring, we are separating out validation logic from scoring logic. This PR separates that logic and updates associated tests.

Issue: LEMS-2597

Test plan:

Confirm all checks pass
Confirm widget still works as expected

github-actions · 2024-11-21T16:16:30Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1898

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

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

github-actions · 2024-11-21T16:17:06Z

Size Change: +77 B (+0.01%)

Total Size: 1.29 MB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	421 kB	+77 B (+0.02%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39 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.9 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`	698 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.68 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.5 kB

_{compressed-size-action}

khan-actions-bot · 2024-11-21T16:21:55Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/famous-horses-grab.md, packages/perseus/src/widgets/dropdown/score-dropdown.test.ts, packages/perseus/src/widgets/dropdown/score-dropdown.ts, packages/perseus/src/widgets/dropdown/validate-dropdown.test.ts, packages/perseus/src/widgets/dropdown/validate-dropdown.ts

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

Myranae · 2024-11-21T16:22:50Z

packages/perseus/src/widgets/dropdown/score-dropdown.test.ts

-    it("returns invalid for user input of 0", () => {
-        // Arrange
-        const userInput: PerseusDropdownUserInput = {
-            value: 0,
-        };
-        const rubric: PerseusDropdownRubric = {
-            choices: question1.widgets["dropdown 1"].options.choices,
-        };
-
-        // Act
-        const result = scoreDropdown(userInput, rubric);
-
-        // Assert
-        expect(result).toHaveInvalidInput();
-    });


Didn't keep a check for this in scoring test since it looks like we might be doing validation before scoring (love that)

jeremywiebe · 2024-11-21T22:51:50Z

packages/perseus/src/widgets/dropdown/validate-dropdown.test.ts

+import type {PerseusDropdownUserInput} from "../../validation.types";
+
+describe("validateDropdown", () => {
+    it("returns invalid for invalid input (user input of 0)", () => {


Tiny improvement suggestion as 0 is the selected index. 🤷‍♂️ You decide. :)

Suggested change

it("returns invalid for invalid input (user input of 0)", () => {

it("returns invalid when nothing selected (user input of 0)", () => {

jeremywiebe · 2024-11-21T22:56:13Z

packages/perseus/src/widgets/dropdown/validate-dropdown.ts

+ * This is shown with a userInput value / index other than 0.
+ * @param userInput


I would suggest removing these two lines:

The first one documents the internal implementation of the function (how it checks validation) which you can see by reading the code. It also presents something that can easily get out of sync with the code. A great guide for comments that I was told one time was "document why, not what". :)

The @param userInput doesn't really add any value.

…ng` to `main` (#1926) ## Summary: This PR includes the following commits: - Numeric-Input: Extract validation from scorer (#1882) - Orderer: Extract validation out of scoring (#1869) - Sorter: Extract validation out of scoring (#1876) - Dropdown: Extract validation out of scoring (#1898) - Plotter: Extract validation out of scoring (#1899) - Radio: Extract validation out of scoring (#1902) - Categorizer: Extract validation out of scoring (#1862) Issue: <none> ## Test plan: Author: jeremywiebe Reviewers: lillialexis, #perseus Required Reviewers: Approved By: lillialexis Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ 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, ✅ Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald, ✅ .github/dependabot.yml Pull Request URL: #1926

@Myranae

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] ### Minor Changes - [#1898](#1898) [`3a9b5921b`](3a9b592) Thanks [@Myranae](https://github.com/Myranae)! - Introduces a validation function for the dropdown widget (extracted from dropdown scoring function). - [#1862](#1862) [`451de899f`](451de89) Thanks [@Myranae](https://github.com/Myranae)! - Split out validation function for the `categorizer` widget. This can be used to check if the user selected an answer for every row, confirming the question is ready to be scored. - [#1882](#1882) [`40d2ebb75`](40d2ebb) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Introduces a validation function for the numeric-input widget (extracted from numeric-input scoring function). - [#1899](#1899) [`2437ce61b`](2437ce6) Thanks [@Myranae](https://github.com/Myranae)! - Introduces a validation function for the plotter widget (extracted from the scoring function). - [#1869](#1869) [`f43edd42c`](f43edd4) Thanks [@Myranae](https://github.com/Myranae)! - Split out validation function for the `orderer` widget. This can be used to check if the user has ordered at least one option, confirming the question is ready to be scored. - [#1902](#1902) [`0cec7628c`](0cec762) Thanks [@Myranae](https://github.com/Myranae)! - Introduces a validation function for the radio widget (extracted from the scoring function), though not all validation logic can be extracted. - [#1876](#1876) [`0bd4270ad`](0bd4270) Thanks [@Myranae](https://github.com/Myranae)! - Split out validation function for the `sorter` widget. This can be used to check if the user has made any changes to the sorting order, confirming whether or not the question can be scored. ### Patch Changes - [#1907](#1907) [`3dbca965a`](3dbca96) Thanks [@benchristel](https://github.com/benchristel)! - Internal: add and pass regression tests for `PerseusItem` parser. - [#1915](#1915) [`ee09e9fc0`](ee09e9f) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove use of findDOMNode in number-input component - [#1920](#1920) [`88ba71bef`](88ba71b) Thanks [@handeyeco](https://github.com/handeyeco)! - Fix some file-wide error suppressions - [#1919](#1919) [`64ea2ee86`](64ea2ee) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove usage of findDOMNode in text-input component - [#1906](#1906) [`799ffe4a5`](799ffe4) Thanks [@anakaren-rojas](https://github.com/anakaren-rojas)! - update moveable point component and use control point method to have optional params - [#1895](#1895) [`841551a65`](841551a) Thanks [@benchristel](https://github.com/benchristel)! - Internal: remove unused fields from `answerArea` when parsing `PerseusItem`s. - [#1921](#1921) [`8ec06f444`](8ec06f4) Thanks [@benchristel](https://github.com/benchristel)! - Internal: Inline widget version into Expression widget parser. - [#1914](#1914) [`3e98b7cd3`](3e98b7c) Thanks [@handeyeco](https://github.com/handeyeco)! - Add tests for propUpgrades functions (and remove underscore usage) - [#1908](#1908) [`7f2866cf4`](7f2866c) Thanks [@benchristel](https://github.com/benchristel)! - Internal: Migrate expression widget options to the latest version in parseAndTypecheckPerseusItem (not yet used in production). - Updated dependencies \[[`88ba71bef`](88ba71b)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - [#1920](#1920) [`88ba71bef`](88ba71b) Thanks [@handeyeco](https://github.com/handeyeco)! - Fix some file-wide error suppressions ## @khanacademy/[email protected] ### Patch Changes - [#1920](#1920) [`88ba71bef`](88ba71b) Thanks [@handeyeco](https://github.com/handeyeco)! - Fix some file-wide error suppressions - [#1863](#1863) [`584153588`](5841535) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Switch two corner usages of deprecated @khanacademy/wonder-blocks-spacing to @khanacademy/wonder-blocks-tokens - Updated dependencies \[[`3dbca965a`](3dbca96), [`ee09e9fc0`](ee09e9f), [`3a9b5921b`](3a9b592), [`88ba71bef`](88ba71b), [`64ea2ee86`](64ea2ee), [`451de899f`](451de89), [`40d2ebb75`](40d2ebb), [`799ffe4a5`](799ffe4), [`2437ce61b`](2437ce6), [`841551a65`](841551a), [`8ec06f444`](8ec06f4), [`f43edd42c`](f43edd4), [`0cec7628c`](0cec762), [`0bd4270ad`](0bd4270), [`3e98b7cd3`](3e98b7c), [`7f2866cf4`](7f2866c)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`88ba71bef`](88ba71b)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - [#1920](#1920) [`88ba71bef`](88ba71b) Thanks [@handeyeco](https://github.com/handeyeco)! - Fix some file-wide error suppressions ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`88ba71bef`](88ba71b)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Cypress (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1918

Myranae added 4 commits November 21, 2024 10:08

Add changeset

c718484

Split validation test and add null test

8eef008

Split out validation logic to own function

ef22f34

Merge branch 'main' into tb/LEMS-2597/dropdown-split-out-validation

1f04130

Myranae self-assigned this Nov 21, 2024

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

Myranae requested review from handeyeco and jeremywiebe November 21, 2024 16:21

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

Myranae commented Nov 21, 2024

View reviewed changes

Clarify valid state in doc comment

d5bb04c

jeremywiebe approved these changes Nov 21, 2024

View reviewed changes

jeremywiebe changed the base branch from main to release/server-side-scoring November 26, 2024 20:19

Just remove an unneeded JSDoc comment

e4d0921

jeremywiebe merged commit 3a9b592 into release/server-side-scoring Nov 26, 2024
9 checks passed

jeremywiebe deleted the tb/LEMS-2597/dropdown-split-out-validation branch November 26, 2024 20:44

jeremywiebe mentioned this pull request Nov 27, 2024

This is a feature-branch pull-request from release/server-side-scoring to main #1926

Merged

khan-actions-bot mentioned this pull request Nov 27, 2024

Version Packages #1918

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dropdown: Extract validation out of scoring #1898

Dropdown: Extract validation out of scoring #1898

Myranae commented Nov 21, 2024

github-actions bot commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

khan-actions-bot commented Nov 21, 2024

Myranae Nov 21, 2024

jeremywiebe Nov 21, 2024

jeremywiebe Nov 21, 2024

	it("returns invalid for invalid input (user input of 0)", () => {
	it("returns invalid when nothing selected (user input of 0)", () => {

		* This is shown with a userInput value / index other than 0.
		* @param userInput

Dropdown: Extract validation out of scoring #1898

Dropdown: Extract validation out of scoring #1898

Conversation

Myranae commented Nov 21, 2024

Summary:

Test plan:

github-actions bot commented Nov 21, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Nov 21, 2024 • edited Loading

khan-actions-bot commented Nov 21, 2024

Gerald

Myranae Nov 21, 2024

Choose a reason for hiding this comment

jeremywiebe Nov 21, 2024

Choose a reason for hiding this comment

jeremywiebe Nov 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading