Skip to content

Commit

Permalink
Lint against static widgets in question stems (#1511)
Browse files Browse the repository at this point in the history
We have logic in the Perseus renderer that prevents widgets from being static
in question stems. This logic was put in place to prevent content creators from
accidentally creating exercises that couldn't be answered (because all their
widgets were static). However the implementation is pretty hacky (it checks
problemNum to know if the renderer is in a question stem), and doesn't work in
the exercise editor preview, or in Perseus storybook. The resulting behavior is
very surprising: `static` widgets in question stems are non-interactive in the
exercise editor but are interactive in the learner experience.

Because this is so confusing, we want to remove the special-case logic and let
`static` always do what it says on the tin. But we still want to ensure that
unanswerable exercises aren't published. To that end, this commit adds a linter
that will do that.

Issue: LEMS-2251

## Test plan:

Deploy a ZND and edit an exercise.

- Add a static widget to a question stem; you should get a linter warning.
- Add a static widget to a hint; you should not get a linter warning.
- Add a non-static widget to a question; you should not get a linter warning.
- Add a static widget to an article; you should not get a linter warning.

Note that only Radio widgets can be made static, currently.

<img width="805" alt="Screen Shot 2024-08-13 at 2 40 06 PM" src="https://github.com/user-attachments/assets/7a3cf4d1-431c-4b06-aa5f-82981fe69771">

Author: benchristel

Reviewers: benchristel, jeremywiebe

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ 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), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (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), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1511
  • Loading branch information
benchristel authored Aug 13, 2024
1 parent 9cb0321 commit 7eb7ab1
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 1 deletion.
6 changes: 6 additions & 0 deletions .changeset/shy-toes-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-linter": minor
---

Warn content creators when a static widget appears in a question stem
96 changes: 95 additions & 1 deletion packages/perseus-linter/src/__tests__/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import mathStartsWithSpaceRule from "../rules/math-starts-with-space";
import mathTextEmptyRule from "../rules/math-text-empty";
import mathWithoutDollarsRule from "../rules/math-without-dollars";
import nestedListsRule from "../rules/nested-lists";
import StaticWidgetInQuestionStem from "../rules/static-widget-in-question-stem";
import tableMissingCellsRule from "../rules/table-missing-cells";
import unbalancedCodeDelimitersRule from "../rules/unbalanced-code-delimiters";
import unescapedDollarRule from "../rules/unescaped-dollar";
Expand All @@ -35,7 +36,11 @@ import TreeTransformer from "../tree-transformer";
type Rule = any;

describe("Individual lint rules tests", () => {
function testRule(rule: Rule, markdown: string, context) {
function testRule(
rule: Rule,
markdown: string,
context,
): {message: string}[] | null {
const tree = PureMarkdown.parse(markdown);
const tt = new TreeTransformer(tree);
const warnings = [];
Expand Down Expand Up @@ -532,4 +537,93 @@ describe("Individual lint rules tests", () => {
"This is definitely okay. Yeah.",
"$a == 3. 125$",
]);

test("Rule static-widget-in-question-stem allows static widgets in hints", () => {
const problems = testRule(
StaticWidgetInQuestionStem,
"[[☃ radio 1]]",
{
contentType: "exercise",
stack: ["hint"],
widgets: {
"radio 1": {
static: true,
},
},
},
);

expect(problems).toBe(null);
});

test("Rule static-widget-in-question-stem allows static widgets in articles", () => {
const problems = testRule(
StaticWidgetInQuestionStem,
"[[☃ radio 1]]",
{
contentType: "article",
stack: [],
widgets: {
"radio 1": {
static: true,
},
},
},
);

expect(problems).toBe(null);
});

test("Rule static-widget-in-question-stem allows non-static widgets in question stems", () => {
const problems = testRule(
StaticWidgetInQuestionStem,
"[[☃ radio 1]]",
{
contentType: "exercise",
stack: [],
widgets: {
"radio 1": {
static: false,
},
},
},
);

expect(problems).toBe(null);
});

test("Rule static-widget-in-question-stem tolerates widget with no definition", () => {
const problems = testRule(
StaticWidgetInQuestionStem,
"[[☃ radio 1]]",
{
contentType: "exercise",
stack: [],
widgets: {},
},
);

expect(problems).toBe(null);
});

test("Rule static-widget-in-question-stem allows warns about static widgets in question stems", () => {
const problems = testRule(
StaticWidgetInQuestionStem,
"[[☃ radio 1]]",
{
contentType: "exercise",
stack: [],
widgets: {
"radio 1": {
static: true,
},
},
},
);

expect(problems?.length).toBe(1);
expect(problems?.[0]?.message).toBe(
"Widget in question stem is static (non-interactive).",
);
});
});
2 changes: 2 additions & 0 deletions packages/perseus-linter/src/rules/all-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import MathStartsWithSpace from "./math-starts-with-space";
import MathTextEmpty from "./math-text-empty";
import MathWithoutDollars from "./math-without-dollars";
import NestedLists from "./nested-lists";
import StaticWidgetInQuestionStem from "./static-widget-in-question-stem";
import TableMissingCells from "./table-missing-cells";
import UnbalancedCodeDelimiters from "./unbalanced-code-delimiters";
import UnescapedDollar from "./unescaped-dollar";
Expand Down Expand Up @@ -59,6 +60,7 @@ export default [
MathStartsWithSpace,
MathTextEmpty,
NestedLists,
StaticWidgetInQuestionStem,
TableMissingCells,
UnescapedDollar,
WidgetInTable,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import Rule from "../rule";

export default Rule.makeRule({
name: "static-widget-in-question-stem",
severity: Rule.Severity.WARNING,
selector: "widget",
lint: (state, content, nodes, match, context) => {
if (context.contentType !== "exercise") {
return;
}

if (context.stack.includes("hint")) {
return;
}

const widget = context?.widgets?.[state.currentNode().id];
if (!widget) {
return;
}

if (widget.static) {
return `Widget in question stem is static (non-interactive).`;
}
},
});

0 comments on commit 7eb7ab1

Please sign in to comment.