Skip to content

Commit be8b783

Browse files
copilot code review fixes
1 parent bc85b2d commit be8b783

6 files changed

Lines changed: 101 additions & 8 deletions

File tree

application/CohortManager/src/Web/app/components/removeDummyGpCodeForm.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export default function RemoveDummyGpCodeForm() {
6868
{errorMessage}
6969
</span>
7070
)}
71-
<input className={`nhsuk-input ${hasError && errorField === "forename" ? "nhsuk-input--error" : ""}`} id="forename" name="forename" type="text" defaultValue={values?.forename} data-testid="forename-input" />
71+
<input className={`nhsuk-input ${hasError && errorField === "forename" ? "nhsuk-input--error" : ""}`} id="forename" name="forename" type="text" defaultValue={values?.forename} aria-describedby={hasError && errorField === "forename" ? "forename-error" : undefined} data-testid="forename-input" />
7272
</div>
7373

7474
<div className={`nhsuk-form-group ${hasError && errorField === "surname" ? "nhsuk-form-group--error" : ""}`}>
@@ -79,11 +79,11 @@ export default function RemoveDummyGpCodeForm() {
7979
{errorMessage}
8080
</span>
8181
)}
82-
<input className={`nhsuk-input ${hasError && errorField === "surname" ? "nhsuk-input--error" : ""}`} id="surname" name="surname" type="text" defaultValue={values?.surname} data-testid="surname-input" />
82+
<input className={`nhsuk-input ${hasError && errorField === "surname" ? "nhsuk-input--error" : ""}`} id="surname" name="surname" type="text" defaultValue={values?.surname} aria-describedby={hasError && errorField === "surname" ? "surname-error" : undefined} data-testid="surname-input" />
8383
</div>
8484

8585
<div className={`nhsuk-form-group ${hasError && errorField === "dob-day" ? "nhsuk-form-group--error" : ""}`}>
86-
<fieldset className="nhsuk-fieldset" aria-describedby="dob-hint" role="group">
86+
<fieldset className="nhsuk-fieldset" aria-describedby={`dob-hint${hasError && errorField === "dob-day" ? " dob-error" : ""}`} role="group">
8787
<legend className="nhsuk-fieldset__legend nhsuk-label nhsuk-u-font-weight-bold">Date of Birth</legend>
8888
<div className="nhsuk-hint" id="dob-hint">For example, 15 3 1984</div>
8989
{hasError && errorField === "dob-day" && (
@@ -124,7 +124,7 @@ export default function RemoveDummyGpCodeForm() {
124124
{errorMessage}
125125
</span>
126126
)}
127-
<input className={`nhsuk-input nhsuk-input--width-10 ${hasError && errorField === "serviceNowTicketNumber" ? "nhsuk-input--error" : ""}`} id="serviceNowTicketNumber" name="serviceNowTicketNumber" type="text" defaultValue={values?.serviceNowTicketNumber} aria-describedby="serviceNowTicketNumber-hint" data-testid="service-now-ticket-input"/>
127+
<input className={`nhsuk-input nhsuk-input--width-10 ${hasError && errorField === "serviceNowTicketNumber" ? "nhsuk-input--error" : ""}`} id="serviceNowTicketNumber" name="serviceNowTicketNumber" type="text" defaultValue={values?.serviceNowTicketNumber} aria-describedby={`serviceNowTicketNumber-hint${hasError && errorField === "serviceNowTicketNumber" ? " serviceNowTicketNumber-error" : ""}`} data-testid="service-now-ticket-input"/>
128128
</div>
129129

130130
<button className="nhsuk-button" data-module="nhsuk-button" type="submit" data-testid="submit-button">Submit</button>

application/CohortManager/src/Web/app/globals.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,14 @@
166166
background-position: -200% 0;
167167
}
168168
}
169+
170+
@media (prefers-reduced-motion: reduce) {
171+
.app-loading-shimmer {
172+
animation: none;
173+
background-position: 0 0;
174+
}
175+
}
176+
169177
.nhsuk-header__content {
170178
display: flex;
171179
align-items: center;

application/CohortManager/src/Web/app/lib/formValidationSchemas.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,36 @@ describe("formValidationSchemas", () => {
501501
expect(result.error.issues[0].message).toBe("Date of Birth must be a valid date");
502502
}
503503
});
504+
505+
it("should reject impossible calendar dates", () => {
506+
const data = {
507+
nhsNumber: "9434765919",
508+
forename: "Jane",
509+
surname: "Smith",
510+
dateOfBirth: "2024-02-31",
511+
serviceNowTicketNumber: "CS1234567",
512+
};
513+
const result = removeDummyGpCodeSchema.safeParse(data);
514+
expect(result.success).toBe(false);
515+
if (!result.success) {
516+
expect(result.error.issues[0].message).toBe("Date of Birth must be a valid date");
517+
}
518+
});
519+
520+
it("should reject dates that are not zero-padded ISO format", () => {
521+
const data = {
522+
nhsNumber: "9434765919",
523+
forename: "Jane",
524+
surname: "Smith",
525+
dateOfBirth: "2024-2-3",
526+
serviceNowTicketNumber: "CS1234567",
527+
};
528+
const result = removeDummyGpCodeSchema.safeParse(data);
529+
expect(result.success).toBe(false);
530+
if (!result.success) {
531+
expect(result.error.issues[0].message).toBe("Date of Birth must be a valid date");
532+
}
533+
});
504534
});
505535

506536
describe("Service Now Ticket Number validation", () => {

application/CohortManager/src/Web/app/lib/formValidationSchemas.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
import { z } from "zod";
22

3+
function isValidIsoDateString(value: string): boolean {
4+
if (!/^\d{4}-\d{2}-\d{2}$/.test(value)) {
5+
return false;
6+
}
7+
8+
const [yearText, monthText, dayText] = value.split("-");
9+
const year = Number.parseInt(yearText, 10);
10+
const month = Number.parseInt(monthText, 10);
11+
const day = Number.parseInt(dayText, 10);
12+
const date = new Date(Date.UTC(year, month - 1, day));
13+
14+
return (
15+
date.getUTCFullYear() === year &&
16+
date.getUTCMonth() === month - 1 &&
17+
date.getUTCDate() === day
18+
);
19+
}
20+
321
// This schema validates the ServiceNow case ID for updating exceptions status.
422
// In edit mode, empty input is allowed to clear the ServiceNow ID (convert raised to non-raised).
523
// In non-edit mode, the ID is required and must start with two letters followed by at least seven digits (e.g. CS0619153).
@@ -69,10 +87,7 @@ export const removeDummyGpCodeSchema = z.object({
6987
dateOfBirth: z
7088
.string()
7189
.min(1, "Date of Birth is required")
72-
.refine((val) => {
73-
const date = new Date(val);
74-
return !Number.isNaN(date.getTime());
75-
}, "Date of Birth must be a valid date"),
90+
.refine((val) => isValidIsoDateString(val), "Date of Birth must be a valid date"),
7691
serviceNowTicketNumber: z
7792
.string()
7893
.min(1, "Service Now Ticket Number is required")
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
Feature: Remove Dummy GP Code page
2+
3+
Background:
4+
Given I sign in with a test account
5+
6+
Scenario: Form is visible with expected fields and actions
7+
When I go to the page "/remove-dummy-gp-code"
8+
Then I should see the heading "Remove Dummy GP Code"
9+
And I see the link "Home" with the href "/"
10+
And I see the input with label "NHS Number"
11+
And I see the text "Forename"
12+
And I see the text "Surname"
13+
And I see the text "Date of Birth"
14+
And I see the text "Service Now Ticket Number"
15+
And I see the button "Submit"
16+
17+
Scenario: Page has no accessibility issues when signed in
18+
When I go to the page "/remove-dummy-gp-code"
19+
Then I should see the heading "Remove Dummy GP Code"
20+
And I should expect 0 accessibility issues
21+
22+
Scenario: Empty submit shows the NHS Number validation error
23+
When I go to the page "/remove-dummy-gp-code"
24+
And I click the button "Submit"
25+
Then I should see the error summary with message "NHS Number is required"
26+
And I should see the inline error message "NHS Number is required"
27+
28+
Scenario: Success confirmation panel is shown when success query parameter is present
29+
When I go to the page "/remove-dummy-gp-code?success=true"
30+
Then I should see the heading "Request submitted successfully"
31+
And I see the text "Your request to remove the dummy GP Code has been accepted."

application/CohortManager/src/Web/tests/features/steps/steps.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,15 @@ Then("I see the button {string}", async ({ page }, buttonText: string) => {
174174
.toBeVisible();
175175
});
176176

177+
Then(
178+
"I see the input with label {string}",
179+
async ({ page }, labelText: string) => {
180+
await test
181+
.expect(page.getByLabel(labelText, { exact: true }))
182+
.toBeVisible();
183+
}
184+
);
185+
177186
Then(
178187
"I see the link {string} with the href {string}",
179188
async ({ page }, linkText: string, expectedHref: string) => {

0 commit comments

Comments
 (0)