Split out validation logic for cs-program #1688

Myranae · 2024-09-27T15:22:09Z

Summary:

For the server-side scoring project, we are moving validation logic out of each widget component. This moves the validation logic for the cs-program widget and adds tests for the validator function.

Issue: LEMS-2353

Test plan:

Confirm all tests pass
Nothing should change. Code is only being moved.

Myranae · 2024-09-27T15:24:20Z

packages/perseus/src/widgets/cs-program/cs-program-validator.ts

+export function csProgramValidator(
+    state: PerseusCSProgramUserInput,
+): PerseusScore {


rubric was not being used, so it was removed as a parameter.

This is one of those sus things we're going to have to figure out in the future. Can you check to make sure there's a ticket for this? Something along the lines on "some widgets don't use rubric for validation"? I imagine all the program/iframe widgets do this.

Here's the relevant ticket: https://khanacademy.atlassian.net/browse/LEMS-2440

github-actions · 2024-09-27T15:24:31Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1688

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

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

github-actions · 2024-09-27T15:24:34Z

Size Change: +4 B (0%)

Total Size: 863 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	417 kB	+4 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.3 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`	78 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`	279 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-09-27T15:31:12Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/strong-numbers-pretend.md, packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts, packages/perseus/src/widgets/cs-program/cs-program-validator.ts, packages/perseus/src/widgets/cs-program/cs-program.tsx

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

handeyeco

Thanks!

handeyeco · 2024-09-27T15:38:07Z

packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts

+        expect(result).toEqual({
+            type: "points",
+            earned: 0,
+            total: 1,
+            message: "Try again!",
+        });


If it's important that we check the message (and I'm not sure it is), I think we should update toHaveBeenAnsweredIncorrectly to optionally handle a message like toHaveInvalidInput. Otherwise I think we should switch this to toHaveBeenAnsweredIncorrectly.

🤦‍♀️ Yep, totally meant to do that. Not sure how I skipped this. Thanks for catching it!

handeyeco · 2024-09-27T15:39:32Z

packages/perseus/src/widgets/cs-program/cs-program-validator.ts

