Add workaround for graded during serialization#3597
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🗄️ Schema Change: No Changes ✅ |
🛠️ Item Splitting: No Changes ✅ |
|
Size Change: +8 B (0%) Total Size: 504 kB 📦 View Changed
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (7b40a8a) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3597If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3597If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3597 |
gradedgraded during serialization
ivyolamit
left a comment
There was a problem hiding this comment.
This change looks logical, since it's specific removal of graded, it will less likely have other side effects.
| // from the higher-up WidgetOptions (bad) | ||
| // See: LEMS-4108 and https://khanacademy.slack.com/archives/C01AZ9H8TTQ/p1778089642003609 | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| (filteredProps as any).graded = props.graded; |
There was a problem hiding this comment.
can we add a test coverage for this.
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@77.5.0 ### Minor Changes - [#3597](#3597) [`a51b5358a0`](a51b535) Thanks [@handeyeco](https://github.com/handeyeco)! - Deprecate excludeDenylistKeys and add workaround for `graded` ### Patch Changes - [#3593](#3593) [`7dd1ff6114`](7dd1ff6) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fix blue line on vector when graph becomes interactive after correct answer - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - [#3584](#3584) [`e5e24fb670`](e5e24fb) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Improving screen reader experience for pause/play button on image gifs. - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/kas@2.2.3 - @khanacademy/math-input@26.4.20 - @khanacademy/perseus-core@26.0.3 - @khanacademy/perseus-linter@5.0.3 - @khanacademy/perseus-score@8.8.2 - @khanacademy/pure-markdown@2.2.8 - @khanacademy/simple-markdown@2.2.3 - @khanacademy/keypad-context@3.2.48 - @khanacademy/kmath@2.4.6 ## @khanacademy/kas@2.2.3 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` ## @khanacademy/keypad-context@3.2.48 ### Patch Changes - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/perseus-core@26.0.3 ## @khanacademy/kmath@2.4.6 ### Patch Changes - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/perseus-core@26.0.3 ## @khanacademy/math-input@26.4.20 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/perseus-core@26.0.3 - @khanacademy/keypad-context@3.2.48 ## @khanacademy/perseus-core@26.0.3 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - [#3570](#3570) [`51dd982ad9`](51dd982) Thanks [@benchristel](https://github.com/benchristel)! - The Perseus parsers now guard against NaN and Infinity values in number fields. These values get converted to null by `JSON.stringify`, and previously caused the parsers to be non-idempotent. - Updated dependencies \[[`651efcc50b`](651efcc)]: - @khanacademy/kas@2.2.3 - @khanacademy/pure-markdown@2.2.8 ## @khanacademy/perseus-editor@31.2.2 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - [#3583](#3583) [`e6e297c793`](e6e297c) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (CX) | Handle null width/height in image editor - [#3597](#3597) [`a51b5358a0`](a51b535) Thanks [@handeyeco](https://github.com/handeyeco)! - Deprecate excludeDenylistKeys and add workaround for `graded` - Updated dependencies \[[`7dd1ff6114`](7dd1ff6), [`651efcc50b`](651efcc), [`e5e24fb670`](e5e24fb), [`a51b5358a0`](a51b535), [`51dd982ad9`](51dd982)]: - @khanacademy/perseus@77.5.0 - @khanacademy/kas@2.2.3 - @khanacademy/math-input@26.4.20 - @khanacademy/perseus-core@26.0.3 - @khanacademy/perseus-linter@5.0.3 - @khanacademy/perseus-score@8.8.2 - @khanacademy/keypad-context@3.2.48 - @khanacademy/kmath@2.4.6 ## @khanacademy/perseus-linter@5.0.3 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/kas@2.2.3 - @khanacademy/perseus-core@26.0.3 - @khanacademy/pure-markdown@2.2.8 - @khanacademy/kmath@2.4.6 ## @khanacademy/perseus-score@8.8.2 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - Updated dependencies \[[`651efcc50b`](651efcc), [`51dd982ad9`](51dd982)]: - @khanacademy/kas@2.2.3 - @khanacademy/perseus-core@26.0.3 - @khanacademy/kmath@2.4.6 ## @khanacademy/pure-markdown@2.2.8 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` - Updated dependencies \[[`651efcc50b`](651efcc)]: - @khanacademy/simple-markdown@2.2.3 ## @khanacademy/simple-markdown@2.2.3 ### Patch Changes - [#3546](#3546) [`651efcc50b`](651efcc) Thanks [@nishasy](https://github.com/nishasy)! - Add new lint rule to avoid using `as` and suppress lint errors for existing instances of `as` Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary:
The problem: in #3466 we added a switch for
graded. Everything was just dandy except that we broke publishes. That's because we started passinggradeddown to widgets (which we needed so IG could change its behavior based on thegradedflag) but then every widgets started gettinggraded. Because of the janky way we serialize, that started addinggradedin the WidgetOptions of some widgets (ticket and Slack convo). Rather than discard unneeded fields, the publish pipeline just explodes when it sees a field it doesn't expect (which is totally cool and awesome) and it wasn't expectinggraded.The best solution is to make data serialization opt-in vs opt-out (see LEMS-4108). However that's probably a medium risk change and we have a playtest today, so this change just skips around the denylist for
graded.Why this should be more safe than the last PR: in #3466 there were some red flags...namely that I had to add
gradedto a bunch of tests. That was reverted as part of the bugfix (#3589) and the fact that I didn't need to add those changes back in this PR to pass checks indicated that this doesn't have the same problem as the last attempt.