Skip to content

Commit fe4d9bb

Browse files
committed
Implement blocking rule validations on the server on blur
src/api/patronBlockingRules.ts (new) — validatePatronBlockingRuleExpression(serviceId, rule, csrfToken) — sends FormData POST, returns null on 200 or the error detail string on failure. src/components/PatronAuthServices.tsx — Added additionalData: { csrfToken: ownProps.csrfToken } to mapStateToProps to thread the CSRF token through to the edit form. src/components/PatronAuthServiceEditForm.tsx — Passes csrfToken and serviceId (this.props.item?.id) to both PatronBlockingRulesEditor instances. src/components/PatronBlockingRulesEditor.tsx — Added csrfToken/serviceId props, serverErrors state, handleRuleBlur async handler, clear-on-edit logic in updateRule, and <div onBlur> wrapper + serverRuleError display in RuleFormListItem. tests/jest/api/patronBlockingRules.test.ts (new) — 5 tests covering the API function (null on 200, detail string on 400, fallback string, correct headers, undefined serviceId). tests/jest/components/PatronBlockingRulesEditor.test.tsx — Added 6 blur/server-error tests and added beforeEach/afterEach fetch mock setup to both describe blocks to prevent incidental blur events from failing unrelated tests. Other changes: .eslintrc — Added a @typescript-eslint/no-unused-vars rule override with three options: argsIgnorePattern: "^_" — suppresses the _library, _protocol, _disabled parameter warnings in ServiceEditForm.tsx (and any similar _-prefixed intentionally-unused parameters elsewhere) ignoreRestSiblings: true — suppresses the _id warnings in PatronBlockingRulesEditor.tsx, which is the standard ESLint way to allow the { _id, ...rest } "strip a key" pattern varsIgnorePattern: "^_" — consistent _-prefix convention for variables too ServiceEditForm.tsx — Renamed library → _library in isLibraryRemovalPermitted (the only parameter that didn't already have the _ prefix the others used) PatronAuthServiceEditForm.tsx — Removed the unused PatronBlockingRule import PatronBlockingRulesEditor.tsx — Added // eslint-disable-next-line jsx-a11y/no-static-element-interactions with an explanatory comment. The wrapper <div> is a focus-event delegation container (React's synthetic blur bubbles from the textarea), not an interactive element — an eslint-disable with a clear rationale is the honest and correct fix here rather than misusing a role to silence the rule.
1 parent 2bb3b86 commit fe4d9bb

File tree

9 files changed

+409
-21
lines changed

9 files changed

+409
-21
lines changed

.eslintrc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,15 @@
3939
"react/no-find-dom-node": 0,
4040
"jsx-a11y/label-has-for": 0,
4141
"react/no-unescaped-entities": 0,
42-
"@typescript-eslint/no-inferrable-types": 0
42+
"@typescript-eslint/no-inferrable-types": 0,
43+
"@typescript-eslint/no-unused-vars": [
44+
"warn",
45+
{
46+
"argsIgnorePattern": "^_",
47+
"varsIgnorePattern": "^_",
48+
"ignoreRestSiblings": true
49+
}
50+
]
4351
},
4452
"settings": {
4553
"react": {

src/api/patronBlockingRules.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { PatronBlockingRule } from "../interfaces";
2+
3+
const VALIDATE_URL = "/admin/patron_auth_service_validate_patron_blocking_rule";
4+
5+
/**
6+
* Validate a patron blocking rule expression against live ILS data on the server.
7+
*
8+
* The server loads the saved PatronAuthService by serviceId, makes a live
9+
* authentication call using its configured test credentials, and evaluates
10+
* the rule expression against the real patron data returned. Only parse/eval
11+
* success or failure is reported — the boolean result is discarded.
12+
*
13+
* Returns null on success, or an error message string on failure.
14+
*/
15+
export const validatePatronBlockingRuleExpression = async (
16+
serviceId: number | undefined,
17+
rule: PatronBlockingRule,
18+
csrfToken: string | undefined
19+
): Promise<string | null> => {
20+
const formData = new FormData();
21+
if (serviceId !== undefined) {
22+
formData.append("service_id", String(serviceId));
23+
}
24+
formData.append("rule", rule.rule);
25+
formData.append("name", rule.name);
26+
27+
const headers: Record<string, string> = {};
28+
if (csrfToken) {
29+
headers["X-CSRF-Token"] = csrfToken;
30+
}
31+
32+
const res = await fetch(VALIDATE_URL, {
33+
method: "POST",
34+
headers,
35+
body: formData,
36+
credentials: "same-origin",
37+
});
38+
39+
if (res.ok) {
40+
return null;
41+
}
42+
43+
try {
44+
const data = await res.json();
45+
return data.detail || "Rule validation failed.";
46+
} catch {
47+
return "Rule validation failed.";
48+
}
49+
};

src/components/PatronAuthServiceEditForm.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as React from "react";
22
import {
33
LibraryWithSettingsData,
44
PatronAuthServicesData,
5-
PatronBlockingRule,
65
ProtocolData,
76
} from "../interfaces";
87
import ServiceEditForm from "./ServiceEditForm";
@@ -59,6 +58,12 @@ export default class PatronAuthServiceEditForm extends ServiceEditForm<
5958
value={library.patron_blocking_rules || []}
6059
disabled={disabled}
6160
error={this.props.error}
61+
csrfToken={this.props.additionalData?.csrfToken}
62+
serviceId={
63+
this.props.item?.id !== undefined
64+
? Number(this.props.item.id)
65+
: undefined
66+
}
6267
/>
6368
);
6469
}
@@ -76,6 +81,12 @@ export default class PatronAuthServiceEditForm extends ServiceEditForm<
7681
value={[]}
7782
disabled={disabled}
7883
error={this.props.error}
84+
csrfToken={this.props.additionalData?.csrfToken}
85+
serviceId={
86+
this.props.item?.id !== undefined
87+
? Number(this.props.item.id)
88+
: undefined
89+
}
7990
/>
8091
);
8192
}

src/components/PatronAuthServices.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ function mapStateToProps(state, ownProps) {
7070
// of the create/edit form.
7171
return {
7272
data: data,
73+
additionalData: { csrfToken: ownProps.csrfToken },
7374
responseBody:
7475
state.editor.patronAuthServices &&
7576
state.editor.patronAuthServices.successMessage,

src/components/PatronBlockingRulesEditor.tsx

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { FetchErrorData } from "@thepalaceproject/web-opds-client/lib/interfaces
44
import { PatronBlockingRule } from "../interfaces";
55
import EditableInput from "./EditableInput";
66
import WithRemoveButton from "./WithRemoveButton";
7+
import { validatePatronBlockingRuleExpression } from "../api/patronBlockingRules";
78

89
function extractErrorMessage(error: FetchErrorData): string | null {
910
if (!error || error.status < 400) return null;
@@ -20,6 +21,8 @@ export interface PatronBlockingRulesEditorProps {
2021
value?: PatronBlockingRule[];
2122
disabled?: boolean;
2223
error?: FetchErrorData;
24+
csrfToken?: string;
25+
serviceId?: number;
2326
}
2427

2528
export interface PatronBlockingRulesEditorHandle {
@@ -29,25 +32,30 @@ export interface PatronBlockingRulesEditorHandle {
2932

3033
type RuleEntry = PatronBlockingRule & { _id: number };
3134
type ClientErrors = { [index: number]: { name?: boolean; rule?: boolean } };
35+
type ServerErrors = { [index: number]: string | null };
3236

3337
interface RuleFormListItemProps {
3438
rule: RuleEntry;
3539
index: number;
3640
disabled: boolean;
3741
rowErrors: { name?: boolean; rule?: boolean };
42+
serverRuleError?: string | null;
3843
error?: FetchErrorData;
3944
onRemove: () => void;
4045
onUpdate: (field: keyof PatronBlockingRule, value: string) => void;
46+
onRuleBlur: () => void;
4147
}
4248

4349
function RuleFormListItem({
4450
rule,
4551
index,
4652
disabled,
4753
rowErrors,
54+
serverRuleError,
4855
error,
4956
onRemove,
5057
onUpdate,
58+
onRuleBlur,
5159
}: RuleFormListItemProps) {
5260
const nameClientError = !!rowErrors.name;
5361
const ruleClientError = !!rowErrors.rule;
@@ -80,19 +88,31 @@ function RuleFormListItem({
8088
Rule Expression is required.
8189
</p>
8290
)}
83-
<EditableInput
84-
elementType="textarea"
85-
label="Rule Expression"
86-
name={`patron_blocking_rule_rule_${index}`}
87-
value={rule.rule}
88-
required={true}
89-
disabled={disabled}
90-
readOnly={disabled}
91-
optionalText={false}
92-
clientError={rowErrors.rule}
93-
error={error}
94-
onChange={(value) => onUpdate("rule", value)}
95-
/>
91+
{serverRuleError && (
92+
<p className="patron-blocking-rule-field-error text-danger">
93+
{serverRuleError}
94+
</p>
95+
)}
96+
{/* This div captures the focusout event that bubbles up from the
97+
textarea inside it, triggering server-side rule validation on blur.
98+
It is not an interactive element and has no keyboard/mouse role —
99+
the textarea itself handles all user interaction. */}
100+
{/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
101+
<div onBlur={onRuleBlur}>
102+
<EditableInput
103+
elementType="textarea"
104+
label="Rule Expression"
105+
name={`patron_blocking_rule_rule_${index}`}
106+
value={rule.rule}
107+
required={true}
108+
disabled={disabled}
109+
readOnly={disabled}
110+
optionalText={false}
111+
clientError={rowErrors.rule}
112+
error={error}
113+
onChange={(value) => onUpdate("rule", value)}
114+
/>
115+
</div>
96116
<EditableInput
97117
elementType="textarea"
98118
label="Message (optional)"
@@ -113,14 +133,15 @@ function RuleFormListItem({
113133
const PatronBlockingRulesEditor = React.forwardRef<
114134
PatronBlockingRulesEditorHandle,
115135
PatronBlockingRulesEditorProps
116-
>(({ value = [], disabled = false, error }, ref) => {
136+
>(({ value = [], disabled = false, error, csrfToken, serviceId }, ref) => {
117137
const nextId = React.useRef(0);
118138
const newId = () => nextId.current++;
119139

120140
const [rules, setRules] = React.useState<RuleEntry[]>(() =>
121141
(value || []).map((r) => ({ ...r, _id: newId() }))
122142
);
123143
const [clientErrors, setClientErrors] = React.useState<ClientErrors>({});
144+
const [serverErrors, setServerErrors] = React.useState<ServerErrors>({});
124145

125146
const serverErrorMessage = extractErrorMessage(error);
126147

@@ -166,6 +187,11 @@ const PatronBlockingRulesEditor = React.forwardRef<
166187
delete next[index];
167188
return next;
168189
});
190+
setServerErrors((prev) => {
191+
const next = { ...prev };
192+
delete next[index];
193+
return next;
194+
});
169195
};
170196

171197
const updateRule = (
@@ -182,6 +208,22 @@ const PatronBlockingRulesEditor = React.forwardRef<
182208
return { ...prev, [index]: { ...prev[index], [field]: !value } };
183209
});
184210
}
211+
if (field === "rule") {
212+
setServerErrors((prev) => ({ ...prev, [index]: null }));
213+
}
214+
};
215+
216+
const handleRuleBlur = async (index: number) => {
217+
const rule = rules[index];
218+
if (!rule || !rule.rule) {
219+
return;
220+
}
221+
const errorMessage = await validatePatronBlockingRuleExpression(
222+
serviceId,
223+
rule,
224+
csrfToken
225+
);
226+
setServerErrors((prev) => ({ ...prev, [index]: errorMessage }));
185227
};
186228

187229
const hasIncompleteRule = rules.some((r) => !r.name || !r.rule);
@@ -214,9 +256,11 @@ const PatronBlockingRulesEditor = React.forwardRef<
214256
index={index}
215257
disabled={disabled}
216258
rowErrors={clientErrors[index] || {}}
259+
serverRuleError={serverErrors[index]}
217260
error={error}
218261
onRemove={() => removeRule(index)}
219262
onUpdate={(field, value) => updateRule(index, field, value)}
263+
onRuleBlur={() => handleRuleBlur(index)}
220264
/>
221265
))}
222266
</ul>

src/components/ServiceEditForm.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { FetchErrorData } from "@thepalaceproject/web-opds-client/lib/interfaces
1717
export interface ServiceEditFormProps<T> {
1818
data: T;
1919
item?: ServiceData;
20+
additionalData?: any;
2021
disabled: boolean;
2122
save?: (data: FormData) => void;
2223
urlBase: string;
@@ -635,10 +636,7 @@ export default class ServiceEditForm<
635636
* Subclasses may override this method to control removal.
636637
* @param library The library to remove.
637638
*/
638-
isLibraryRemovalPermitted(library: LibraryWithSettingsData): boolean {
639-
// library should be provided on every call.
640-
// It is not used here, but might be in subclass implementations.
641-
// Removal is permitted by default.
639+
isLibraryRemovalPermitted(_library: LibraryWithSettingsData): boolean {
642640
return true;
643641
}
644642

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import * as fetchMock from "fetch-mock-jest";
2+
import { validatePatronBlockingRuleExpression } from "../../../src/api/patronBlockingRules";
3+
import { PatronBlockingRule } from "../../../src/interfaces";
4+
5+
const VALIDATE_URL = "/admin/patron_auth_service_validate_patron_blocking_rule";
6+
7+
const sampleRule: PatronBlockingRule = {
8+
name: "Fine Check",
9+
rule: "{fines} > 10.0",
10+
};
11+
12+
describe("validatePatronBlockingRuleExpression", () => {
13+
afterEach(() => {
14+
fetchMock.mockReset();
15+
});
16+
17+
it("returns null on a 200 response", async () => {
18+
fetchMock.post(VALIDATE_URL, { status: 200 });
19+
const result = await validatePatronBlockingRuleExpression(
20+
42,
21+
sampleRule,
22+
"test-token"
23+
);
24+
expect(result).toBeNull();
25+
});
26+
27+
it("returns the detail string from a 400 response", async () => {
28+
fetchMock.post(VALIDATE_URL, {
29+
status: 400,
30+
body: { detail: "Unknown placeholder: {unknown_field}" },
31+
});
32+
const result = await validatePatronBlockingRuleExpression(
33+
42,
34+
sampleRule,
35+
"test-token"
36+
);
37+
expect(result).toBe("Unknown placeholder: {unknown_field}");
38+
});
39+
40+
it("returns a fallback string when a 400 response body has no detail", async () => {
41+
fetchMock.post(VALIDATE_URL, { status: 400, body: {} });
42+
const result = await validatePatronBlockingRuleExpression(
43+
42,
44+
sampleRule,
45+
"test-token"
46+
);
47+
expect(result).not.toBeNull();
48+
expect(typeof result).toBe("string");
49+
});
50+
51+
it("sends the correct URL, method, and CSRF header", async () => {
52+
fetchMock.post(VALIDATE_URL, { status: 200 });
53+
await validatePatronBlockingRuleExpression(42, sampleRule, "my-csrf-token");
54+
expect(fetchMock).toHaveBeenCalledWith(
55+
VALIDATE_URL,
56+
expect.objectContaining({
57+
method: "POST",
58+
headers: expect.objectContaining({ "X-CSRF-Token": "my-csrf-token" }),
59+
})
60+
);
61+
});
62+
63+
it("omits service_id from the form body when serviceId is undefined", async () => {
64+
fetchMock.post(VALIDATE_URL, { status: 200 });
65+
// Should not throw and should still make the request (server returns "save first" error)
66+
const result = await validatePatronBlockingRuleExpression(
67+
undefined,
68+
sampleRule,
69+
"tok"
70+
);
71+
expect(fetchMock).toHaveBeenCalledWith(
72+
VALIDATE_URL,
73+
expect.objectContaining({ method: "POST" })
74+
);
75+
// Server would return an error detail in a real call; here it returns 200 (mocked)
76+
expect(result).toBeNull();
77+
});
78+
});

tests/jest/components/PatronAuthServiceEditForm.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import * as React from "react";
22
import { render, screen } from "@testing-library/react";
33
import userEvent from "@testing-library/user-event";
4+
import * as fetchMock from "fetch-mock-jest";
45
import PatronAuthServiceEditForm from "../../../src/components/PatronAuthServiceEditForm";
56
import {
67
PatronAuthServicesData,
78
PatronBlockingRule,
89
} from "../../../src/interfaces";
910
import { SIP2_PROTOCOL } from "../../../src/utils/patronBlockingRules";
1011

12+
const VALIDATE_URL = "/admin/patron_auth_service_validate_patron_blocking_rule";
13+
1114
async function expandLibrariesPanel(user: ReturnType<typeof userEvent.setup>) {
1215
const toggle = screen
1316
.getAllByRole("button")
@@ -63,6 +66,17 @@ function buildSIP2Item(rules: PatronBlockingRule[] = []) {
6366
};
6467
}
6568

69+
// Guard: any blur on the Rule Expression textarea calls validatePatronBlockingRuleExpression.
70+
// All describe blocks get a default 200 mock so tests that incidentally trigger blur
71+
// don't fail with "only absolute URLs are supported" from the fetch polyfill.
72+
beforeEach(() => {
73+
fetchMock.post(VALIDATE_URL, { status: 200 });
74+
});
75+
76+
afterEach(() => {
77+
fetchMock.mockReset();
78+
});
79+
6680
describe("PatronAuthServiceEditForm – capability gating", () => {
6781
it("shows PatronBlockingRulesEditor in expanded library settings for SIP2", async () => {
6882
const user = userEvent.setup();

0 commit comments

Comments
 (0)