Skip to content

Commit 5c3bc9c

Browse files
committed
Refactor PP-3866 help modal: DRY, useQuery, dedicated tests, simplify sync script
- Restore shiftEntries<T> helper; removeRule was duplicating the same index-shifting logic inline for clientErrors and serverErrors - Replace manual useState/useEffect prefetch with useQuery (@tanstack/react-query), extracting logic into a new src/hooks/useAvailableFields.ts custom hook; blur-validation results update the query cache via queryClient.setQueryData - Add Modal.Footer Close button to PatronBlockingRulesHelpModal so users at the bottom of the modal can dismiss without scrolling back up - Add dedicated PatronBlockingRulesHelpModal.test.tsx with 12 isolated unit tests; update editor tests to wrap with QueryClientProvider - Simplify syncPatronBlockingDocs.js from 203 to 38 lines by removing the custom Markdown fallback converter and requiring marked directly (added as an explicit devDependency) - Add stale-copy warning comment to src/content/patronBlockingFunctions.md
1 parent 150af88 commit 5c3bc9c

9 files changed

Lines changed: 339 additions & 286 deletions

package-lock.json

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
"jsdom": "^20.0.3",
115115
"json-loader": "^0.5.4",
116116
"lint-staged": "^10.4.0",
117+
"marked": "^17.0.6",
117118
"mini-css-extract-plugin": "1.6.0",
118119
"mocha": "^10.2.0",
119120
"msw": "^2.7.3",

scripts/syncPatronBlockingDocs.js

Lines changed: 2 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -6,172 +6,13 @@
66
* exports the HTML as a string constant.
77
*
88
* Run via: npm run sync-patron-blocking-docs
9-
*
10-
* Uses the UMD build of `marked` (bundled with the package) for markdown→HTML
11-
* conversion so this script has no extra runtime dependencies.
129
*/
1310

1411
"use strict";
1512

1613
const fs = require("fs");
1714
const path = require("path");
18-
19-
// ---------------------------------------------------------------------------
20-
// Locate marked UMD build (CJS-compatible, no extra install needed)
21-
// ---------------------------------------------------------------------------
22-
let markedFn;
23-
try {
24-
// marked ≥ v5 ships a UMD build at lib/marked.umd.js
25-
const { marked } = require("marked/lib/marked.umd.js");
26-
markedFn = marked;
27-
} catch {
28-
try {
29-
const { marked } = require("marked");
30-
markedFn = marked;
31-
} catch {
32-
// Fallback: very small inline converter covering the elements used in
33-
// FUNCTIONS.md (headings, code blocks, inline code, tables, paragraphs,
34-
// horizontal rules, bold, lists).
35-
markedFn = null;
36-
}
37-
}
38-
39-
function escapeHtml(str) {
40-
return str
41-
.replace(/&/g, "&amp;")
42-
.replace(/</g, "&lt;")
43-
.replace(/>/g, "&gt;")
44-
.replace(/"/g, "&quot;");
45-
}
46-
47-
function convertMarkdown(md) {
48-
if (markedFn) {
49-
return markedFn(md);
50-
}
51-
52-
// Minimal inline converter for the specific elements in FUNCTIONS.md.
53-
const lines = md.split("\n");
54-
const out = [];
55-
let i = 0;
56-
57-
while (i < lines.length) {
58-
const line = lines[i];
59-
60-
// Fenced code block
61-
if (line.startsWith("```")) {
62-
const lang = line.slice(3).trim();
63-
const codeLines = [];
64-
i++;
65-
while (i < lines.length && !lines[i].startsWith("```")) {
66-
codeLines.push(escapeHtml(lines[i]));
67-
i++;
68-
}
69-
out.push(
70-
`<pre><code${
71-
lang ? ` class="language-${escapeHtml(lang)}"` : ""
72-
}>${codeLines.join("\n")}</code></pre>`
73-
);
74-
i++;
75-
continue;
76-
}
77-
78-
// Horizontal rule
79-
if (/^---+$/.test(line.trim())) {
80-
out.push("<hr>");
81-
i++;
82-
continue;
83-
}
84-
85-
// Headings
86-
const headingMatch = line.match(/^(#{1,6})\s+(.*)/);
87-
if (headingMatch) {
88-
const level = headingMatch[1].length;
89-
out.push(`<h${level}>${inlineMarkdown(headingMatch[2])}</h${level}>`);
90-
i++;
91-
continue;
92-
}
93-
94-
// Table (detect by pipe characters)
95-
if (
96-
line.includes("|") &&
97-
i + 1 < lines.length &&
98-
lines[i + 1].match(/^\|?[\s\-|]+\|?$/)
99-
) {
100-
const tableLines = [];
101-
while (i < lines.length && lines[i].includes("|")) {
102-
tableLines.push(lines[i]);
103-
i++;
104-
}
105-
out.push(renderTable(tableLines));
106-
continue;
107-
}
108-
109-
// Blank line
110-
if (line.trim() === "") {
111-
i++;
112-
continue;
113-
}
114-
115-
// List item
116-
if (/^[-*]\s/.test(line)) {
117-
out.push("<ul>");
118-
while (i < lines.length && /^[-*]\s/.test(lines[i])) {
119-
out.push(`<li>${inlineMarkdown(lines[i].replace(/^[-*]\s/, ""))}</li>`);
120-
i++;
121-
}
122-
out.push("</ul>");
123-
continue;
124-
}
125-
126-
// Paragraph
127-
const paraLines = [];
128-
while (
129-
i < lines.length &&
130-
lines[i].trim() !== "" &&
131-
!lines[i].startsWith("#") &&
132-
!lines[i].startsWith("```") &&
133-
!lines[i].startsWith("---") &&
134-
!lines[i].includes("|")
135-
) {
136-
paraLines.push(lines[i]);
137-
i++;
138-
}
139-
if (paraLines.length > 0) {
140-
out.push(`<p>${inlineMarkdown(paraLines.join(" "))}</p>`);
141-
}
142-
}
143-
144-
return out.join("\n");
145-
}
146-
147-
function inlineMarkdown(text) {
148-
return text
149-
.replace(/`([^`]+)`/g, (_, code) => `<code>${escapeHtml(code)}</code>`)
150-
.replace(/\*\*([^*]+)\*\*/g, "<strong>$1</strong>")
151-
.replace(/\[([^\]]+)\]\(([^)]+)\)/g, '<a href="$2">$1</a>');
152-
}
153-
154-
function renderTable(tableLines) {
155-
const rows = tableLines.map((l) =>
156-
l
157-
.split("|")
158-
.filter((_, idx, arr) => idx > 0 && idx < arr.length - 1)
159-
.map((cell) => cell.trim())
160-
);
161-
const [header, , ...body] = rows; // second row is the separator
162-
const headerHtml = header
163-
.map((c) => `<th>${inlineMarkdown(escapeHtml(c))}</th>`)
164-
.join("");
165-
const bodyHtml = body
166-
.map(
167-
(row) =>
168-
`<tr>${row
169-
.map((c) => `<td>${inlineMarkdown(escapeHtml(c))}</td>`)
170-
.join("")}</tr>`
171-
)
172-
.join("\n");
173-
return `<table class="table table-condensed table-bordered"><thead><tr>${headerHtml}</tr></thead><tbody>${bodyHtml}</tbody></table>`;
174-
}
15+
const { marked } = require("marked");
17516

17617
// ---------------------------------------------------------------------------
17718
// Main
@@ -188,7 +29,7 @@ if (!fs.existsSync(srcMd)) {
18829
}
18930

19031
const markdown = fs.readFileSync(srcMd, "utf8");
191-
const html = convertMarkdown(markdown);
32+
const html = marked(markdown);
19233

19334
const tsContent = `// AUTO-GENERATED — do not edit by hand.
19435
// Run \`npm run sync-patron-blocking-docs\` to regenerate from

src/components/PatronBlockingRulesEditor.tsx

Lines changed: 22 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,21 @@ import EditableInput from "./EditableInput";
66
import WithRemoveButton from "./WithRemoveButton";
77
import { validatePatronBlockingRuleExpression } from "../api/patronBlockingRules";
88
import PatronBlockingRulesHelpModal from "./PatronBlockingRulesHelpModal";
9+
import { useAvailableFields } from "../hooks/useAvailableFields";
10+
11+
/** Shifts a numeric-indexed map after an entry at `removedIndex` is deleted.
12+
* Entries below the removed index are kept as-is; entries above are re-keyed
13+
* to index - 1; the entry at the removed index is dropped. */
14+
function shiftEntries<T>(
15+
map: { [k: number]: T },
16+
removedIndex: number
17+
): { [k: number]: T } {
18+
return Object.fromEntries(
19+
Object.entries(map)
20+
.filter(([k]) => Number(k) !== removedIndex)
21+
.map(([k, v]) => [Number(k) > removedIndex ? Number(k) - 1 : Number(k), v])
22+
) as { [k: number]: T };
23+
}
924

1025
/** Returns the set of non-empty rule names that appear more than once. */
1126
function getDuplicateNameSet(rules: RuleEntry[]): Set<string> {
@@ -182,15 +197,10 @@ const PatronBlockingRulesEditor = React.forwardRef<
182197
);
183198
const [clientErrors, setClientErrors] = React.useState<ClientErrors>({});
184199
const [serverErrors, setServerErrors] = React.useState<ServerErrors>({});
185-
186-
// Help modal state
187200
const [showHelp, setShowHelp] = React.useState(false);
188-
const [availableFields, setAvailableFields] = React.useState<Record<
189-
string,
190-
unknown
191-
> | null>(null);
192-
const [fieldsLoading, setFieldsLoading] = React.useState(false);
193-
const [fieldsError, setFieldsError] = React.useState<string | null>(null);
201+
202+
const { availableFields, fieldsLoading, fieldsError, updateFields } =
203+
useAvailableFields(serviceId, csrfToken);
194204

195205
// Keep stable refs to the latest callbacks so the useEffects below do not
196206
// need them as dependencies (avoids extra calls when a class-component parent
@@ -220,53 +230,6 @@ const PatronBlockingRulesEditor = React.forwardRef<
220230
onServerValidationStateChangeRef.current?.(hasServerError);
221231
}, [serverErrors]);
222232

223-
// Eagerly pre-fetch available template fields on mount (and whenever
224-
// serviceId changes) by calling the validation endpoint with a trivial
225-
// passing rule. This ensures the template-variable table in the help
226-
// modal is populated before the user opens it.
227-
React.useEffect(() => {
228-
if (serviceId === undefined) {
229-
setAvailableFields(null);
230-
setFieldsError(
231-
"Save the service before template variables can be fetched."
232-
);
233-
return;
234-
}
235-
236-
let cancelled = false;
237-
setFieldsLoading(true);
238-
setFieldsError(null);
239-
240-
const trivialRule: PatronBlockingRule = {
241-
name: "__prefetch__",
242-
rule: "True",
243-
};
244-
validatePatronBlockingRuleExpression(serviceId, trivialRule, csrfToken)
245-
.then((result) => {
246-
if (cancelled) return;
247-
if (result.error) {
248-
setFieldsError(result.error);
249-
} else if (result.availableFields) {
250-
setAvailableFields(result.availableFields);
251-
} else {
252-
setFieldsError(
253-
"Available fields could not be retrieved. Check that test credentials are configured."
254-
);
255-
}
256-
})
257-
.catch(() => {
258-
if (cancelled) return;
259-
setFieldsError("Failed to fetch available fields.");
260-
})
261-
.finally(() => {
262-
if (!cancelled) setFieldsLoading(false);
263-
});
264-
265-
return () => {
266-
cancelled = true;
267-
};
268-
}, [serviceId, csrfToken]);
269-
270233
const serverErrorMessage = extractErrorMessage(error);
271234

272235
React.useImperativeHandle(
@@ -309,26 +272,8 @@ const PatronBlockingRulesEditor = React.forwardRef<
309272

310273
const removeRule = (index: number) => {
311274
setRules((prev) => prev.filter((_, i) => i !== index));
312-
setClientErrors((prev) => {
313-
const next: ClientErrors = {};
314-
Object.entries(prev).forEach(([key, val]) => {
315-
const k = Number(key);
316-
if (k < index) next[k] = val;
317-
else if (k > index) next[k - 1] = val;
318-
// k === index is dropped
319-
});
320-
return next;
321-
});
322-
setServerErrors((prev) => {
323-
const next: ServerErrors = {};
324-
Object.entries(prev).forEach(([key, val]) => {
325-
const k = Number(key);
326-
if (k < index) next[k] = val;
327-
else if (k > index) next[k - 1] = val;
328-
// k === index is dropped
329-
});
330-
return next;
331-
});
275+
setClientErrors((prev) => shiftEntries(prev, index));
276+
setServerErrors((prev) => shiftEntries(prev, index));
332277
};
333278

334279
const updateRule = (
@@ -361,9 +306,9 @@ const PatronBlockingRulesEditor = React.forwardRef<
361306
csrfToken
362307
);
363308
setServerErrors((prev) => ({ ...prev, [index]: result.error }));
364-
// Keep the most-recent non-null available fields snapshot.
309+
// Keep the query cache up-to-date with the most-recent non-null snapshot.
365310
if (result.availableFields) {
366-
setAvailableFields(result.availableFields);
311+
updateFields(result.availableFields);
367312
}
368313
};
369314

src/components/PatronBlockingRulesHelpModal.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as React from "react";
22
import { Modal } from "react-bootstrap";
3+
import { Button } from "library-simplified-reusable-components";
34
import patronBlockingFunctionsHtml from "../content/patronBlockingFunctionsHtml";
45

56
const SIMPLEEVAL_URL = "https://github.com/danthedeckie/simpleeval";
@@ -111,6 +112,14 @@ export function PatronBlockingRulesHelpModal({
111112
</p>
112113
</section>
113114
</Modal.Body>
115+
<Modal.Footer>
116+
<Button
117+
className="inverted left-align small inline"
118+
callback={onHide}
119+
content="Close"
120+
title="Close help modal"
121+
/>
122+
</Modal.Footer>
114123
</Modal>
115124
);
116125
}

src/content/patronBlockingFunctions.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
<!-- REFERENCE COPY — do not edit here.
2+
The source of truth is circulation/docs/FUNCTIONS.md.
3+
Run `npm run sync-patron-blocking-docs` after updating that file. -->
4+
15
# Patron Blocking Rules — Allowed Functions
26

37
Patron blocking rule expressions are evaluated by a locked-down

0 commit comments

Comments
 (0)