+export function csProgramValidator(
+    state: PerseusCSProgramUserInput,
+): PerseusScore {


This is one of those sus things we're going to have to figure out in the future. Can you check to make sure there's a ticket for this? Something along the lines on "some widgets don't use rubric for validation"? I imagine all the program/iframe widgets do this.

jeremywiebe · 2024-09-27T20:24:51Z

packages/perseus/src/widgets/cs-program/cs-program.tsx

+        return csProgramValidator({
+            status: this.props.status,
+            message: this.props.message,


The data we pass to the csProgramValidator should be extracted into a getUserInput function. This will help us as we split the scoring logic further out of the renderer system.

You can see this pattern in play in many of our widgets (eg grapher or number-line)

Could you do this in a new PR?

Here's the ticket for that: https://khanacademy.atlassian.net/browse/LEMS-2459

@nishasy

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/perseus@35.1.0 ### Minor Changes - [#1687](#1687) [`c0cbb484a`](c0cbb48) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked function aria labels (graph + editor) - [#1690](#1690) [`a65da60c2`](a65da60) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked polygon labels - [#1683](#1683) [`2d7cadaf0`](2d7cada) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked line aria label behavior on graph - [#1691](#1691) [`fc16bc77f`](fc16bc7) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked polygon aria labels (graph + editor) - [#1677](#1677) [`3c73f4aa4`](3c73f4a) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked point aria label behavior on graph - [#1684](#1684) [`17ebfc0de`](17ebfc0) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked line aria label editor UI - [#1685](#1685) [`b3de0d80e`](b3de0d8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked vector aria labels (graph + editor) - [#1689](#1689) [`eda436f00`](eda436f) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked polygon labels - [#1682](#1682) [`039e0a360`](039e0a3) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked point aria label editor UI - [#1686](#1686) [`8230d96ee`](8230d96) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked ellipse aria labels (graph + editor) ### Patch Changes - [#1702](#1702) [`30725ec88`](30725ec) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out validation logic for NumberLine - [#1671](#1671) [`f326139ee`](f326139) Thanks [@handeyeco](https://github.com/handeyeco)! - Move and test Grapher's validator - [#1704](#1704) [`82e0ebd71`](82e0ebd) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out validation logic from table - [#1664](#1664) [`c41e4b2f3`](c41e4b2) Thanks [@benchristel](https://github.com/benchristel)! - Omit unused data from interactive graph onChange callback - [#1678](#1678) [`49efaaff5`](49efaaf) Thanks [@handeyeco](https://github.com/handeyeco)! - Port some tests to new custom matcher - [#1676](#1676) [`f5af24371`](f5af243) Thanks [@benchristel](https://github.com/benchristel)! - Internal: remove dead code from InteractiveGraph.validate() - [#1698](#1698) [`339a6db38`](339a6db) Thanks [@handeyeco](https://github.com/handeyeco)! - Refine Expression's Rubric type - [#1700](#1700) [`493715e3d`](493715e) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out InteractiveGraph validator - [#1705](#1705) [`e432666fd`](e432666) Thanks [@Myranae](https://github.com/Myranae)! - Move validation logic out of the iframe widget and add tests - [#1693](#1693) [`466d010c6`](466d010) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add color select to locked figure labels settings - [#1707](#1707) [`d3767f720`](d3767f7) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out matcherValidator from Matcher - [#1670](#1670) [`463755970`](4637559) Thanks [@handeyeco](https://github.com/handeyeco)! - Split validation logic out of Matrix - [#1688](#1688) [`10ce86925`](10ce869) Thanks [@Myranae](https://github.com/Myranae)! - Move validation logic out of the cs-program widget and add tests - [#1703](#1703) [`e818b0f15`](e818b0f) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out Plotter validator ## @khanacademy/perseus-editor@14.6.0 ### Minor Changes - [#1687](#1687) [`c0cbb484a`](c0cbb48) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked function aria labels (graph + editor) - [#1690](#1690) [`a65da60c2`](a65da60) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked polygon labels - [#1683](#1683) [`2d7cadaf0`](2d7cada) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked line aria label behavior on graph - [#1691](#1691) [`fc16bc77f`](fc16bc7) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked polygon aria labels (graph + editor) - [#1677](#1677) [`3c73f4aa4`](3c73f4a) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked point aria label behavior on graph - [#1684](#1684) [`17ebfc0de`](17ebfc0) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked line aria label editor UI - [#1685](#1685) [`b3de0d80e`](b3de0d8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked vector aria labels (graph + editor) - [#1689](#1689) [`eda436f00`](eda436f) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked polygon labels - [#1682](#1682) [`039e0a360`](039e0a3) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked point aria label editor UI - [#1686](#1686) [`8230d96ee`](8230d96) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked ellipse aria labels (graph + editor) ### Patch Changes - [#1706](#1706) [`624be8143`](624be81) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Aria] Use TextArea instead of TextField in the locked figures aria settings - [#1693](#1693) [`466d010c6`](466d010) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add color select to locked figure labels settings - Updated dependencies \[[`30725ec88`](30725ec), [`c0cbb484a`](c0cbb48), [`f326139ee`](f326139), [`a65da60c2`](a65da60), [`82e0ebd71`](82e0ebd), [`c41e4b2f3`](c41e4b2), [`2d7cadaf0`](2d7cada), [`49efaaff5`](49efaaf), [`fc16bc77f`](fc16bc7), [`f5af24371`](f5af243), [`339a6db38`](339a6db), [`3c73f4aa4`](3c73f4a), [`17ebfc0de`](17ebfc0), [`493715e3d`](493715e), [`e432666fd`](e432666), [`466d010c6`](466d010), [`b3de0d80e`](b3de0d8), [`eda436f00`](eda436f), [`d3767f720`](d3767f7), [`463755970`](4637559), [`10ce86925`](10ce869), [`039e0a360`](039e0a3), [`8230d96ee`](8230d96), [`e818b0f15`](e818b0f)]: - @khanacademy/perseus@35.1.0 Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ⏭️ Publish npm snapshot, ✅ Cypress (ubuntu-latest, 20.x), ✅ 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: #1680

Myranae added 7 commits September 27, 2024 10:09

Split out validate logic

84846b9

Add related types to a new file

1fd65fb

Add validator tests

7b65c8c

Merge branch 'main' into tb/LEMS-2353/split-out-cs-program-validation

cdd0267

Remove types file after merging validation types

521b2d2

Re-split and update logic after merge

69e33ed

Update tests with new validation types

848ff14

Myranae self-assigned this Sep 27, 2024

Myranae commented Sep 27, 2024

View reviewed changes

Add changeset

40ef5aa

Myranae marked this pull request as ready for review September 27, 2024 15:30

Myranae requested a review from handeyeco September 27, 2024 15:30

khan-actions-bot requested a review from a team September 27, 2024 15:31

handeyeco approved these changes Sep 27, 2024

View reviewed changes

Refactor test

5e36cc1

Myranae merged commit 10ce869 into main Sep 27, 2024
9 checks passed

Myranae deleted the tb/LEMS-2353/split-out-cs-program-validation branch September 27, 2024 16:08

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

Version Packages #1680

Merged

jeremywiebe reviewed Sep 27, 2024

View reviewed changes

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

Version Packages #1709

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split out validation logic for cs-program #1688

Split out validation logic for cs-program #1688

Myranae commented Sep 27, 2024

Myranae Sep 27, 2024

handeyeco Sep 27, 2024

Myranae Sep 27, 2024

github-actions bot commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

khan-actions-bot commented Sep 27, 2024

handeyeco left a comment

handeyeco Sep 27, 2024

Myranae Sep 27, 2024

handeyeco Sep 27, 2024

jeremywiebe Sep 27, 2024

Myranae Sep 27, 2024

Split out validation logic for cs-program #1688

Split out validation logic for cs-program #1688

Conversation

Myranae commented Sep 27, 2024

Summary:

Test plan:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 27, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Sep 27, 2024 • edited Loading

khan-actions-bot commented Sep 27, 2024

Gerald

handeyeco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading