Skip to content

Allow itemDataVersion to be null when parsing Perseus assessment item JSON #2321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 26, 2025

Conversation

benchristel
Copy link
Member

Summary:

There are a few content items that have itemDataVersion: null, and these are
causing parser errors in production because the parser doesn't like null.
This PR updates the parser to accept null values for itemDataVersion.
itemDataVersion is already marked deprecated, and can be undefined, so it
seems harmless to allow null as well.

Issue: none

Test plan:

pnpm test

@benchristel benchristel self-assigned this Mar 21, 2025
Copy link
Contributor

github-actions bot commented Mar 21, 2025

Size Change: +5 B (0%)

Total Size: 736 kB

Filename Size Change
packages/perseus-core/dist/es/index.js 32.2 kB +5 B (+0.02%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 39.7 kB
packages/keypad-context/dist/es/index.js 760 B
packages/kmath/dist/es/index.js 11.1 kB
packages/math-input/dist/es/index.js 77.5 kB
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-editor/dist/es/index.js 136 kB
packages/perseus-linter/dist/es/index.js 22.8 kB
packages/perseus-score/dist/es/index.js 20.7 kB
packages/perseus/dist/es/index.js 370 kB
packages/perseus/dist/es/strings.js 6.74 kB
packages/pure-markdown/dist/es/index.js 4.14 kB
packages/simple-markdown/dist/es/index.js 13.1 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Mar 21, 2025

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2321

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

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

Copy link
Contributor

@Myranae Myranae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! If it can be undefined already, then allowing null also seems fine. I don't know the current uses for itemDataVersion, maybe some sort of upgrading? Would be nice to confirm any usages still work with null, but I don't think it would really be any different than undefined, so may not be necessary.

Copy link
Contributor

@handeyeco handeyeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's cool you're finding these things. I wonder if it's worth filing tickets as we run into things like this that could likely just be removed (or maybe add it to the Confluence page for things to address during a backfill).

@@ -0,0 +1,111 @@
{
"question": {
"content": "\n [[☃ image 1]]\n\n##A shot after all\n\n*By Heather M. Meston*\n\n___\n\n1. I peered through the neglected bushes crowning the hill overlooking the field, a vicious debate raging in my mind.\n\n2. *To go or not to go?*\n\n3. *To try or not to try?*\n\n4. I winced as Marcus Fitzwilliam barreled into a blocking sled, 300 pounds of foam padding and metal. The sled flew back six feet.\n\n5. *I can’t compete with that,* I thought. I barely weighed a hundred pounds in snow boots. It would take a miracle for me to move that thing an inch.\n\n6. *Stop it, Jamila,* commanded my braver side. *You don’t need to be big to be a quarterback, just fast and smart. Marcus may be big, but his bad knee makes him slow.* My eyes drifted to a sleek form running sprints across the field. *Now, Hiro, on the other hand…he’s fast. Gotta watch out for him.*\n\n7. Before I could stop myself, I shoved through the bushes, my head held high even as my heart trembled.\n\n8. Donny, who’d lost two teeth playing last year, was the first to spy me. He completely missed the left-handed pass Raj sent his way, drawing Coach Franklin’s irritation. But before Coach could begin lecturing, Raj interrupted him to point out the intruder on the field.\n\n9. Me.\n\n10. Coach looked startled for a moment, then came to his senses. “Jamila! You looking for cheerleading tryouts? In the gym.”\n\n11. I thought of all the hours my brother Malik had spent practicing with me. I couldn’t disappoint him. “No, I’m here to try out. Er…that is…for football, I mean. For quarterback.” Smooth. So smooth.\n\n12. Raj laughed, Marcus grinned, and Coach’s mouth twisted like I'd just force-fed him a lemon \n\n13. “Well, you see,” he started, “It’s just that girls aren’t allowed on the football team. Too dangerous, you know.”\n\n14. *But it's not too dangerous for boys? What’s wrong with people?*\n\n15. “Actually, sir, that’s not true. There’s nothing saying we can’t play.” I pulled out my copy of the school handbook. “See?” I said. “The only qualifications are that a player must have at least a C average and that the coach and a doctor must declare the player physically fit. I have straight As, and here’s my doctor’s note.”\n\n16. He grimaced, and then his face lit up. “Well, I don’t declare you physically fit. So that’s that.” He folded his arms across his chest with a satisfied grin.\n\n17. “Why?” I asked.\n\n18. “Girls are too delicate for football. I don't care how good your brother was. You'll snap like a twig out there. Sorry.”\n\n19. I’d come prepared with a second printout. Silently, I handed the paper to Coach. It contained only 37 words.\n\n20. “What in the name of the [[☃ definition 1]] is this?” he asked.\n\n21. “A copy of Title 9, sir. It’s a policy stating that any school that receives money from the government–which ours does–can’t discriminate on the basis of gender. So, I think your reasoning might be invalid. Can we get on with tryouts now?”\n\n22. His face contorted with rage, then confusion, and finally resignation as he pointed towards the center line. “You wanna be quarterback? Show me you can get past these three–” he pointed to Marcus, Raj, and Hiro, “and get the ball to the end zone.” He tossed me a ball.\n\n23. I gulped. I’d expected we’d start slow, maybe with some sprints. \n\n24. *You’ve got this,* I coached myself as I walked toward the center line. *You’re ready. Even Malik said so.* \n\n25. When I arrived at the center line and saw the three boys between me and the end zone, a sense of calm settled over me. Strategy flooded my mind as I thought of everything I’d observed from the bushes. Marcus, on the right, was big but slow. Raj, in the center, had a tendency to cover the left side of his area. Hiro, on the left, was the biggest threat–he was fast and clever, and, as he smirked at me, I knew immediately what my best play would be.\n\n26. As Coach blew his whistle, I raced hard towards the left, ignoring the obvious opening on the right side of the field. No one who knew him ever tried to get past Hiro, so that’s exactly what I would do. I barely saw Hiro’s smirk change into a look of surprise as I approached him, sliding under his outstretched arms. I leapt into the end zone with a whoop of delight. \n\n27. Coach looked reluctantly impressed when I trotted back to him. “Well, that wasn’t half-bad. I think we might give you a shot after all.”\n\n[[☃ explanation 1]]\n\n\n=====",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really love how much noise we get from using full content. It feels a little counter-productive because I'm not sure what parts of this are actually important to the PR. Maybe it'll make more sense when we start making changes to the snapshots, but right now it just feels like a distraction.

@benchristel benchristel merged commit ca06cb8 into main Mar 26, 2025
8 checks passed
@benchristel benchristel deleted the benc/allow-null-itemDataVersion branch March 26, 2025 21:57
ivyolamit pushed a commit that referenced this pull request Mar 26, 2025
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

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name


-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2296](#2296) [`7b76274f0`](7b76274) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix expression widget styling issues. Close button focus outline is now visible, backspace button styling is now consistent with other buttons, and adjusted the popover padding.


-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2296](#2296) [`7b76274f0`](7b76274) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix expression widget styling issues. Close button focus outline is now visible, backspace button styling is now consistent with other buttons, and adjusted the popover padding.


-   [#2301](#2301) [`11a3b8b24`](11a3b8b) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix number line input outline to meet accessible contrast standards


-   [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name


-   [#2326](#2326) [`2e26c0872`](2e26c08) Thanks [@nishasy](https://github.com/nishasy)! - [SR] Remove instructions for static graphs, mark interactive elements as disabled


-   [#2324](#2324) [`7a60db8e8`](7a60db8) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Adding roles to the Image Widget to help improve A11Y.


-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`7b76274f0`](7b76274), [`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name


-   [#2321](#2321) [`ca06cb806`](ca06cb8) Thanks [@benchristel](https://github.com/benchristel)! - Allow `itemDataVersion` to be null when parsing Perseus assessment item JSON


-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`7b76274f0`](7b76274), [`11a3b8b24`](11a3b8b), [`5b6e9df5b`](5b6e9df), [`2e26c0872`](2e26c08), [`7a60db8e8`](7a60db8), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name


-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

-   Updated dependencies \[[`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]

## [email protected]

### Patch Changes

-   [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled

## @khanacademy/[email protected]

### Patch Changes

-   Updated dependencies \[[`7b76274f0`](7b76274), [`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

Author: khan-actions-bot

Reviewers: ivyolamit

Required Reviewers:

Approved By: ivyolamit

Checks: ⏭️  1 check has been skipped, ✅ 4 checks were successful

Pull Request URL: #2330
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants