diff --git a/AGENTS.md b/AGENTS.md index 7a9f772ce..0643e1010 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -181,6 +181,8 @@ Results enquiries and 16-19 windows: `docs/results-enquiry.md`, `docs/16-19-wind `x-ingress.source`. Never put a supplier column name in a C# property name. - **Inclusion is the `INCLUDED` stamp.** Ingress decides it once. Code that reads pupils must not know a key stage's inclusion codes. +- **Who may be included is the `INCLUDABLE` stamp.** Ingress writes it from the schema's + `x-ingress.includable` rule (#711). The Include journey reads the stamp, never a code. - **The service is not live yet.** Do not write fallbacks for data written before a change. Treat missing new-format data as an error. (This does not change the migration rule above: that rule protects deployed databases.) diff --git a/docs/blob-storage.md b/docs/blob-storage.md index 2eab066e5..eca46ae9b 100644 --- a/docs/blob-storage.md +++ b/docs/blob-storage.md @@ -44,7 +44,15 @@ reach the journey as `QAN`, `SYLLABUS`, `SESSION` and `QUAL_NAME`; in the 16-18 schools see. Schools see the label, never the tag. A dataset's `SourceFile` is the exact analogue of its `Included`: `Included` stamps inclusion by file of origin, `SourceFile` stamps provenance, and both are stamped by the processor before validation and guarded by the schema declaring the column (`INCLUDED` / `SOURCE`), because -`AllowAdditionalProperties` is false. A run writes +`AllowAdditionalProperties` is false. A third stamp, `INCLUDABLE` (#711), says whether a school may +ask to include the pupil: true when the record is not included, or when its code is one the +schema's `x-ingress.includable` rule names (the 16-19 included file names `P_INCL` 502). It is +derived from `INCLUDED`, so it follows the same guard. The Include journey reads it. A rule with +no field, an undeclared field, no codes or a blank code fails the run by name before any row is +read, and so does a schema that declares `INCLUDED` but not `INCLUDABLE`: it would otherwise +offer nobody for inclusion. A pupil blob written before `INCLUDABLE` existed lacks it, on every +key stage, and reads as nobody includable: the Include search finds nobody, and the "already +included" check matches every typed name. Validate the exercise again to restamp it. A run writes `CheckingExerciseBlobPaths.DataBlobName(exercise, laestab)` — never `PupilsBlobName` directly: the two normalise the laestab differently on purpose. Every feed must carry a `LAESTAB` column, or the column its schema's `x-ingress.institutionKey` names (`LAESTAB_0` in the previously published file): it is what splits one supplier file into one blob per school; a file without it fails the run by name. diff --git a/docs/domain-model.md b/docs/domain-model.md index 56d704ca6..5e27f82ba 100644 --- a/docs/domain-model.md +++ b/docs/domain-model.md @@ -135,7 +135,7 @@ Session readiness for the journey requires only `SelectedWhatToChange` and `Chec - The controller holds **no `TimeProvider`** — every "is it open" question goes through `ICheckingExerciseService`. - Displays paged, searchable pupil lists for a given checking window and school (by `laestab`). - `CheckYourPupilDataRepository` loads the school's pupil JSON (cached) and filters/searches/sorts/pages **in memory** — sorted by surname/firstname. **The pupil search boxes share one rule, `PupilSearchTerms`** (parse the query once with `Parse`, then `Matches` per row): the query splits on spaces, tabs and commas, and each word must match the start of a word in some field (words start after a space, tab, hyphen or apostrophe), ignoring case, in any order ("John Smith", "Smith, John"; "ohn" finds nothing). Identifiers match from their start too ("A860" finds a UPN, "8604" does not). The journey autocomplete checks names, UPN/ULN, CYPMD ID and displayed DOB; the pupil tabs check names; the results table checks names, CYPMD ID and subject; the schema-driven tabs check the schema's `searchable` columns. Only the Add-pupil duplicate check (fuzzy on purpose, not a search box) does not use it. -- Pupils are split into included (`IsIncluded`, the `INCLUDED` stamp) and non-included (all others) tabs. +- Pupils are split into included (`IsIncluded`, the `INCLUDED` stamp) and non-included (all others) tabs. A second stamp, `INCLUDABLE` (#711), says who a school may ask to include; only the Include journey reads it. - **Since #466 the tabs can be exercise-driven.** `IExerciseTabBuilder` (Application) returns one tab per *visible* exercise — enabled, within `VisibleFrom`/`VisibleUntil` (exclusive at the end), and every exercise has a `TabName`. When no exercise is visible the list is empty and the page @@ -176,7 +176,7 @@ Session readiness for the journey requires only `SelectedWhatToChange` and `Chec - Multi-step question flow for submitting a pupil data change request. - Session state (`RequestState`) tracks answers, page history, selected pupil(s), and reference number. - Pupil selection is an in-journey `PupilSearch` page (the first page of every flow config). `WhatToChangeController` redirects to `Journey.Page(firstPageId)` after saving the window and `WhatToChange` to session. The journey's `IsSessionReady` check requires only those two fields. -- **What to change offers a change only when its flow file exists** (`IQuestionFlowService.GetPupilDataChangesAsync`: Merge, Include, Remove, Add, in that order, each only when `{WhatToChange}_{CheckingWindowType}.json` exists). The view renders `WhatToChangeViewModel.AvailableChanges` and names no change or window type; the POST refuses any change outside the same list. To give a window type a journey, add its flow file. There are no window-type lists. +- **What to change offers a change only when its flow file exists** (`IQuestionFlowService.GetPupilDataChangesAsync`: Merge, Include, Remove, Add, in that order, each only when `{WhatToChange}_{CheckingWindowType}.json` exists). The view renders `WhatToChangeViewModel.AvailableChanges` and names no change or window type; the POST refuses any change outside the same list. To give a window type a journey, add its flow file. There are no window-type lists. 16-19 offers Include since #711: `Include_Post16.json`, whose search uses `pupilFilter: "Includable"` (the `INCLUDABLE` stamp — the non-included file plus the included file's P_INCL 502 students). Every 16-19 include goes to Scrutiny: the `Inclusion` rules name KS4 codes only. - On submission, `IRequestService.ConfirmRequestAsync` builds the `RequestDocument` (including an optional `MatchedPupil` for Merge) and saves it to blob storage (container named by `windowId`, blob `request_{referenceNumber}.json`). The close sweep sends this saved copy. - The Summary page shows "First record to merge" / "Second record to merge" rows (with Change links to the respective `PupilSearchPage`) when `MatchedPupil` is set; otherwise shows the standard "Pupil name" row. @@ -240,7 +240,7 @@ through `IExerciseEarlyClosureService`, then sweeps). Once it has closed, the su Requests page is removed. **`ICheckYourPupilDataService`** — pupil data queries (partial) -`GetPupilSuggestionsAsync(windowId, query, PupilFilter filter, Guid? excludeId, bool requireResults)` returns up to 10 `PupilSuggestionDto(Id, Label)` records. `PupilFilter.Included` restricts to pupils stamped `INCLUDED`; `PupilFilter.All` returns all pupils for the school. `excludeId` removes a specific pupil from results (used by the match-pupil page to prevent selecting the primary pupil again). `requireResults` restricts the search to students in `IResultsClient.GetCypmdIdsWithResultsAsync` (the same cached school results file the enquiry itself reads), and is passed down to `SearchPupilsAsync` as a CYPMD id allow-list so it applies **before** the ten-suggestion cap — filtering afterwards would drop the one student who holds results whenever ten who do not sort ahead of them. Served by `PupilSuggestionsController` at `GET /pupils/suggestions`. The `WhatToChange?`-based overload no longer exists — callers must pass an explicit `PupilFilter`. +`GetPupilSuggestionsAsync(windowId, query, PupilFilter filter, Guid? excludeId, bool requireResults)` returns up to 10 `PupilSuggestionDto(Id, Label)` records. `PupilFilter.Included` restricts to pupils stamped `INCLUDED`; `PupilFilter.All` returns all pupils for the school; `PupilFilter.Includable` restricts to pupils stamped `INCLUDABLE` and `PupilFilter.NotIncludable` to the rest (#711). `excludeId` removes a specific pupil from results (used by the match-pupil page to prevent selecting the primary pupil again). `requireResults` restricts the search to students in `IResultsClient.GetCypmdIdsWithResultsAsync` (the same cached school results file the enquiry itself reads), and is passed down to `SearchPupilsAsync` as a CYPMD id allow-list so it applies **before** the ten-suggestion cap — filtering afterwards would drop the one student who holds results whenever ten who do not sort ahead of them. Served by `PupilSuggestionsController` at `GET /pupils/suggestions`. The `WhatToChange?`-based overload no longer exists — callers must pass an explicit `PupilFilter`. **`ICountryService`** — country/autocomplete lookup `SearchAsync(query)` returns up to 10 `CountrySuggestionDto(Code, Name)` records matching the query against `Country.Name` (case-insensitive `ILike`). Used by `CountryController.Suggestions` to serve the `Autocomplete` question type. The underlying `Countries` table is seeded from the FCDO Geographical Names Index and is not checking-window-specific. diff --git a/docs/ks2-journeys.md b/docs/ks2-journeys.md index dd63c6da3..dd9218797 100644 --- a/docs/ks2-journeys.md +++ b/docs/ks2-journeys.md @@ -28,7 +28,7 @@ KS2 has no Merge, Include or Add journey. The requirements say so. | Window | Flow files | Options shown | |---|---|---| | KS4 June | Add, Include, Merge, Remove | All four (no change) | - | Post16 | Merge, Remove | Merge, Remove (no change) | + | Post16 | Include, Merge, Remove | Include (since #711), Merge, Remove | | KS4 Autumn | Add | Add only. Before, it also showed Merge, Include and Remove, which had no flow file and went nowhere. | | KS2 | Remove | Remove only | diff --git a/docs/question-flows.md b/docs/question-flows.md index 860699a39..72e660248 100644 --- a/docs/question-flows.md +++ b/docs/question-flows.md @@ -36,13 +36,13 @@ The journey feature (pupil change requests) is driven by JSON config files that | Field | Required | Values | Notes | |---|---|---|---| -| `pupilFilter` | yes | `"Included"` / `"All"` | `Included` limits results to pupils stamped `INCLUDED`; `All` returns every pupil for the school | +| `pupilFilter` | yes | `"Included"` / `"All"` / `"Includable"` / `"NotIncludable"` | `Included` limits results to pupils stamped `INCLUDED`; `All` returns every pupil for the school; `Includable` returns pupils stamped `INCLUDABLE` — the Include journey's search, which on 16-19 adds the included file's P_INCL 502 students to the non-included file (#711); `NotIncludable` is the rest, which the Include journey's "already included" guards look a name up in | | `pupilKey` | yes | `"primary"` / `"match"` | `primary` saves to `RequestState.SelectedPupil*` and generates the reference number; `match` saves to `RequestState.MatchedPupil*` | | `requireResults` | no | `true` / `false` (default) | Limits the search to students the school holds a 16-19 result for. Independent of `pupilFilter`, which selects by inclusion status. Set on the results-enquiry student search — there is no grade to correct for a student with no result. The page must say so in its `subheading`, because the restriction silently hides students | | `nextPageId` | no | page id string | Absent means redirect to Summary after selection | | `validationFailure` | no | string (supports `{pupilName}`) | Error shown when no pupil is submitted. Falls back to `"Enter the name of the pupil"` | -**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All&excludePupilId={guid}&requireResults=true` (served by `PupilSuggestionsController`). The `match` pupil page automatically passes the primary pupil's ID as `excludePupilId` so it cannot be selected twice. `requireResults` is sent only when the page config sets it; it is a search restriction, never a permission — it can reach no pupil outside the signed-in school's own file. When it is on, the autocomplete's no-match text changes to "No students found with results" so a school can tell a typo from a student who holds nothing. +**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All|Includable|NotIncludable&excludePupilId={guid}&requireResults=true` (served by `PupilSuggestionsController`). The `match` pupil page automatically passes the primary pupil's ID as `excludePupilId` so it cannot be selected twice. `requireResults` is sent only when the page config sets it; it is a search restriction, never a permission — it can reach no pupil outside the signed-in school's own file. When it is on, the autocomplete's no-match text changes to "No students found with results" so a school can tell a typo from a student who holds nothing. **Navigation:** `JourneyController.Page` (GET) transparently dispatches to `PupilSearchPage` when the requested page has `Type == PupilSearch`, so all navigation (summary redirects, `GetNextPageId`, navigation guard) works uniformly. diff --git a/docs/request-journey.md b/docs/request-journey.md index 761ff52ee..cb5fa05de 100644 --- a/docs/request-journey.md +++ b/docs/request-journey.md @@ -120,7 +120,7 @@ All flow configs begin with one or more `PupilSearch` pages. These are full-page | Field | Required | Values | Notes | |---|---|---|---| -| `pupilFilter` | yes | `"Included"` / `"All"` | `Included` limits results to Pincl codes `[401,403,414,421,431]`; `All` returns every pupil for the school | +| `pupilFilter` | yes | `"Included"` / `"All"` / `"Includable"` / `"NotIncludable"` | `Included` limits results to pupils stamped `INCLUDED`; `All` returns every pupil for the school; `Includable` returns pupils stamped `INCLUDABLE` — the Include journey's search, which on 16-19 adds the included file's P_INCL 502 students to the non-included file (#711); `NotIncludable` is the rest, which the Include journey's "already included" guards look a name up in | | `pupilKey` | yes | `"primary"` / `"match"` | Controls which session field is populated (see below) | | `requireResults` | no | `true` / `false` (default) | Limits the search to students the school holds a 16-19 result for. Independent of `pupilFilter`, which selects by inclusion status. Used by the results enquiry — there is no grade to correct for a student with no result. A page that sets it must say so in its `subheading`, because the restriction hides students silently | | `pupilSearchField` | no | `"All"` / `"CypmdId"` (default `All`) | Which field(s) the query is matched against. `All` (or absent) matches forename, surname, split names, UPN/ULN and CYPMD ID, plus date of birth on 16-19. `CypmdId` matches **only** a trimmed, case-insensitive prefix of `Cypmd_Id` — a name, UPN or date of birth returns nothing. Set on `Merge_KS4June.json`'s `select-match-pupil`, whose `subheading` already reads "Start typing ID to search records"; had it said nothing, narrowing the search would hide records silently and read as a typo | @@ -129,7 +129,7 @@ All flow configs begin with one or more `PupilSearch` pages. These are full-page `pupilSearchField` also changes what the suggestion text says. On a `CypmdId` page a KS4 suggestion is `"Surname, Firstname, DOB (Cypmd_Id)"`, because the KS4 suggestion leads with name and date of birth — which are identical across the duplicate records a merge exists to reconcile, so without the ID two rows in the dropdown cannot be told apart. `All` keeps the original `"Surname, Firstname, DOB"`; 16-19 is unchanged either way, as it already prints `CYPMD ID:`. The narrowed value is a property of one page: the merge journey's `select-pupil` page and every other journey keep `All`. -**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All&excludePupilId={guid}&requireResults=true&pupilSearchField=All|CypmdId` +**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All|Includable|NotIncludable&excludePupilId={guid}&requireResults=true&pupilSearchField=All|CypmdId` Served by `PupilSuggestionsController`. Reads the school's pupils from the cached per-school-window blob through `ICheckYourPupilDataService.GetPupilSuggestionsAsync` (`CheckYourPupilDataRepository`); it does **not** query PostgreSQL — the `Pupils` table was dropped when pupil data moved to blob storage. The `match` pupil page automatically passes the primary pupil's ID as `excludePupilId` so the same pupil cannot be selected twice. `requireResults` is sent only when the page config sets it. The service resolves the school's set of CYPMD ids with results (`IResultsClient.GetCypmdIdsWithResultsAsync`, served from the same cached results file the enquiry itself reads) and passes it to `SearchPupilsAsync` as an allow-list, which applies it **before** the ten-suggestion cap — filtering afterwards would drop the one student who holds results whenever ten who do not sort ahead of them. It is a search restriction, never a permission: it can reach no pupil outside the signed-in school's own file. With it on, the autocomplete's no-match text becomes "No students found with results", so a school can tell a typo from a student who holds nothing. @@ -164,6 +164,8 @@ POST → saves MatchedPupil → no nextPageId → redirects to Summary ``` +**Include** uses one `PupilSearch` page with `pupilFilter: "Includable"` (the `INCLUDABLE` stamp: not included, or a code the schema names — 16-19 P_INCL 502), then an evidence page; `Include_KS4June.json` and `Include_Post16.json` (#711). + **Session after primary PupilSearch page:** | Field | Value | |---|---| diff --git a/docs/technical-stakeholders-functional-overview.md b/docs/technical-stakeholders-functional-overview.md index f64c5985f..6ece09496 100644 --- a/docs/technical-stakeholders-functional-overview.md +++ b/docs/technical-stakeholders-functional-overview.md @@ -78,7 +78,7 @@ The journey is data-driven from JSON flow configs: - **Types** (`Application/Journey/`): `QuestionFlowConfig` → `JourneyPage` (`PageType`: `Question`, `Content`, `EvidenceUpload`, `PupilSearch`) → `Question` (`Radio`, `FreeText`, `Date`, `FileUpload`, `TextArea`, `Autocomplete`; supports `Optional`, `CharacterLimit`, named `Validator`, per-option `NextPageId` branching, `VisibleWhen` conditions). - **Config source**: the files that ship in the release image at `Web/Data/QuestionFlows/{WhatToChange}_{CheckingWindowType}.json`, read via `IQuestionFlowConfigSource` / `FileSystemQuestionFlowClient` and cached in `IMemoryCache` with `NeverRemove` priority (flow edits need a redeploy). There is **no blob container and no seeding step** — a flow reaches an environment by deploying the commit that contains it. See `docs/question-flow-deployment.md`. -- **Flows that exist today**: `Include_KS4June.json`, `Merge_KS4June.json`, `Remove_KS4June.json` (12-reason branch tree). **Only KS4 June has flows** — for KS2/Post16/KS4Autumn the config lookup returns null and `WhatToChangeController.Confirm` bounces back to the pupil list. +- **Flows that exist today**: KS4 June — `Add_KS4June.json`, `Include_KS4June.json`, `Merge_KS4June.json`, `Remove_KS4June.json`; KS4 Autumn — `Add_KS4Autumn.json`; KS2 — `Remove_KS2.json`; 16-19 — `Include_Post16.json` (#711), `Merge_Post16.json`, `Remove_Post16.json`, plus the results-enquiry flows `IncorrectGrade_Post16.json`, `MissingQualification_Post16.json`, `ResultDoesNotBelong_Post16.json`. What to change offers exactly the pupil-data changes whose flow file exists for the window type (`IQuestionFlowService.GetPupilDataChangesAsync`); a posted change with no flow file is refused. - **State**: `RequestState` per window in session — answers keyed by question id, plus `QuestionHistory` (ordered visited pages). Session is a **Postgres distributed cache** (`AddDistributedPostgreSqlCache`, table `session_cache`) so journeys survive load-balancing across replicas. Auth cookie is 30-min sliding; there is **no auto-save** — drafts persist only on explicit "Save and continue later". - **Mechanics** (`JourneyController`, ~750 lines): navigation guard prevents URL-hacking ahead of answered history; changing a branch answer trims downstream history; `fromSummary=true` returns to Summary after single-page edits unless the branch changed. Evidence upload: **PDF only** (verified by PdfPig page counting in `Web/FileStorage/PdfPageCounter.cs`), **10 MB max per file**; the page-count cap exists but is **disabled** (`maxEvidencePages = 0` at `Application/DependencyManager.cs`). Validation failures emit `ValidationErrorEvent` analytics with coded reasons. - **Reference numbers**: `CYPMD_{CheckingWindowType}_{7-char-uppercase-GUID-fragment}` from `JourneyValidationService.GenerateReference`. diff --git a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/IPupilRecord.cs b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/IPupilRecord.cs index 3ee28a9e0..3155dfb64 100644 --- a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/IPupilRecord.cs +++ b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/IPupilRecord.cs @@ -36,6 +36,15 @@ public interface IPupilRecord /// bool IsIncluded { get; } + /// + /// Whether a school may ask to include this pupil: the INCLUDABLE stamp ingress writes + /// (#711). Not included, or a code the schema's x-ingress.includable rule names — on + /// 16-19, a student in the included file who is not in the performance measures. Never a code + /// here: see + /// . + /// + bool IsIncludable { get; } + /// KS4 add-back semantics (see PupilIsAddBackCondition). Null for Post16. int? Pincl { get; } } diff --git a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/Post16PupilRecord.cs b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/Post16PupilRecord.cs index 8e8df2f0d..817be510d 100644 --- a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/Post16PupilRecord.cs +++ b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/Post16PupilRecord.cs @@ -10,7 +10,9 @@ namespace DfE.CheckPerformanceData.Application.CheckYourPupilData; /// /// Ingress stamps an INCLUDED boolean from the file of origin, and that alone decides /// : every student in the included file is included, every one in the -/// non-included file is not, whatever P_INCL says. +/// non-included file is not, whatever P_INCL says. A second stamp, INCLUDABLE, says who may be asked to be +/// included: every non-included student, and included students whose P_INCL is a code the +/// schema's x-ingress.includable rule names (#711). /// /// Field names differ from KS4: FORENAMES (plural), and identity is ULN — 16-19 /// has no UPN. URN/UKPRN are strings here (the supplier declares them varchar and @@ -33,6 +35,12 @@ public sealed class Post16PupilRecord : IPupilRecord [JsonPropertyName("INCLUDED")] public bool Included { get; init; } + /// Stamped by ingress (#711) from the included file's x-ingress.includable + /// rule and, for the non-included file, from INCLUDED being false. Decides + /// who the Include journey can pick; it never changes . + [JsonPropertyName("INCLUDABLE")] + public bool Includable { get; init; } + [JsonPropertyName("CYPMD_ID")] public string Cypmd_Id { get; init; } = string.Empty; @@ -94,4 +102,7 @@ public sealed class Post16PupilRecord : IPupilRecord [JsonIgnore] public bool IsIncluded => Included; + + [JsonIgnore] + public bool IsIncludable => Includable; } diff --git a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilInclusion.cs b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilInclusion.cs index b97329241..f64acb463 100644 --- a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilInclusion.cs +++ b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilInclusion.cs @@ -5,6 +5,8 @@ namespace DfE.CheckPerformanceData.Application.CheckYourPupilData; /// record: from the slot's file of origin (16-19 included and non-included files), or else from the /// schema's x-ingress.inclusion rule (KS4 and KS2, whose files carry each pupil's own code). /// Readers read only the stamp, so no reader knows any key stage's codes. +/// A second stamp, INCLUDABLE (#711), says who a school may ask to include: ingress writes it +/// from the schema's x-ingress.includable rule, or as not-included where there is no rule. /// public static class PupilInclusion { diff --git a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilRecord.cs b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilRecord.cs index 49280cc31..f2824dcdd 100644 --- a/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilRecord.cs +++ b/src/DfE.CheckPerformanceData.Application/CheckYourPupilData/PupilRecord.cs @@ -63,6 +63,11 @@ public sealed class PupilRecord : IPupilRecord [JsonPropertyName("INCLUDED")] public bool Included { get; init; } + /// Stamped by ingress (#711): true when a school may ask to include this pupil. On + /// KS4 and KS2 that is "not included". Absent means not includable. + [JsonPropertyName("INCLUDABLE")] + public bool Includable { get; init; } + /// The words schools see for , stamped by ingress from the schema's /// inclusion rule when the supplier file does not carry them. [JsonPropertyName("P_INCL_DESC")] @@ -98,4 +103,7 @@ public sealed class PupilRecord : IPupilRecord [JsonIgnore] public bool IsIncluded => Included; + + [JsonIgnore] + public bool IsIncludable => Includable; } diff --git a/src/DfE.CheckPerformanceData.Application/Journey/PupilFilter.cs b/src/DfE.CheckPerformanceData.Application/Journey/PupilFilter.cs index 6153c63b9..4322fc623 100644 --- a/src/DfE.CheckPerformanceData.Application/Journey/PupilFilter.cs +++ b/src/DfE.CheckPerformanceData.Application/Journey/PupilFilter.cs @@ -1,3 +1,10 @@ namespace DfE.CheckPerformanceData.Application.Journey; -public enum PupilFilter { Included, All, NonIncluded } +/// +/// Which pupils a search returns. and read the +/// INCLUDED stamp. and read the +/// INCLUDABLE stamp (#711): the Include journey searches — on +/// 16-19 that adds the included-file students the schema's rule names to the non-included file — and its +/// "already included" guards look a name up among . +/// +public enum PupilFilter { Included, All, Includable, NotIncludable } diff --git a/src/DfE.CheckPerformanceData.Infrastructure/Ingress/CsvSchemaFileProcessor.cs b/src/DfE.CheckPerformanceData.Infrastructure/Ingress/CsvSchemaFileProcessor.cs index a57cc9a5f..24dd0ac50 100644 --- a/src/DfE.CheckPerformanceData.Infrastructure/Ingress/CsvSchemaFileProcessor.cs +++ b/src/DfE.CheckPerformanceData.Infrastructure/Ingress/CsvSchemaFileProcessor.cs @@ -193,6 +193,25 @@ public async IAsyncEnumerable ProcessAsync( yield break; } + // #711: who may be asked to be included. Validated like the inclusion rule, so a schema + // that cannot be honoured fails by name before any row is read. + IncludableRule? includable = IncludableRule.From(schema); + if (includable?.ErrorFor(schema) is { } includableError) + { + summaryCapture?.SetFailure("schema", "identifier_mapping_invalid"); + yield return Failed($"Schema '{dataset.SchemaFile}': {includableError}"); + yield break; + } + + // INCLUDABLE is derived from INCLUDED. A pupil file that declares only INCLUDED would + // ingest cleanly and then offer nobody for inclusion, so the pair is required together. + if (schema.Properties.ContainsKey("INCLUDED") && !schema.Properties.ContainsKey("INCLUDABLE")) + { + summaryCapture?.SetFailure("schema", "includable_not_declared"); + yield return Failed($"Schema '{dataset.SchemaFile}': it declares INCLUDED but not INCLUDABLE, so it would offer nobody for inclusion."); + yield break; + } + // Capture each complete source record before any schema transformation. CsvHelper // counts CSV records, so a quoted field containing line breaks remains one row. List> records = []; @@ -308,6 +327,16 @@ public async IAsyncEnumerable ProcessAsync( record["INCLUDED"] = inclusion.IsIncluded(record); } + // #711: who may be asked to be included — not included, or a code the schema's + // x-ingress.includable rule names (only the 16-19 included file has one). Derived + // from INCLUDED, so it is stamped only when INCLUDED was, and only when the schema + // declares it (AllowAdditionalProperties is false). Readers read the stamp. + if (schema.Properties.ContainsKey("INCLUDABLE") && + record["INCLUDED"] is { Type: JTokenType.Boolean } stampedIncluded) + { + record["INCLUDABLE"] = !stampedIncluded.Value() || (includable?.Matches(record) ?? false); + } + // Provenance by file of origin, the exact analogue of INCLUDED above: the // results CSVs carry no SOURCE column, so the tag comes from the dataset slot // the file was uploaded to. Stamped BEFORE validation for the same reason @@ -818,6 +847,38 @@ private string Code(JObject record) => record[Field] is { Type: not JTokenType.Null } value ? value.ToString().Trim() : string.Empty; } + /// + /// Which records a school may ask to include although their file counts them as included: + /// "x-ingress": { "includable": { "field": "P_INCL", "codes": ["502"] } } at the + /// schema's root. Codes are compared as trimmed text. A record that is not included is + /// includable without a rule; the rule only adds to that (#711). + /// + private sealed record IncludableRule(string Field, IReadOnlySet Codes) + { + public static IncludableRule? From(JSchema schema) + { + if (!schema.ExtensionData.TryGetValue("x-ingress", out JToken? ingress) || + ingress["includable"] is not JObject rule) + return null; + + return new IncludableRule( + rule["field"]?.Value() ?? string.Empty, + (rule["codes"] as JArray ?? []).Select(code => code.ToString().Trim()).ToHashSet(StringComparer.Ordinal)); + } + + public string? ErrorFor(JSchema schema) => + string.IsNullOrWhiteSpace(Field) ? "x-ingress.includable names no field." + : !schema.Properties.ContainsKey(Field) ? $"x-ingress.includable reads {Field}, which the schema does not declare." + : !schema.Properties.ContainsKey("INCLUDABLE") ? "x-ingress.includable needs the schema to declare INCLUDABLE, the stamp it writes." + // A rule with no codes adds nobody, and a blank code would match every empty field. + : Codes.Count == 0 ? "x-ingress.includable names no codes." + : Codes.Contains(string.Empty) ? "x-ingress.includable names a blank code." + : null; + + public bool Matches(JObject record) => + Codes.Contains(record[Field] is { Type: not JTokenType.Null } value ? value.ToString().Trim() : string.Empty); + } + private static IReadOnlyList SourceColumns(JSchema schema) { List sources = []; diff --git a/src/DfE.CheckPerformanceData.Persistence/Repositories/CheckYourPupilDataRepository.cs b/src/DfE.CheckPerformanceData.Persistence/Repositories/CheckYourPupilDataRepository.cs index be2177eb5..cb45b9367 100644 --- a/src/DfE.CheckPerformanceData.Persistence/Repositories/CheckYourPupilDataRepository.cs +++ b/src/DfE.CheckPerformanceData.Persistence/Repositories/CheckYourPupilDataRepository.cs @@ -174,7 +174,9 @@ public async Task> SearchPupilsAsync(Guid wind { PupilFilter.All => true, PupilFilter.Included => p.IsIncluded, - _ => !p.IsIncluded + PupilFilter.Includable => p.IsIncludable, + PupilFilter.NotIncludable => !p.IsIncludable, + _ => throw new ArgumentOutOfRangeException(nameof(filter), filter, "No search rule for this pupil filter.") }) .Where(matches); diff --git a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeAlreadyIncludedViewModel.cs b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeAlreadyIncludedViewModel.cs index 4303ec50c..e68bc777a 100644 --- a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeAlreadyIncludedViewModel.cs +++ b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeAlreadyIncludedViewModel.cs @@ -1,6 +1,7 @@ namespace DfE.CheckPerformanceData.Web.Controllers.Journey; using DfE.CheckPerformanceData.Application.CheckYourPupilData; +using DfE.CheckPerformanceData.Application.WindowManagement; /// /// View model for the Include journey's "already included" warning page. Shown when the typed @@ -27,4 +28,11 @@ public sealed class IncludeAlreadyIncludedViewModel /// The pupils found on the included list matching the typed entry. Display only — /// never logged or placed in analytics. Empty when no match details were carried. public IReadOnlyList Matches { get; set; } = []; + + /// The window's word for a learner: "student" on 16-19, "pupil" elsewhere. + public LearnerNoun LearnerNoun { get; set; } = LearnerNoun.Pupil; + + /// True when the window type has an Add flow, so "Start adding" leads somewhere. + /// 16-19 has none (#711). + public bool CanAdd { get; set; } } \ No newline at end of file diff --git a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeNoResultsViewModel.cs b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeNoResultsViewModel.cs index e7ba98769..8bc2369d8 100644 --- a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeNoResultsViewModel.cs +++ b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/IncludeNoResultsViewModel.cs @@ -1,3 +1,5 @@ +using DfE.CheckPerformanceData.Application.WindowManagement; + namespace DfE.CheckPerformanceData.Web.Controllers.Journey; /// @@ -15,4 +17,11 @@ public sealed class IncludeNoResultsViewModel /// The name the user typed. Display only — never logged or placed in analytics. public string? TypedPupilLabel { get; set; } + + /// The window's word for a learner: "student" on 16-19, "pupil" elsewhere. + public LearnerNoun LearnerNoun { get; set; } = LearnerNoun.Pupil; + + /// True when the window type has an Add flow, so "Start adding" leads somewhere. + /// 16-19 has none (#711). + public bool CanAdd { get; set; } } \ No newline at end of file diff --git a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/JourneyController.cs b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/JourneyController.cs index ae198df17..736f66d3c 100644 --- a/src/DfE.CheckPerformanceData.Web/Controllers/Journey/JourneyController.cs +++ b/src/DfE.CheckPerformanceData.Web/Controllers/Journey/JourneyController.cs @@ -153,11 +153,14 @@ public async Task PupilSearchPost(Guid windowId, string pageId, s // AB#027: Include journey no-results path. When the user typed a name on the Include // primary search but did not select a suggestion, look the typed entry up against BOTH // the included and non-included populations and branch three ways: - // - included match → warn "already included" (abort); - // - non-included-only match → a valid include candidate, so proceed as normal: fall + // - a match among pupils who cannot be included (NotIncludable — included for good) + // → warn "already included" (abort); + // - a match among includable pupils only (#711: not included, or on 16-19 an included-file + // student the schema's rule names) → a valid include candidate, so proceed as normal: fall // through to the ordinary no-selection flow (re-render the search page with the // candidate's suggestions visible) with no warning and no decision page; - // - match on neither list → offer "Pupil not found" (start Add / search again). + // - match on neither → offer "Pupil not found" (start Add where the window type has an + // Add flow / search again). // Blank entry keeps the existing validation, and so does a lookup failure (never a // dead end): the decision text above only ever runs for a deliberately typed name. if (!string.IsNullOrWhiteSpace(selectedPupilLabel) @@ -168,19 +171,19 @@ public async Task PupilSearchPost(Guid windowId, string pageId, s // web server or proxy would log it): it is PII. HttpContext.Session.SaveRequestState(windowId, s => s.IncludeSearchLabel = selectedPupilLabel.Trim()); - IReadOnlyList? includedSuggestions = null; + IReadOnlyList? notIncludableSuggestions = null; try { - includedSuggestions = await pupilDataService.GetPupilSuggestionsAsync( - windowId, selectedPupilLabel, PupilFilter.Included); + notIncludableSuggestions = await pupilDataService.GetPupilSuggestionsAsync( + windowId, selectedPupilLabel, PupilFilter.NotIncludable); } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogError(ex, "Included-pupil lookup failed for Include no-results decision; falling back to validation. windowId={WindowId}", windowId); - includedSuggestions = null; + logger.LogError(ex, "Not-includable lookup failed for Include no-results decision; falling back to validation. windowId={WindowId}", windowId); + notIncludableSuggestions = null; } - if (includedSuggestions is null) + if (notIncludableSuggestions is null) { // Lookup failed/timed out → fall through to the existing validation below // (never a redirect to the decision pages, so never a dead end). @@ -195,29 +198,29 @@ await analytics.TrackSafeAsync(new ValidationErrorEvent return View("PupilSearch", viewModelBuilder.BuildPupilSearchVm(windowId, pageId, page, journey, config)); } - if (includedSuggestions.Count > 0) + if (notIncludableSuggestions.Count > 0) { // Carry the matching included pupils across the post/redirect/get round-trip in // session (not the URL, where a web server or proxy would log them): they are PII. HttpContext.Session.SaveRequestState(windowId, s => - s.IncludeMatchedPupils = includedSuggestions.ToList()); + s.IncludeMatchedPupils = notIncludableSuggestions.ToList()); return RedirectToAction(nameof(AlreadyIncluded), new { windowId, pageId }); } - IReadOnlyList? nonIncludedSuggestions = null; + IReadOnlyList? includableSuggestions = null; try { - nonIncludedSuggestions = await pupilDataService.GetPupilSuggestionsAsync( - windowId, selectedPupilLabel, PupilFilter.NonIncluded); + includableSuggestions = await pupilDataService.GetPupilSuggestionsAsync( + windowId, selectedPupilLabel, PupilFilter.Includable); } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogError(ex, "Non-included-pupil lookup failed for Include no-results decision; falling back to validation. windowId={WindowId}", windowId); - nonIncludedSuggestions = null; + logger.LogError(ex, "Includable lookup failed for Include no-results decision; falling back to validation. windowId={WindowId}", windowId); + includableSuggestions = null; } - if (nonIncludedSuggestions is null) + if (includableSuggestions is null) { // Lookup failed/timed out → fall through to the existing validation below // (never a redirect to the decision pages, so never a dead end). @@ -232,7 +235,7 @@ await analytics.TrackSafeAsync(new ValidationErrorEvent return View("PupilSearch", viewModelBuilder.BuildPupilSearchVm(windowId, pageId, page, journey, config)); } - if (nonIncludedSuggestions.Count == 0) + if (includableSuggestions.Count == 0) return RedirectToAction(nameof(PupilNotFound), new { windowId, pageId }); // A non-included-only match is a valid include candidate: proceed as normal. Fall @@ -274,29 +277,31 @@ await analytics.TrackSafeAsync(new ValidationErrorEvent // check the selected name against the included list too — a different pupil with the same // name may already be on the included list, and the select-from-dropdown path would // otherwise bypass the included-list guard the typed-entry path (lines 160-239) enforces. - // The search page's autocomplete filters to NonIncluded, so the *selected* record itself is - // non-included; the guard therefore looks up the typed label, exactly as the typed path does. + // The search page's autocomplete filters to Includable, so the *selected* record itself is + // includable and can never match this NotIncludable lookup (#711: a 16-19 student can be + // INCLUDED by file but includable by rule, so an Included lookup would find them and block + // their own request); the guard looks up the typed label, exactly as the typed path does. if (journey.SelectedWhatToChange == WhatToChange.Include && page.PupilKey == JourneyPage.PrimaryKey) { - IReadOnlyList? includedSuggestions = null; + IReadOnlyList? notIncludableSuggestions = null; try { - includedSuggestions = await pupilDataService.GetPupilSuggestionsAsync( - windowId, $"{pupil.Firstname} {pupil.Surname}".Trim(), PupilFilter.Included); + notIncludableSuggestions = await pupilDataService.GetPupilSuggestionsAsync( + windowId, $"{pupil.Firstname} {pupil.Surname}".Trim(), PupilFilter.NotIncludable); } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogError(ex, "Included-pupil lookup failed for Include autocomplete guard; proceeding as normal. windowId={WindowId}", windowId); - includedSuggestions = null; + logger.LogError(ex, "Not-includable lookup failed for Include autocomplete guard; proceeding as normal. windowId={WindowId}", windowId); + notIncludableSuggestions = null; } - if (includedSuggestions is not null && includedSuggestions.Count > 0) + if (notIncludableSuggestions is not null && notIncludableSuggestions.Count > 0) { var displayLabel = selectedPupilLabel ?? $"{pupil.Surname}, {pupil.Firstname}, {pupil.DateOfBirth}"; HttpContext.Session.SaveRequestState(windowId, s => { s.IncludeSearchLabel = displayLabel; - s.IncludeMatchedPupils = includedSuggestions.ToList(); + s.IncludeMatchedPupils = notIncludableSuggestions.ToList(); }); return RedirectToAction(nameof(AlreadyIncluded), new { windowId, pageId }); } @@ -1797,25 +1802,25 @@ await analytics.TrackSafeAsync(new DuplicateCheckDecisionEvent // Include flow's own select-pupil search page. The duplicate check may surface a non-included // pupil whose name matches a pupil already on the included list, and switching to include must // not silently bypass the warning the search page enforces. - IReadOnlyList? includedSuggestions = null; + IReadOnlyList? notIncludableSuggestions = null; try { - includedSuggestions = await pupilDataService.GetPupilSuggestionsAsync( - windowId, $"{pupil.Firstname} {pupil.Surname}".Trim(), PupilFilter.Included); + notIncludableSuggestions = await pupilDataService.GetPupilSuggestionsAsync( + windowId, $"{pupil.Firstname} {pupil.Surname}".Trim(), PupilFilter.NotIncludable); } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogError(ex, "Included-pupil lookup failed for Include hand-off guard; proceeding as normal. windowId={WindowId}", windowId); - includedSuggestions = null; + logger.LogError(ex, "Not-includable lookup failed for Include hand-off guard; proceeding as normal. windowId={WindowId}", windowId); + notIncludableSuggestions = null; } - if (includedSuggestions is not null && includedSuggestions.Count > 0) + if (notIncludableSuggestions is not null && notIncludableSuggestions.Count > 0) { // The duplicate-check page already surfaces each match's Included/Not-included status, so a // same-name included pupil that was already on screen is not a bypass — only block when the // included namesake is new information (e.g. a different-DOB pupil the check did not surface). var alreadySurfaced = journey.DuplicateCheck?.Matches.Select(m => m.Id).ToHashSet() ?? []; - var hiddenNamesakes = includedSuggestions.Where(s => !alreadySurfaced.Contains(s.Id)).ToList(); + var hiddenNamesakes = notIncludableSuggestions.Where(s => !alreadySurfaced.Contains(s.Id)).ToList(); if (hiddenNamesakes.Count > 0) { var displayLabel = $"{pupil.Surname}, {pupil.Firstname}, {pupil.DateOfBirth}"; @@ -1921,6 +1926,14 @@ await analytics.TrackSafeAsync(new ValidationErrorEvent // ── Include no-results decision flow (AB#027) ──────────────────────────── + /// + /// #711: the decision pages offer "Start adding" only when What to change would offer Add — + /// the same list, so the two can never disagree. 16-19 has no Add flow. + /// + private async Task CanAddAsync(RequestState journey) => + journey.CheckingWindow?.CheckingWindowType is { } windowType + && (await flowService.GetPupilDataChangesAsync(windowType)).Contains(WhatToChange.Add); + /// /// AB#027 (US1): shown when the Include journey's select-pupil search found no included match /// for a freely typed (not-selected) name. Offers "Start adding this pupil" (the Add journey) @@ -1943,7 +1956,9 @@ public async Task PupilNotFound(Guid windowId, string pageId) { WindowId = windowId, PageId = pageId, - TypedPupilLabel = label + TypedPupilLabel = label, + LearnerNoun = journey.LearnerNoun, + CanAdd = await CanAddAsync(journey) }); } @@ -1975,7 +1990,9 @@ public async Task AlreadyIncluded(Guid windowId, string? pageId = TypedPupilLabel = label, Matches = matches, BackPageId = pageId, - BackPageAction = backAction ?? nameof(PupilSearchPage) + BackPageAction = backAction ?? nameof(PupilSearchPage), + LearnerNoun = journey.LearnerNoun, + CanAdd = await CanAddAsync(journey) }); } diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils-december_schema.json b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils-december_schema.json index 5520ad49b..a96425ed5 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils-december_schema.json +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils-december_schema.json @@ -45,6 +45,19 @@ "order": 999 } }, + "INCLUDABLE": { + "type": "boolean", + "description": "Stamped by ingress: true when a school may ask to include this pupil - not included, or P_INCL is a code x-ingress.includable names. Not a supplier column, so not downloaded or shown.", + "x-csv": { + "columns": {}, + "heading": "Includable" + }, + "x-display": { + "label": "Includable", + "visible": false, + "order": 999 + } + }, "LAESTAB": { "type": [ "string", diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils_schema.json b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils_schema.json index d7d0d1fae..13e616fda 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils_schema.json +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks2/pupils_schema.json @@ -45,6 +45,19 @@ "order": 999 } }, + "INCLUDABLE": { + "type": "boolean", + "description": "Stamped by ingress: true when a school may ask to include this pupil - not included, or P_INCL is a code x-ingress.includable names. Not a supplier column, so not downloaded or shown.", + "x-csv": { + "columns": {}, + "heading": "Includable" + }, + "x-display": { + "label": "Includable", + "visible": false, + "order": 999 + } + }, "LAESTAB": { "type": [ "string", diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks4june/pupils_schema.json b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks4june/pupils_schema.json index 996eb68e3..dcc4fad0b 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks4june/pupils_schema.json +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/ks4june/pupils_schema.json @@ -59,6 +59,19 @@ "order": 999 } }, + "INCLUDABLE": { + "type": "boolean", + "description": "Stamped by ingress: true when a school may ask to include this pupil - not included, or P_INCL is a code x-ingress.includable names. Not a supplier column, so not downloaded or shown.", + "x-csv": { + "columns": {}, + "heading": "Includable" + }, + "x-display": { + "label": "Includable", + "visible": false, + "order": 999 + } + }, "UPN": { "type": [ "string", diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/README.md b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/README.md index 11abddcf5..67c3623e4 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/README.md +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/README.md @@ -7,6 +7,7 @@ These row schemas transcribe the field references, SQL types, CSV column letters Each row schema uses the same extensions: - `x-ingress.collection` is the key in the combined JSON; `pageTab` groups collections on the page; `institutionKey` identifies the source field used to split incoming records by institution. +- `x-ingress.includable` (#711), on `students-included_schema.json` only, names the `P_INCL` codes of students a school may ask to include although they are in the included file: `502`, "Student not included in performance measures". Ingress stamps `INCLUDABLE` from it; every row of the non-included file is includable without a rule. - `x-download.fileName` and `properties.*.x-csv.columns` identify the separate CSV and its column order. The variant keys distinguish provisional/revised from retention columns where the workbook does. - `x-display.section` gives the selector label; `properties.*.x-display` gives the page label, visibility, order and, for names, searchability. Only selected pupil identity fields are marked visible by default. Other fields remain available for a details view. diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-included_schema.json b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-included_schema.json index 69bbb39a9..6ecff46ec 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-included_schema.json +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-included_schema.json @@ -8,7 +8,8 @@ "x-ingress": { "collection": "students-included", "pageTab": "students", - "institutionKey": "LAESTAB" + "institutionKey": "LAESTAB", + "includable": { "field": "P_INCL", "codes": ["502"] } }, "x-download": { "fileName": "ks5pup25.csv", @@ -26,6 +27,19 @@ "x-csv": { "columns": {}, "heading": "Included" }, "x-display": { "label": "Included", "visible": false, "order": 999 } }, + "INCLUDABLE": { + "type": "boolean", + "description": "Stamped by ingress: true when a school may ask to include this pupil - not included, or P_INCL is a code x-ingress.includable names. Not a supplier column, so not downloaded or shown.", + "x-csv": { + "columns": {}, + "heading": "Includable" + }, + "x-display": { + "label": "Includable", + "visible": false, + "order": 999 + } + }, "CYPMD_ID": { "type": [ "string", diff --git a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-non-included_schema.json b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-non-included_schema.json index 80cc35948..00d63a743 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-non-included_schema.json +++ b/src/DfE.CheckPerformanceData.Web/Data/Ingress/post16/students-non-included_schema.json @@ -26,6 +26,19 @@ "x-csv": { "columns": {}, "heading": "Included" }, "x-display": { "label": "Included", "visible": false, "order": 999 } }, + "INCLUDABLE": { + "type": "boolean", + "description": "Stamped by ingress: true when a school may ask to include this pupil - not included, or P_INCL is a code x-ingress.includable names. Not a supplier column, so not downloaded or shown.", + "x-csv": { + "columns": {}, + "heading": "Includable" + }, + "x-display": { + "label": "Includable", + "visible": false, + "order": 999 + } + }, "SCHCNO": { "type": [ "string", diff --git a/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_KS4June.json b/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_KS4June.json index e93f457d3..3a7375a04 100644 --- a/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_KS4June.json +++ b/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_KS4June.json @@ -5,7 +5,7 @@ "id": "select-pupil", "type": "PupilSearch", "title": "What is the name of the pupil to be included?", - "pupilFilter": "NonIncluded", + "pupilFilter": "Includable", "pupilKey": "primary", "nextPageId": "evidence", "validationFailure": "Enter the name of the pupil to be included." diff --git a/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_Post16.json b/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_Post16.json new file mode 100644 index 000000000..054344393 --- /dev/null +++ b/src/DfE.CheckPerformanceData.Web/Data/QuestionFlows/Include_Post16.json @@ -0,0 +1,38 @@ +{ + "firstPageId": "select-pupil", + "pages": [ + { + "id": "select-pupil", + "type": "PupilSearch", + "title": "What is the name of the student to be included?", + "subheading": "Start typing the first name, last name, CYPMD ID, ULN or date of birth (DOB)", + "pupilFilter": "Includable", + "pupilKey": "primary", + "nextPageId": "evidence", + "validationFailure": "Enter the name of the student to be included." + }, + { + "id": "evidence", + "pageTitle": "Provide evidence for the inclusion", + "type": "EvidenceUpload", + "title": "Provide evidence for the inclusion of {pupilName}", + "questions": [ + { + "id": "evidence", + "type": "FileUpload", + "title": "Upload files", + "hint": "Evidence must be in a PDF format", + "optional": true + }, + { + "id": "how-evidence-supports", + "type": "TextArea", + "title": "Explain how the evidence supports including this student", + "summaryTitle": "How evidence supports student inclusion", + "characterLimit": 500, + "optional": true + } + ] + } + ] +} diff --git a/src/DfE.CheckPerformanceData.Web/Seeding/SeedExerciseFixtures.cs b/src/DfE.CheckPerformanceData.Web/Seeding/SeedExerciseFixtures.cs index fcf2cfccd..ffb45a1b0 100644 --- a/src/DfE.CheckPerformanceData.Web/Seeding/SeedExerciseFixtures.cs +++ b/src/DfE.CheckPerformanceData.Web/Seeding/SeedExerciseFixtures.cs @@ -91,7 +91,7 @@ internal static async Task IngestAsync(BlobServiceClient blobs, ICheckingExercis // Ours, not the supplier's: ingress generates a pupil's Id, and nothing reads the window id. // INCLUDED and P_INCL_DESC are stamped by ingress from the schema's inclusion rule. - private static readonly HashSet NotSupplierColumns = ["Id", "CheckingWindowId", "INCLUDED", "P_INCL_DESC"]; + private static readonly HashSet NotSupplierColumns = ["Id", "CheckingWindowId", "INCLUDED", "INCLUDABLE", "P_INCL_DESC"]; // One row per record, one column per JSON property, serialised exactly as the read models // bind them. Columns the schema does not declare are dropped by ingress. diff --git a/src/DfE.CheckPerformanceData.Web/Seeding/SeedKs2PupilData.cs b/src/DfE.CheckPerformanceData.Web/Seeding/SeedKs2PupilData.cs index 07f57213a..6584260da 100644 --- a/src/DfE.CheckPerformanceData.Web/Seeding/SeedKs2PupilData.cs +++ b/src/DfE.CheckPerformanceData.Web/Seeding/SeedKs2PupilData.cs @@ -37,7 +37,7 @@ public static class SeedKs2PupilData private const int NonIncludedPerSchool = 6; // Stamped by ingress from the schema's inclusion rule, never sent by the supplier. - private static readonly HashSet StampedByIngress = ["INCLUDED", "P_INCL_DESC"]; + private static readonly HashSet StampedByIngress = ["INCLUDED", "INCLUDABLE", "P_INCL_DESC"]; public static async Task ExecuteSeedAsync(IPortalDbContext dbContext, BlobServiceClient blobs, ICheckingExerciseIngress ingress, string contentRootPath) diff --git a/src/DfE.CheckPerformanceData.Web/Views/Journey/AlreadyIncluded.cshtml b/src/DfE.CheckPerformanceData.Web/Views/Journey/AlreadyIncluded.cshtml index a2ebc994f..0b40f9593 100644 --- a/src/DfE.CheckPerformanceData.Web/Views/Journey/AlreadyIncluded.cshtml +++ b/src/DfE.CheckPerformanceData.Web/Views/Journey/AlreadyIncluded.cshtml @@ -1,7 +1,7 @@ @model DfE.CheckPerformanceData.Web.Controllers.Journey.IncludeAlreadyIncludedViewModel @{ - ViewBag.Title = "This pupil is already included"; + ViewBag.Title = $"This {Model.LearnerNoun.Singular} is already included"; Layout = "_Layout"; } @@ -15,22 +15,22 @@ QuestionModels = [] }) -

This pupil is already included

+

This @Model.LearnerNoun.Singular is already included

- There may already be a pupil matching "@Model.TypedPupilLabel" on the included list. + There may already be a @Model.LearnerNoun.Singular matching "@Model.TypedPupilLabel" on the included list.

- No non-included pupils were found. The pupil may already be the subject of an active request on the included list. You do not need to include them again. + No @Model.LearnerNoun.Plural who can be included were found. The @Model.LearnerNoun.Singular may already be the subject of an active request on the included list. You do not need to include them again.

@if (Model.Matches.Count > 0) { -

Matching pupils on the included list

+

Matching @Model.LearnerNoun.Plural on the included list

- + @@ -53,14 +53,17 @@
Pupils matching "@Model.TypedPupilLabel"@Model.LearnerNoun.PluralCapitalised matching "@Model.TypedPupilLabel"
Name
} -

- If the pupil should still be included, you can add them as a new pupil instead. -

+@if (Model.CanAdd) +{ +

+ If the @Model.LearnerNoun.Singular should still be included, you can add them as a new @Model.LearnerNoun.Singular instead. +

-
- @Html.AntiForgeryToken() - Start adding this pupil -
+
+ @Html.AntiForgeryToken() + Start adding this @Model.LearnerNoun.Singular +
+}
@Html.AntiForgeryToken() diff --git a/src/DfE.CheckPerformanceData.Web/Views/Journey/PupilNotFound.cshtml b/src/DfE.CheckPerformanceData.Web/Views/Journey/PupilNotFound.cshtml index 67d19aef7..2d7bf8b7a 100644 --- a/src/DfE.CheckPerformanceData.Web/Views/Journey/PupilNotFound.cshtml +++ b/src/DfE.CheckPerformanceData.Web/Views/Journey/PupilNotFound.cshtml @@ -1,24 +1,33 @@ @model DfE.CheckPerformanceData.Web.Controllers.Journey.IncludeNoResultsViewModel @{ - ViewBag.Title = "Pupil not found"; + ViewBag.Title = $"{Model.LearnerNoun.SingularCapitalised} not found"; Layout = "_Layout"; } -

Pupil not found

+

@Model.LearnerNoun.SingularCapitalised not found

- No pupil matching "@Model.TypedPupilLabel" was found. This means the pupil is not currently + No @Model.LearnerNoun.Singular matching "@Model.TypedPupilLabel" was found. This means the @Model.LearnerNoun.Singular is not currently included and is not known to this school's records.

-

- You can add them as a new pupil or search again with a different name. -

+@if (Model.CanAdd) +{ +

+ You can add them as a new @Model.LearnerNoun.Singular or search again with a different name. +

- - @Html.AntiForgeryToken() - Start adding this pupil -
+
+ @Html.AntiForgeryToken() + Start adding this @Model.LearnerNoun.Singular +
+} +else +{ +

+ You can search again with a different name. +

+}
@Html.AntiForgeryToken() diff --git a/tests/DfE.CheckPerformanceData.E2ETests/Journey/IncludeNoResultsPathTests.cs b/tests/DfE.CheckPerformanceData.E2ETests/Journey/IncludeNoResultsPathTests.cs index ff1409c1f..b5169c3e1 100644 --- a/tests/DfE.CheckPerformanceData.E2ETests/Journey/IncludeNoResultsPathTests.cs +++ b/tests/DfE.CheckPerformanceData.E2ETests/Journey/IncludeNoResultsPathTests.cs @@ -108,7 +108,7 @@ public async Task NonIncludedMatch_ProceedsAsNormal_NoDecisionPage() { await StartIncludeJourneyAsync(); // Bob Johnson is a non-included pupil — a valid include candidate. The Include search's - // autocomplete targets NonIncluded pupils, so typing a single surname token ("Johnson" — + // autocomplete targets Includable pupils, so typing a single surname token ("Johnson" — // "Johnson" is a non-included surname bucket and matches no included pupil) and continuing // without selecting a suggestion must NOT divert to "Pupil not found" or "Already included": // the journey proceeds as normal, re-rendering the search page with the candidate's @@ -130,8 +130,8 @@ public async Task NonIncludedMatch_ProceedsAsNormal_NoDecisionPage() public async Task IncludedMatch_ShowsAlreadyIncluded_WithAbortOption() { await StartIncludeJourneyAsync(); - // Alice Smith is included. The Include search autocomplete targets NonIncluded pupils and - // the feature's included-list lookup reuses the single-field name matcher, so we type a + // Alice Smith is included. The Include search autocomplete targets Includable pupils and + // the guard's NotIncludable lookup reuses the single-field name matcher, so we type a // partial surname — "the user typed a partial name" (US2) — that resolves to her, and // submit WITHOUT selecting a suggestion. await TypePupilWithoutSelectingAsync(IncludedSurname); diff --git a/tests/DfE.CheckPerformanceData.E2ETests/Journey/Post16IncludeJourneyTests.cs b/tests/DfE.CheckPerformanceData.E2ETests/Journey/Post16IncludeJourneyTests.cs new file mode 100644 index 000000000..dc98545d9 --- /dev/null +++ b/tests/DfE.CheckPerformanceData.E2ETests/Journey/Post16IncludeJourneyTests.cs @@ -0,0 +1,143 @@ +using DfE.CheckPerformanceData.E2ETests.Fixtures; +using DfE.CheckPerformanceData.E2ETests.Helpers; +using Microsoft.Playwright; +using DfE.CheckPerformanceData.E2ETests.Retrying; + +namespace DfE.CheckPerformanceData.E2ETests.Journey; + +// #711 (AB#301154): the 16-19 Include journey on the seeded "16 to 19 Oct" window, as Kingsmead +// School (860/4070). Who can be picked is the INCLUDABLE stamp: every student in the non-included +// file, and the included file's P_INCL 502 students. The seed (SeedPupilData.GeneratePost16Pupils) +// gives the included file Alice Smith (501), Bob Smith (502), Charlie Smith (505) … and the +// non-included file Alice Johnson, Bob Johnson … +[Trait("Category", "FullRegression")] +[Collection("E2E")] +public sealed class Post16IncludeJourneyTests(PlaywrightFixture fixture) : SeedingPageTest(fixture) +{ + private static readonly Guid WindowId = Guid.Parse("EC6493B8-9B66-4090-8BE6-9DFC3805751F"); + + private const string NonIncludedStudent = "Alice Johnson"; // non-included file + private const string Code502Student = "Bob Smith"; // included file, P_INCL 502 + private const string Code501Student = "Alice Smith"; // included file, P_INCL 501 + private const string NoSuchName = "No Such"; + + [RetryFact] + [Trait("Category", "Smoke")] + public async Task NonIncludedStudent_CanBeIncluded_ThroughToTheConfirmation() + { + // A leftover request for this student would divert the search into the conflict banner. + await SeedHelpers.CleanupDevRequestsAsync(Fixture.SeedClient); + + await StartIncludeJourneyAsync(); + await SelectStudentAsync(NonIncludedStudent); + + // AC2/AC3: the evidence page, both answers optional — continue with nothing. + await Page.WaitForURLAsync($"**/Journey/{WindowId}/page/evidence"); + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })) + .ToContainTextAsync($"Provide evidence for the inclusion of {NonIncludedStudent}"); + await Expect(Page.GetByRole(AriaRole.Button, new() { Name = "Save and continue later" })).ToBeVisibleAsync(); + await ContinueAsync(); + + // AC6: the summary names the student, the change and a Change link. + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })) + .ToContainTextAsync("Summary of amendment request"); + var summary = Page.Locator(".govuk-summary-list"); + await Expect(summary).ToContainTextAsync("Include a student in data"); + var nameRow = Page.Locator(".govuk-summary-list__row", new() { HasText = "Student name" }); + await Expect(nameRow).ToContainTextAsync(NonIncludedStudent); + await Expect(nameRow.GetByRole(AriaRole.Link, new() { Name = "Change" })).ToBeVisibleAsync(); + + // AC7/AC8: a reference number and the deadline for further amendments. + await Page.GetByRole(AriaRole.Button, new() { Name = "Submit request" }).ClickAsync(); + await Page.WaitForURLAsync($"**/Journey/{WindowId}/confirmation"); + var panel = await Page.Locator(".govuk-panel").InnerTextAsync(); + Assert.Matches(@"CYPMD_16to19_[0-9A-F]{7}", panel); + await Expect(Page.GetByText("to request any further amendments")).ToBeVisibleAsync(); + } + + [RetryFact] + public async Task Code502Student_IsOffered_AndReachesTheEvidencePage() + { + await SeedHelpers.CleanupDevRequestsAsync(Fixture.SeedClient); + + await StartIncludeJourneyAsync(); + await SelectStudentAsync(Code502Student); + + // Not "This student is already included": the 502 student is includable by rule. + await Page.WaitForURLAsync($"**/Journey/{WindowId}/page/evidence"); + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })) + .ToContainTextAsync($"Provide evidence for the inclusion of {Code502Student}"); + } + + [RetryFact] + public async Task Code501Student_IsNotOffered_AndTypingTheName_WarnsAlreadyIncluded_WithoutAdd() + { + await StartIncludeJourneyAsync(); + + var input = Page.Locator("#pupil-search").First; + await input.FillAsync(Code501Student); + // AC9: a 501 student is not a suggestion. Every word of the query must match, so "Alice + // Smith" can match nobody else; wait for the component's empty-state row rather than + // counting options straight after typing (Ks4MergeJourneyTests.ExpectNoSuggestionsAsync). + await Expect(Page.Locator(".autocomplete__option--no-results")).ToBeVisibleAsync(new() { Timeout = 10_000 }); + await Expect(Page.Locator("li[role='option']")).ToHaveCountAsync(0); + await input.PressAsync("Escape"); + await ContinueAsync(); + + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })) + .ToContainTextAsync("This student is already included"); + Assert.Contains("/already-included", Page.Url); + await Expect(Page.GetByRole(AriaRole.Button, new() { Name = "Abort" })).ToBeVisibleAsync(); + // 16-19 has no Add journey, so no "Start adding" button. + Assert.Equal(0, await Page.GetByRole(AriaRole.Button, new() { NameRegex = new("^Start adding") }).CountAsync()); + } + + [RetryFact] + public async Task UnknownName_ShowsStudentNotFound_WithSearchAgainOnly() + { + await StartIncludeJourneyAsync(); + + var input = Page.Locator("#pupil-search").First; + await input.FillAsync(NoSuchName); + // Wait for the component's empty-state row, not a fixed time, before closing the list. + await Expect(Page.Locator(".autocomplete__option--no-results")).ToBeVisibleAsync(new() { Timeout = 10_000 }); + await input.PressAsync("Escape"); + await ContinueAsync(); + + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })).ToContainTextAsync("Student not found"); + Assert.Contains("/pupil-not-found", Page.Url); + await Expect(Page.GetByRole(AriaRole.Button, new() { Name = "Search again" })).ToBeVisibleAsync(); + Assert.Equal(0, await Page.GetByRole(AriaRole.Button, new() { NameRegex = new("^Start adding") }).CountAsync()); + + await Page.GetByRole(AriaRole.Button, new() { Name = "Search again" }).ClickAsync(); + await Page.WaitForURLAsync($"**/Journey/{WindowId}/pupil-search/select-pupil"); + } + + // ── helpers ────────────────────────────────────────────────────────────── + + private async Task StartIncludeJourneyAsync() + { + await Page.GotoAsync($"{Fixture.BaseUrl}/WhatToChange/{WindowId}"); + await Page.Locator("input[name='SelectedWhatToChange'][value='Include']").CheckAsync(); + await ContinueAsync(); + await Page.WaitForURLAsync($"**/Journey/{WindowId}/pupil-search/select-pupil"); + } + + // Types the name, picks the suggestion that carries it, and continues. The 16-19 suggestion + // reads "Firstname, Surname, (CYPMD ID:…, ULN:…, DOB:…, INCLUDED|NOT INCLUDED)" + // (PupilSuggestionFormat.Label); the rows are li[role='option'], as Ks4MergeJourneyTests picks them. + private async Task SelectStudentAsync(string name) + { + var input = Page.Locator("#pupil-search").First; + await Expect(input).ToBeVisibleAsync(); + await input.FillAsync(name); + var parts = name.Split(' '); + var option = Page.Locator("li[role='option']").GetByText($"{parts[0]}, {parts[1]},"); + await Expect(option.First).ToBeVisibleAsync(); + await option.First.ClickAsync(); + await ContinueAsync(); + } + + private async Task ContinueAsync() => + await Page.GetByRole(AriaRole.Button, new() { Name = "Continue", Exact = true }).ClickAsync(); +} diff --git a/tests/DfE.CheckPerformanceData.E2ETests/Journey/WhatToChangePost16OptionsTests.cs b/tests/DfE.CheckPerformanceData.E2ETests/Journey/WhatToChangePost16OptionsTests.cs index 66ae2e491..754b7fc06 100644 --- a/tests/DfE.CheckPerformanceData.E2ETests/Journey/WhatToChangePost16OptionsTests.cs +++ b/tests/DfE.CheckPerformanceData.E2ETests/Journey/WhatToChangePost16OptionsTests.cs @@ -4,35 +4,46 @@ namespace DfE.CheckPerformanceData.E2ETests.Journey; -// #439: the Include amendment option is not offered on a 16-19 (Post16) window and a crafted -// submission of it is refused server-side, while a KS4 window keeps the option exactly as before. +// What to change on a 16-19 (Post16) window offers exactly the changes with a flow file: Include +// (#711), Merge and Remove. Add has no 16-19 flow, so it is not offered and a crafted post of it is +// refused server-side (docs/ks2-journeys.md, decision 1). A KS4 June window keeps all four. // -// The seeded "16 to 19 Oct" window (DevDataSeeder.Post16OctoberCheckingWindowId) and the seeded KS4June window -// (DevDataSeeder.KeyStage4JuneCheckingWindowId) — the same ids AddPupilJourneyTests uses. +// The seeded "16 to 19 Oct" window (DevDataSeeder.Post16OctoberCheckingWindowId) and the seeded +// KS4June window (DevDataSeeder.KeyStage4JuneCheckingWindowId) — the same ids AddPupilJourneyTests uses. [Trait("Category", "FullRegression")] [Collection("E2E")] public sealed class WhatToChangePost16OptionsTests(PlaywrightFixture fixture) : SeedingPageTest(fixture) { - // The seeded "16 to 19 Oct" window — Include must not be offered (US1) and a crafted Include post - // must be refused (US3). private static readonly Guid Post16WindowId = Guid.Parse("EC6493B8-9B66-4090-8BE6-9DFC3805751F"); - - // The seeded KS4June window — Include must still be offered (US2). private static readonly Guid Ks4JuneWindowId = Guid.Parse("F34D285B-8660-4D12-9C30-787328DEAA0A"); [RetryFact] [Trait("Category", "Smoke")] - public async Task Post16Window_ShowsOnlyMergeAndRemove() + public async Task Post16Window_OffersIncludeMergeAndRemove_AndNoAdd() { await Page.GotoAsync($"{Fixture.BaseUrl}/WhatToChange/{Post16WindowId}"); - // Exactly two radios: Merge and Remove. + await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Include']")).ToHaveCountAsync(1); await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Merge']")).ToHaveCountAsync(1); await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Remove']")).ToHaveCountAsync(1); - - // No Include (this feature) and no Add (pre-existing AB#297310 gate) anywhere. - Assert.Equal(0, await Page.Locator("input[name='SelectedWhatToChange'][value='Include']").CountAsync()); Assert.Equal(0, await Page.Locator("input[name='SelectedWhatToChange'][value='Add']").CountAsync()); + + // The 16-19 noun. + await Expect(Page.GetByText("Include a student in data")).ToBeVisibleAsync(); + } + + [RetryFact] + public async Task Post16Window_SelectingInclude_OpensTheIncludeJourney() + { + await Page.GotoAsync($"{Fixture.BaseUrl}/WhatToChange/{Post16WindowId}"); + + await Page.Locator("input[name='SelectedWhatToChange'][value='Include']").CheckAsync(); + await Page.GetByRole(AriaRole.Button, new() { Name = "Continue", Exact = true }).ClickAsync(); + + await Page.WaitForURLAsync($"**/Journey/{Post16WindowId}/pupil-search/select-pupil"); + await Expect(Page.GetByRole(AriaRole.Heading, new() { Level = 1 })) + .ToContainTextAsync("What is the name of the student to be included?"); + await Expect(Page.Locator("#pupil-search").First).ToBeVisibleAsync(); } [RetryFact] @@ -40,13 +51,11 @@ public async Task Ks4JuneWindow_StillOffersInclude() { await Page.GotoAsync($"{Fixture.BaseUrl}/WhatToChange/{Ks4JuneWindowId}"); - // The option must be present and selectable next to Merge/Remove/Add exactly as before. await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Include']")).ToHaveCountAsync(1); await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Merge']")).ToHaveCountAsync(1); await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Remove']")).ToHaveCountAsync(1); await Expect(Page.Locator("input[name='SelectedWhatToChange'][value='Add']")).ToHaveCountAsync(1); - // Selecting Include and continuing still opens the Include journey. await Page.Locator("input[name='SelectedWhatToChange'][value='Include']").CheckAsync(); await Page.GetByRole(AriaRole.Button, new() { Name = "Continue", Exact = true }).ClickAsync(); await Page.WaitForURLAsync($"**/Journey/{Ks4JuneWindowId}/pupil-search/select-pupil"); @@ -54,24 +63,23 @@ public async Task Ks4JuneWindow_StillOffersInclude() } [RetryFact] - public async Task Post16Window_CraftedIncludeSubmission_IsRefused() + public async Task Post16Window_CraftedAddSubmission_IsRefused() { await Page.GotoAsync($"{Fixture.BaseUrl}/WhatToChange/{Post16WindowId}"); - // The radio is gone from the page, but the POST endpoint still exists. Re-inject the removed - // Include radio into the live form — the bookmark/replay case — select it, and submit. + // The radio is not on the page, but the POST endpoint still exists. Re-inject an Add radio + // into the live form — the bookmark/replay case — select it, and submit. await Page.EvaluateAsync(""" const input = document.createElement('input'); input.type = 'radio'; input.name = 'SelectedWhatToChange'; - input.value = 'Include'; + input.value = 'Add'; document.querySelector('input[name="SelectedWhatToChange"][value="Merge"]') .closest('form').appendChild(input); input.checked = true; """); - // Check it is selected with the form prepared. - var injected = Page.Locator("input[name='SelectedWhatToChange'][value='Include']"); + var injected = Page.Locator("input[name='SelectedWhatToChange'][value='Add']"); await Expect(injected).ToHaveCountAsync(1); await injected.CheckAsync(); @@ -81,4 +89,4 @@ await Page.EvaluateAsync(""" await Page.WaitForURLAsync($"**/CheckYourPupilData/{Post16WindowId}"); Assert.False(Page.Url.Contains("/Journey/", StringComparison.OrdinalIgnoreCase)); } -} \ No newline at end of file +} diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchExclusionTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchExclusionTests.cs index 49e233272..7b4ef9aff 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchExclusionTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchExclusionTests.cs @@ -59,6 +59,7 @@ private async Task ResetAsync() FirstLanguage = "English", Pincl = pincl, Included = pincl == IncludedPincl, + Includable = pincl != IncludedPincl, NewMobile = false, ActualYearGroup = "11", Ethnicity = "A1", @@ -289,7 +290,7 @@ public async Task FilterIncluded_ReturnsOnlyIncludedPupils() } [Fact] - public async Task FilterNonIncluded_ReturnsOnlyNonIncludedPupils() + public async Task FilterIncludable_ReturnsOnlyIncludablePupils() { await ResetAsync(); var windowId = Guid.NewGuid(); @@ -304,11 +305,33 @@ public async Task FilterNonIncluded_ReturnsOnlyNonIncludedPupils() ]); var repo = CreateRepo(blobClient); - var results = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Wilson", PupilFilter.NonIncluded); + var results = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Wilson", PupilFilter.Includable); Assert.Single(results); } + [Fact] + public async Task FilterNotIncludable_ReturnsOnlyPupilsWhoCannotBeIncluded() + { + await ResetAsync(); + var windowId = Guid.NewGuid(); + + await SeedWindowsAndRequestsAsync([NewWindow(windowId)], []); + + var blobClient = new FakePupilDataBlobClient(); + blobClient.SetPupils(windowId, TestLaestab, + [ + NewPupil(windowId, "Wilson", "A100000000011", pincl: 401), // included, so not includable + NewPupil(windowId, "Wilson", "A100000000012", pincl: 999), // not included, so includable + ]); + + var repo = CreateRepo(blobClient); + var results = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Wilson", PupilFilter.NotIncludable); + + var only = Assert.Single(results); + Assert.Contains("A100000000011", only.Label); + } + [Fact] public async Task ExcludeId_ExcludesSpecificPupilFromResults() { diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchIncludableFilterTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchIncludableFilterTests.cs new file mode 100644 index 000000000..b0af29bce --- /dev/null +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchIncludableFilterTests.cs @@ -0,0 +1,141 @@ +using DfE.CheckPerformanceData.Application.CheckYourPupilData; +using DfE.CheckPerformanceData.Application.Journey; +using DfE.CheckPerformanceData.Domain.Enums; +using DfE.CheckPerformanceData.IntegrationTests.Fixtures; +using DfE.CheckPerformanceData.Persistence.Entities; +using DfE.CheckPerformanceData.Persistence.Repositories; +using Microsoft.Extensions.Caching.Memory; +using Npgsql; + +namespace DfE.CheckPerformanceData.IntegrationTests.CheckYourPupilData; + +// #711: the Include journey on 16-19 may pick a student from the non-included file or a student +// the included file carries with P_INCL 502. Ingress stamps INCLUDABLE for both; the repository +// only reads the stamp. Included stays the file of origin, so the 502 student is still included +// for Remove and Merge. +[Collection(nameof(PostgresCollection))] +public sealed class PupilSearchIncludableFilterTests(PostgresFixture fixture) +{ + private const string TestUrn = "136309"; + private const string TestLaestab = "860/4070"; + + private async Task ResetAsync() + { + await using var conn = new NpgsqlConnection(fixture.ConnectionString); + await conn.OpenAsync(); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = @" + TRUNCATE ""ChangeRequests"" CASCADE; + TRUNCATE ""CheckingWindows"" RESTART IDENTITY CASCADE; + "; + await cmd.ExecuteNonQueryAsync(); + } + + private static DateTime Unspecified(DateTime dt) => DateTime.SpecifyKind(dt, DateTimeKind.Unspecified); + + private static CheckingWindow NewWindow(Guid id) => new() + { + Id = id, + Title = "16 to 19 test window", + StartDate = Unspecified(DateTime.UtcNow.AddDays(-7)), + EndDate = Unspecified(DateTime.UtcNow.AddDays(7)), + KeyStage = KeyStages.Post16, + CheckingWindowType = CheckingWindowType.Post16, + }; + + private static Post16PupilRecord NewStudent(string firstname, string surname, string cypmdId, + bool included, bool includable, int? pincl) => new() + { + Id = Guid.NewGuid(), + Included = included, + Includable = includable, + Cypmd_Id = cypmdId, + Surname = surname, + Firstname = firstname, + Sex = "F", + DateOfBirth = "01/09/2007", + Age = 18, + Pincl = pincl, + PinclAims = included ? 503 : null, + Laestab = "8604070", + Urn = TestUrn, + Ukprn = "10001234", + Uln = $"99{cypmdId.PadLeft(8, '0')}", + }; + + private async Task<(Guid WindowId, CheckYourPupilDataRepository Repo)> SeededAsync(params Post16PupilRecord[] students) + { + await ResetAsync(); + var windowId = Guid.NewGuid(); + + await using (var ctx = fixture.CreateContext()) + { + ctx.CheckingWindows.Add(NewWindow(windowId)); + await ctx.SaveChangesAsync(); + } + + var blobClient = new FakePupilDataBlobClient(); + blobClient.SetPupils(windowId, TestLaestab, students); + + return (windowId, new CheckYourPupilDataRepository( + fixture.CreateContext(), blobClient, new MemoryCache(new MemoryCacheOptions()))); + } + + private static (Post16PupilRecord Alice501, Post16PupilRecord Bob502, Post16PupilRecord AliceNonIncluded) Students() => + ( + NewStudent("Alice", "Smith", "500001", included: true, includable: false, pincl: 501), + NewStudent("Bob", "Smith", "500002", included: true, includable: true, pincl: 502), + NewStudent("Alice", "Johnson", "500201", included: false, includable: true, pincl: null) + ); + + [Fact] + public async Task Includable_returns_the_502_student_and_the_non_included_student() + { + var (alice501, bob502, aliceNonIncluded) = Students(); + var (windowId, repo) = await SeededAsync(alice501, bob502, aliceNonIncluded); + + var smiths = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Smith", PupilFilter.Includable); + var johnsons = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Johnson", PupilFilter.Includable); + var results = smiths.Concat(johnsons).ToList(); + + Assert.Equal(2, results.Count); + Assert.Contains(results, r => r.Id == bob502.Id); + Assert.Contains(results, r => r.Id == aliceNonIncluded.Id); + } + + [Fact] + public async Task NotIncludable_returns_only_the_student_who_is_included_for_good() + { + var (alice501, bob502, aliceNonIncluded) = Students(); + var (windowId, repo) = await SeededAsync(alice501, bob502, aliceNonIncluded); + + var results = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Alice", PupilFilter.NotIncludable); + + var only = Assert.Single(results); + Assert.Equal(alice501.Id, only.Id); + } + + [Fact] + public async Task Included_still_counts_the_502_student_as_included() + { + var (alice501, bob502, aliceNonIncluded) = Students(); + var (windowId, repo) = await SeededAsync(alice501, bob502, aliceNonIncluded); + + var results = await repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Smith", PupilFilter.Included); + + Assert.Equal(2, results.Count); + Assert.DoesNotContain(results, r => r.Id == aliceNonIncluded.Id); + } + + // A filter the repository has no rule for fails loudly, so a new PupilFilter value cannot + // quietly return the wrong pupils. + [Fact] + public async Task An_unknown_filter_throws() + { + var (alice501, bob502, aliceNonIncluded) = Students(); + var (windowId, repo) = await SeededAsync(alice501, bob502, aliceNonIncluded); + + await Assert.ThrowsAsync(() => + repo.SearchPupilsAsync(windowId, TestLaestab, TestUrn, "Smith", (PupilFilter)99)); + } +} diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchTwoPartNameTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchTwoPartNameTests.cs index 0bd6f3d54..eb05f8483 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchTwoPartNameTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/CheckYourPupilData/PupilSearchTwoPartNameTests.cs @@ -11,7 +11,7 @@ namespace DfE.CheckPerformanceData.IntegrationTests.CheckYourPupilData; // US3: The Include journey's pupil-search step inherits two-part name matching via // PupilSuggestionFormat.Matches. These integration tests verify SearchPupilsAsync returns -// correct results for two-part queries with both Included and NonIncluded filters. +// correct results for two-part queries with both Included and Includable filters. [Collection(nameof(PostgresCollection))] public sealed class PupilSearchTwoPartNameTests(PostgresFixture fixture) { @@ -56,6 +56,7 @@ private static PupilRecord NewPupil( FirstLanguage = "English", Pincl = included ? 401 : 402, Included = included, + Includable = !included, NewMobile = false, ActualYearGroup = "11", Ethnicity = "A1", @@ -123,7 +124,7 @@ public async Task Two_part_query_matches_both_parts_in_non_included_pupils() NewPupil("Jane", "Smith", "500002", included: false)); var results = await repo.SearchPupilsAsync( - windowId, TestLaestab, TestUrn, "John Smith", PupilFilter.NonIncluded); + windowId, TestLaestab, TestUrn, "John Smith", PupilFilter.Includable); Assert.Single(results); Assert.Contains("John", results[0].Label); diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CheckingExerciseReleaseTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CheckingExerciseReleaseTests.cs index f7b25c4e6..c7ee7e986 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CheckingExerciseReleaseTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CheckingExerciseReleaseTests.cs @@ -51,7 +51,8 @@ public sealed class CheckingExerciseReleaseTests(AzuriteFixture azurite) : IAsyn "SESSION": { "type": ["string", "null"] }, "GRADE": { "type": ["string", "null"] }, "SOURCE": { "type": "string" }, - "INCLUDED": { "type": "boolean" } + "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" } } } """; diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CsvSchemaFileProcessorTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CsvSchemaFileProcessorTests.cs index 3c52e4d3a..fe5f201bd 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CsvSchemaFileProcessorTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/CsvSchemaFileProcessorTests.cs @@ -24,6 +24,7 @@ public sealed class CsvSchemaFileProcessorTests(AzuriteFixture fixture) "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "CYPMD_ID": { "type": ["string", "null"] }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES":{ "type": ["string", "null"] }, @@ -40,6 +41,7 @@ public sealed class CsvSchemaFileProcessorTests(AzuriteFixture fixture) "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "CYPMD_ID": { "type": ["string", "null"] }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES":{ "type": ["string", "null"] }, @@ -172,6 +174,7 @@ public async Task An_error_in_either_dataset_writes_nothing() "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES":{ "type": ["string", "null"] }, "CYPMD_ID": { "type": ["string", "null"] }, @@ -259,6 +262,7 @@ public async Task A_single_ks4_dataset_run_is_unchanged_and_stamps_nothing() }, "properties": { "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "SURNAME": { "type": ["string", "null"] }, "LAESTAB": { "type": ["string", "null"] }, "P_INCL": { "type": ["string", "null"] }, @@ -367,6 +371,147 @@ public async Task A_dataset_with_no_marker_does_not_claim_inclusion_when_the_sch Assert.Null(await ReadPupilsAsync(container, "8604070")); } + // #711: who may be asked to be included. A schema names the codes in x-ingress.includable + // (16-19 P_INCL 502, a student in the included file who is not in the measures); a record that + // is not included is includable without a rule. Readers read INCLUDABLE, never a code. + private const string SchemaWithIncludableRule = """ + { + "type": "object", + "additionalProperties": false, + "x-ingress": { + "includable": { "field": "P_INCL", "codes": ["502"] } + }, + "properties": { + "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, + "SURNAME": { "type": ["string", "null"] }, + "LAESTAB": { "type": ["string", "null"] }, + "P_INCL": { "type": ["string", "null"] } + } + } + """; + + [Theory] + [InlineData("SURNAME,LAESTAB,P_INCL\nSmith,8604070,502\n", true, true)] + [InlineData("SURNAME,LAESTAB,P_INCL\nSmith,8604070, 502 \n", true, true)] + [InlineData("SURNAME,LAESTAB,P_INCL\nSmith,8604070,501\n", true, false)] + [InlineData("SURNAME,LAESTAB,P_INCL\nSmith,8604070,\n", true, false)] + [InlineData("SURNAME,LAESTAB,P_INCL\nSmith,8604070,501\n", false, true)] + public async Task The_schemas_includable_rule_stamps_INCLUDABLE_from_the_code_or_from_not_included( + string csv, bool slotIncluded, bool includable) + { + var (last, pupil) = await RunInclusionRuleAsync(csv, SchemaWithIncludableRule, slotIncluded); + + Assert.False(last.IsError, last.Message); + Assert.Equal(slotIncluded, pupil!["INCLUDED"]!.Value()); + Assert.Equal(includable, pupil["INCLUDABLE"]!.Value()); + } + + [Fact] + public async Task A_schema_with_no_includable_rule_stamps_INCLUDABLE_as_not_included() + { + var (included, includedPupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,401\n", SchemaWithInclusionRule); + var (notIncluded, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,402\n", SchemaWithInclusionRule); + + Assert.False(included.IsError, included.Message); + Assert.False(notIncluded.IsError, notIncluded.Message); + Assert.False(includedPupil!["INCLUDABLE"]!.Value()); + Assert.True(pupil!["INCLUDABLE"]!.Value()); + } + + // Without INCLUDABLE a pupil file would offer nobody for inclusion, and nothing would say so. + // An uploaded schema can do this as easily as a shipped one, so the run fails by name. + [Theory] + [InlineData(null)] + [InlineData(true)] + public async Task A_schema_declaring_INCLUDED_but_not_INCLUDABLE_fails_the_run_by_name(bool? slotIncluded) + { + var schema = SchemaWithInclusionRule.Replace("\"INCLUDABLE\": { \"type\": \"boolean\" },", string.Empty); + + var (last, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,402\n", schema, slotIncluded); + + Assert.True(last.IsError); + Assert.Contains("declares INCLUDED but not INCLUDABLE", last.Message); + Assert.Null(pupil); + } + + [Fact] + public async Task An_includable_rule_on_a_schema_that_does_not_declare_INCLUDABLE_fails_the_run_by_name() + { + var schema = SchemaWithIncludableRule.Replace("\"INCLUDABLE\": { \"type\": \"boolean\" },", string.Empty); + + var (last, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,502\n", schema, slotIncluded: true); + + Assert.True(last.IsError); + Assert.Contains("INCLUDABLE", last.Message); + Assert.Null(pupil); + } + + [Fact] + public async Task An_includable_rule_that_reads_an_undeclared_field_fails_the_run_by_name() + { + var schema = SchemaWithIncludableRule.Replace("\"field\": \"P_INCL\"", "\"field\": \"P_NOPE\""); + + var (last, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,502\n", schema, slotIncluded: true); + + Assert.True(last.IsError); + Assert.Contains("P_NOPE", last.Message); + Assert.Null(pupil); + } + + // INCLUDABLE is derived from INCLUDED. A slot with no file of origin and a schema with no + // inclusion rule stamp neither, so both are left null and the rows fail validation: the run + // fails rather than write a file that offers nobody for inclusion. + [Fact] + public async Task A_schema_declaring_INCLUDABLE_with_nothing_to_stamp_INCLUDED_fails_the_run() + { + var (last, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,502\n", SchemaWithIncludableRule); + + Assert.True(last.IsError); + Assert.Contains("Validation failed", last.Message); + Assert.Null(pupil); + } + + // A rule that cannot be honoured fails the run before any row is read, like the inclusion rule. + [Theory] + [InlineData("{ \"codes\": [\"502\"] }", "names no field")] + [InlineData("{ \"field\": \"P_INCL\" }", "names no codes")] + [InlineData("{ \"field\": \"P_INCL\", \"codes\": [] }", "names no codes")] + [InlineData("{ \"field\": \"P_INCL\", \"codes\": [\"502\", \" \"] }", "names a blank code")] + public async Task An_includable_rule_that_cannot_be_honoured_fails_the_run_by_name(string rule, string error) + { + var schema = SchemaWithIncludableRule.Replace( + "\"includable\": { \"field\": \"P_INCL\", \"codes\": [\"502\"] }", $"\"includable\": {rule}"); + + var (last, pupil) = await RunInclusionRuleAsync("SURNAME,LAESTAB,P_INCL\nSmith,8604070,502\n", schema, slotIncluded: true); + + Assert.True(last.IsError); + Assert.Contains(error, last.Message); + Assert.Null(pupil); + } + + // The shipped schemas: KS4 and KS2 stamp INCLUDABLE as "not included"; the 16-19 included file + // names 502; the 16-19 non-included file has no rule and every row is includable. + [Theory] + [InlineData("ks4june", "pupils_schema.json", "UPN,CYPMD_ID,SURNAME,FORENAME,P_INCL,LAESTAB\nA1,1,Smith,Al,401,8604070\n", null, false)] + [InlineData("ks4june", "pupils_schema.json", "UPN,CYPMD_ID,SURNAME,FORENAME,P_INCL,LAESTAB\nA1,1,Smith,Al,402,8604070\n", null, true)] + [InlineData("ks2", "pupils_schema.json", "UPN,CYPMD_ID,MATCHREF,SURNAME,FORENAME,PINCL_Flag,LAESTAB\nA1,1,7,Smith,Al,201,8604070\n", null, false)] + [InlineData("ks2", "pupils-december_schema.json", "UPN,CYPMD_ID,MATCHREF,SURNAME,FORENAME,PINCL_Flag,LAESTAB\nA1,1,7,Smith,Al,202,8604070\n", null, true)] + [InlineData("post16", "students-included_schema.json", "CYPMD_ID,SURNAME,FORENAMES,P_INCL,LAESTAB\n1,Smith,Al,501,8604070\n", true, false)] + [InlineData("post16", "students-included_schema.json", "CYPMD_ID,SURNAME,FORENAMES,P_INCL,LAESTAB\n1,Smith,Al,502,8604070\n", true, true)] + [InlineData("post16", "students-included_schema.json", "CYPMD_ID,SURNAME,FORENAMES,P_INCL,LAESTAB\n1,Smith,Al,505,8604070\n", true, false)] + [InlineData("post16", "students-non-included_schema.json", "CYPMD_ID,SURNAME,FORENAMES,LAESTAB\n1,Smith,Al,8604070\n", false, true)] + public async Task A_shipped_pupil_schema_stamps_INCLUDABLE(string folder, string file, string csv, bool? slotIncluded, bool includable) + { + var schema = await File.ReadAllTextAsync( + Path.Combine(AppContext.BaseDirectory, "Data", "Ingress", folder, file)); + + var (last, pupil) = await RunInclusionRuleAsync(csv, schema, slotIncluded); + + Assert.False(last.IsError, last.Message); + Assert.Equal(includable, pupil!["INCLUDABLE"]!.Value()); + } + [Fact] public async Task A_schema_with_an_institution_key_splits_the_file_by_that_column() { @@ -470,6 +615,7 @@ public async Task AFailedRun_LeavesThePreviousOutputInPlace() "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES":{ "type": ["string", "null"] }, "CYPMD_ID": { "type": ["string", "null"] }, @@ -553,8 +699,9 @@ public async Task Pupil_outputs_keep_their_complete_contract_and_existing_csv_su if (post16) { ((JObject)expected[0])["INCLUDED"] = true; + ((JObject)expected[0])["INCLUDABLE"] = false; if (school == "8604070") - expected.Add(JObject.Parse("""{"CYPMD_ID":"500002","SURNAME":"Jones","FORENAMES":"Bob","LAESTAB":"8604070","ULN":"9900000002","CampID_0":"C0","INCLUDED":false}""")); + expected.Add(JObject.Parse("""{"CYPMD_ID":"500002","SURNAME":"Jones","FORENAMES":"Bob","LAESTAB":"8604070","ULN":"9900000002","CampID_0":"C0","INCLUDED":false,"INCLUDABLE":true}""")); } Assert.True(JToken.DeepEquals(expected, actual), $"Unexpected pupil output for {school}: {actual}"); } diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/ExerciseScopedIngressTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/ExerciseScopedIngressTests.cs index a371ff824..74bd5825b 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/ExerciseScopedIngressTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/ExerciseScopedIngressTests.cs @@ -24,6 +24,7 @@ public sealed class ExerciseScopedIngressTests(AzuriteFixture fixture) "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "CYPMD_ID": { "type": ["string", "null"] }, "SURNAME": { "type": ["string", "null"] }, "LAESTAB": { "type": ["string", "null"] } diff --git a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/Post16EndToEndTests.cs b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/Post16EndToEndTests.cs index 22aa5bad2..319cb6788 100644 --- a/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/Post16EndToEndTests.cs +++ b/tests/DfE.CheckPerformanceData.IntegrationTests/Ingress/Post16EndToEndTests.cs @@ -38,6 +38,7 @@ public sealed class Post16EndToEndTests(AzuriteFixture azurite, PostgresFixture "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "CYPMD_ID": { "type": ["string", "null"] }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES": { "type": ["string", "null"] }, @@ -59,6 +60,7 @@ public sealed class Post16EndToEndTests(AzuriteFixture azurite, PostgresFixture "properties": { "Id": { "type": ["string", "null"] }, "INCLUDED": { "type": "boolean" }, + "INCLUDABLE": { "type": "boolean" }, "CYPMD_ID": { "type": ["string", "null"] }, "SURNAME": { "type": ["string", "null"] }, "FORENAMES": { "type": ["string", "null"] }, diff --git a/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/Post16PupilRecordDeserializationTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/Post16PupilRecordDeserializationTests.cs index fa9e430ea..0ac1a1954 100644 --- a/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/Post16PupilRecordDeserializationTests.cs +++ b/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/Post16PupilRecordDeserializationTests.cs @@ -121,4 +121,51 @@ public void Binds_numeric_fields_sent_as_quoted_strings() Assert.Equal(18, pupil.Age); Assert.Equal(502, pupil.Pincl); } + + [Fact] + public void Binds_INCLUDABLE_from_the_stamp() + { + const string json = """ + [ + { + "Id": "11111111-1111-1111-1111-111111111111", + "INCLUDED": true, + "INCLUDABLE": true, + "CYPMD_ID": "500124", + "SURNAME": "Smith", + "FORENAMES": "Bob", + "P_INCL": 502, + "LAESTAB": "860/4070" + } + ] + """; + + var pupil = DeserializeSingle(json); + + Assert.True(pupil.IsIncluded); + Assert.True(pupil.IsIncludable); + } + + [Fact] + public void A_record_with_no_INCLUDABLE_stamp_is_not_includable() + { + const string json = """ + [ + { + "Id": "11111111-1111-1111-1111-111111111111", + "INCLUDED": false, + "CYPMD_ID": "500201", + "SURNAME": "Johnson", + "FORENAMES": "Alice", + "LAESTAB": "860/4070" + } + ] + """; + + var pupil = DeserializeSingle(json); + + // Fail closed: a file ingested before the stamp existed offers nobody for inclusion until + // the exercise is validated again. The service is not live, so there is no fallback. + Assert.False(pupil.IsIncludable); + } } diff --git a/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/PupilRecordDeserializationTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/PupilRecordDeserializationTests.cs index 0e9c9100a..9c56660a0 100644 --- a/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/PupilRecordDeserializationTests.cs +++ b/tests/DfE.CheckPerformanceData.UnitTests/CheckYourPupilData/PupilRecordDeserializationTests.cs @@ -221,6 +221,27 @@ public void IsIncluded_is_false_when_the_stamp_is_absent() Assert.False(pupil.IsIncluded); } + // Ingress stamps INCLUDABLE (#711); on KS4 and KS2 it is "not included". The record reads only the stamp. + [Theory] + [InlineData("true", true, true)] + [InlineData("false", false, false)] + public void IsIncludable_follows_the_stamp_not_INCLUDED(string stamp, bool included, bool expected) + { + var pupil = DeserializeSingle($$"""[ { "INCLUDED": {{(included ? "true" : "false")}}, "INCLUDABLE": {{stamp}} } ]"""); + + Assert.Equal(expected, pupil.IsIncludable); + } + + [Fact] + public void IsIncludable_is_false_when_the_stamp_is_absent() + { + // A blob written before the stamp existed offers nobody for inclusion until the exercise is + // validated again (docs/blob-storage.md). + var pupil = DeserializeSingle("""[ { "INCLUDED": false, "SURNAME": "Smith" } ]"""); + + Assert.False(pupil.IsIncludable); + } + [Fact] public void The_inclusion_description_is_read_from_P_INCL_DESC() { diff --git a/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeHandoffConflictTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeHandoffConflictTests.cs index 2e17f5fcc..e5afb9529 100644 --- a/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeHandoffConflictTests.cs +++ b/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeHandoffConflictTests.cs @@ -58,7 +58,7 @@ public class IncludeHandoffConflictTests Id = "select-pupil", Type = PageType.PupilSearch, Title = "Which pupil do you want to include?", - PupilFilter = PupilFilter.Included, + PupilFilter = PupilFilter.Includable, PupilKey = JourneyPage.PrimaryKey, NextPageId = "evidence" }; @@ -327,7 +327,7 @@ public async Task IncludeThisPupil_WhenSameNameAlreadyIncluded_RedirectsToAlread SetupSession(AddSessionAwaitingInclude(SingleNonIncludedCheck)); _flowService.GetConfigAsync(WhatToChange.Include, CheckingWindowType.KS4June).Returns(IncludeConfig); _pupilDataService.GetPupilAsync(WindowId, MatchPupilId).Returns(MatchPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.NotIncludable) .Returns([new PupilSuggestionDto(IncludedPupilId, "Doe, John", "John", "Doe", "02/02/2010")]); var result = await _sut.IncludeThisPupil(WindowId); @@ -357,7 +357,7 @@ public async Task SwitchToInclude_WhenSameNameAlreadyIncluded_RedirectsToAlready SetupSession(AddSessionAwaitingInclude(MultipleCheck)); _flowService.GetConfigAsync(WhatToChange.Include, CheckingWindowType.KS4June).Returns(IncludeConfig); _pupilDataService.GetPupilAsync(WindowId, MatchPupilId).Returns(MatchPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.NotIncludable) .Returns([new PupilSuggestionDto(IncludedPupilId, "Doe, John", "John", "Doe", "02/02/2010")]); var result = await _sut.SwitchToInclude(WindowId, MatchPupilId); @@ -385,7 +385,7 @@ public async Task SwitchToInclude_WhenIncludedNamesakeAlreadyOnScreen_ProceedsTo SetupSession(AddSessionAwaitingInclude(MultipleSameNameCheck)); _flowService.GetConfigAsync(WhatToChange.Include, CheckingWindowType.KS4June).Returns(IncludeConfig); _pupilDataService.GetPupilAsync(WindowId, MatchPupilId).Returns(MatchPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.NotIncludable) .Returns([new PupilSuggestionDto(IncludedNamesakeId, "Doe, John", "John", "Doe", "02/02/2010")]); var result = await _sut.SwitchToInclude(WindowId, MatchPupilId); @@ -412,7 +412,7 @@ public async Task SwitchToInclude_WhenNoAlreadyIncludedNameMatch_ProceedsToSeed( SetupSession(AddSessionAwaitingInclude(MultipleCheck)); _flowService.GetConfigAsync(WhatToChange.Include, CheckingWindowType.KS4June).Returns(IncludeConfig); _pupilDataService.GetPupilAsync(WindowId, MatchPupilId).Returns(MatchPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "John Doe", PupilFilter.NotIncludable) .Returns([]); var result = await _sut.SwitchToInclude(WindowId, MatchPupilId); diff --git a/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeNoResultsDecisionTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeNoResultsDecisionTests.cs index e6277a36e..a68f65eeb 100644 --- a/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeNoResultsDecisionTests.cs +++ b/tests/DfE.CheckPerformanceData.UnitTests/Journey/IncludeNoResultsDecisionTests.cs @@ -22,8 +22,8 @@ namespace DfE.CheckPerformanceData.Application.UnitTests.Journey; /// /// AB#027: Include journey no-results decision flow. When a user on the Include journey's /// select-pupil step types a name, gets no autocomplete result, and clicks Continue without -/// selecting a pupil, the system looks the typed entry up against BOTH the included and -/// non-included populations: +/// selecting a pupil, the system looks the typed entry up among the pupils who cannot be included +/// (`NotIncludable`) and those who can (`Includable`): /// - included match warns "already included" (abort); /// - non-included-only match is a valid include candidate — the journey proceeds as normal /// (re-renders the search page with the candidate visible, no warning, no decision page); @@ -54,7 +54,7 @@ public class IncludeNoResultsDecisionTests Id = "select-pupil", Type = PageType.PupilSearch, Title = "What is the name of the pupil to be included?", - PupilFilter = PupilFilter.NonIncluded, + PupilFilter = PupilFilter.Includable, PupilKey = JourneyPage.PrimaryKey, ValidationFailure = "Enter the name of the pupil to be included." }; @@ -132,9 +132,9 @@ public IncludeNoResultsDecisionTests() public async Task PupilSearchPost_NeitherListMatch_ReturnsPupilNotFoundView() { SetupSession(SessionForInclude()); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "No Such", PupilFilter.Included, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "No Such", PupilFilter.NotIncludable, Arg.Any(), Arg.Any()) .Returns(new List()); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "No Such", PupilFilter.NonIncluded, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "No Such", PupilFilter.Includable, Arg.Any(), Arg.Any()) .Returns(new List()); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", null, "No Such"); @@ -152,14 +152,14 @@ public async Task PupilSearchPost_NeitherListMatch_ReturnsPupilNotFoundView() // → a valid include candidate: proceed as normal (no decision page, no "not found" warning) -- [Fact] - public async Task PupilSearchPost_NonIncludedOnlyMatch_ProceedsAsNormal_NoDecisionRedirect() + public async Task PupilSearchPost_IncludableOnlyMatch_ProceedsAsNormal_NoDecisionRedirect() { SetupSession(SessionForInclude()); // "Johnson" is a non-included surname bucket — it matches the non-included pupil but no // included one (included surnames are Smith..Davies). - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.Included, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.NotIncludable, Arg.Any(), Arg.Any()) .Returns(new List()); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.NonIncluded, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.Includable, Arg.Any(), Arg.Any()) .Returns(new List { NonIncludedSuggestion }); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", null, "Johnson"); @@ -172,7 +172,29 @@ public async Task PupilSearchPost_NonIncludedOnlyMatch_ProceedsAsNormal_NoDecisi Assert.Equal("Johnson", _session.GetRequestState(WindowId).IncludeSearchLabel); // The three-way decision consults the non-included list to discover the valid candidate. await _pupilDataService.Received() - .GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.NonIncluded, Arg.Any(), Arg.Any()); + .GetPupilSuggestionsAsync(WindowId, "Johnson", PupilFilter.Includable, Arg.Any(), Arg.Any()); + } + + // #711: on 16-19 a P_INCL 502 student is INCLUDED by file but includable by rule. The guards + // ask NotIncludable, so a typed name that matches only includable students is a candidate. + [Fact] + public async Task PupilSearchPost_IncludableMatchOnly_ProceedsAsNormal_EvenWhenThatPupilIsIncluded() + { + SetupSession(SessionForInclude()); + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Bob Smith", PupilFilter.NotIncludable, Arg.Any(), Arg.Any()) + .Returns(new List()); + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Bob Smith", PupilFilter.Includable, Arg.Any(), Arg.Any()) + .Returns(new List + { + new(Guid.NewGuid(), "Bob, Smith, (CYPMD ID:500002, ULN:9900000002, DOB:01/09/2007, INCLUDED)", "Bob", "Smith", "01/09/2007") + }); + + var result = await _sut.PupilSearchPost(WindowId, "select-pupil", null, "Bob Smith"); + + var view = Assert.IsType(result); + Assert.Equal("PupilSearch", view.ViewName); + await _pupilDataService.DidNotReceive() + .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.Included, Arg.Any(), Arg.Any()); } // -- T005: non-blank typed text + no selection + included match → "Already included" -- @@ -182,7 +204,7 @@ public async Task PupilSearchPost_IncludedMatch_RedirectsToAlreadyIncluded() { SetupSession(SessionForInclude()); // "Smith" is an included surname bucket — it matches the included pupil Alice Smith. - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Smith", PupilFilter.Included, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Smith", PupilFilter.NotIncludable, Arg.Any(), Arg.Any()) .Returns(new List { IncludedSuggestion }); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", null, "Smith"); @@ -200,7 +222,7 @@ public async Task PupilSearchPost_IncludedMatch_RedirectsToAlreadyIncluded() Assert.True(_sut.ModelState.IsValid); // An included match is conclusive — there is no need to consult the non-included list. await _pupilDataService.DidNotReceive() - .GetPupilSuggestionsAsync(WindowId, "Smith", PupilFilter.NonIncluded, Arg.Any(), Arg.Any()); + .GetPupilSuggestionsAsync(WindowId, "Smith", PupilFilter.Includable, Arg.Any(), Arg.Any()); } // -- The "Already included" GET surfaces the persisted matches and consumes them on arrival -- @@ -228,6 +250,36 @@ public async Task AlreadyIncluded_Get_ShowsTypedLabelAndMatches_ThenClearsSessio Assert.Null(_session.GetRequestState(WindowId).IncludeMatchedPupils); } + // #711: 16-19 has no Add flow, so the decision pages must not offer "Start adding". CanAdd + // comes from the same list What to change renders. + [Fact] + public async Task AlreadyIncluded_OnAWindowTypeWithNoAddFlow_DoesNotOfferAdd() + { + SetupSession(SessionForIncludeWith(label: "Bob Smith")); + _flowService.GetPupilDataChangesAsync(Arg.Any()) + .Returns(Task.FromResult>([WhatToChange.Merge, WhatToChange.Remove])); + + var result = await _sut.AlreadyIncluded(WindowId); + + var model = Assert.IsType(Assert.IsType(result).Model); + Assert.False(model.CanAdd); + Assert.Equal("pupil", model.LearnerNoun.Singular); // the session's window is KS4 June + } + + [Fact] + public async Task PupilNotFound_OnAWindowTypeWithAnAddFlow_OffersAdd() + { + SetupSession(SessionForIncludeWith(label: "No Such")); + _flowService.GetPupilDataChangesAsync(Arg.Any()) + .Returns(Task.FromResult>([WhatToChange.Merge, WhatToChange.Include, WhatToChange.Remove, WhatToChange.Add])); + + var result = await _sut.PupilNotFound(WindowId, "select-pupil"); + + var model = Assert.IsType(Assert.IsType(result).Model); + Assert.True(model.CanAdd); + Assert.Equal("pupil", model.LearnerNoun.Singular); + } + // -- AB#297780: when reached from the duplicate-check hand-off, the back link returns to // the match list rather than the Include search page -- @@ -261,7 +313,7 @@ public async Task PupilSearchPost_BlankLabel_ReturnsExistingValidationError() var error = _sut.ModelState["selectedPupilId"]?.Errors.FirstOrDefault()?.ErrorMessage; Assert.Equal("Enter the name of the pupil to be included.", error); await _pupilDataService.DidNotReceive() - .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.Included, Arg.Any(), Arg.Any()); + .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.NotIncludable, Arg.Any(), Arg.Any()); } // -- T007: non-blank typed text + no selection on non-Include journey → unchanged -- @@ -278,7 +330,7 @@ public async Task PupilSearchPost_NonIncludeJourney_ReturnsExistingValidationErr Assert.Equal("PupilSearch", view.ViewName); Assert.True(_sut.ModelState.ContainsKey("selectedPupilId")); await _pupilDataService.DidNotReceive() - .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.Included, Arg.Any(), Arg.Any()); + .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.NotIncludable, Arg.Any(), Arg.Any()); } // -- T008: lookup failure → falls back to existing validation (fail safe) -- @@ -287,7 +339,7 @@ await _pupilDataService.DidNotReceive() public async Task PupilSearchPost_LookupFailure_FallsBackToExistingValidation() { SetupSession(SessionForInclude()); - _pupilDataService.GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.Included, Arg.Any(), Arg.Any()) + _pupilDataService.GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.NotIncludable, Arg.Any(), Arg.Any()) .Returns(Task.FromException>(new InvalidOperationException("blob unavailable"))); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", null, "No Such"); @@ -329,6 +381,22 @@ public async Task StartAddingPupil_ResetsToAddJourney_RedirectsToLearnerDetails( Assert.Equal("learner-details", saved.QuestionHistory.Single()); } + // #711: the decision pages hide "Start adding" where there is no Add flow (16-19), and a forged + // POST is refused by the action itself, leaving the Include attempt untouched. + [Fact] + public async Task StartAddingPupil_OnAWindowTypeWithNoAddFlow_RedirectsToCheckYourData() + { + SetupSession(SessionForIncludeWith(label: "No Such")); + _flowService.GetConfigAsync(WhatToChange.Add, CheckingWindowType.KS4June).Returns((QuestionFlowConfig?)null); + + var result = await _sut.StartAddingPupil(WindowId); + + AssertRedirectToCheckYourData(result); + var saved = _session.GetRequestState(WindowId); + Assert.Equal(WhatToChange.Include, saved.SelectedWhatToChange); + Assert.Equal("No Such", saved.IncludeSearchLabel); + } + // -- T019: PupilSearchAgain returns to the Include select-pupil search page -- [Fact] diff --git a/tests/DfE.CheckPerformanceData.UnitTests/Journey/PupilSearchJourneyTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/Journey/PupilSearchJourneyTests.cs index 448cacd94..b1108cc95 100644 --- a/tests/DfE.CheckPerformanceData.UnitTests/Journey/PupilSearchJourneyTests.cs +++ b/tests/DfE.CheckPerformanceData.UnitTests/Journey/PupilSearchJourneyTests.cs @@ -607,7 +607,7 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndSelectedNameMatchesInclud }; SetupIncludeJourney(); _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(nonIncludedPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.NotIncludable) .Returns([new PupilSuggestionDto(Guid.NewGuid(), "Carter, Casey, 01/01/2010", "Casey", "Carter", "01/01/2010")]); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Carter, Casey, 01/01/2010"); @@ -627,7 +627,7 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndSelectedNameMatchesInclud }; SetupIncludeJourney(); _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(nonIncludedPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.NotIncludable) .Returns([new PupilSuggestionDto(Guid.NewGuid(), "Carter, Casey, 01/01/2010", "Casey", "Carter", "01/01/2010")]); await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Carter, Casey, 01/01/2010"); @@ -647,7 +647,7 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndSelectedNameMatchesInclud var includedSuggestion = new PupilSuggestionDto(Guid.NewGuid(), "Carter, Casey, 01/01/2010", "Casey", "Carter", "01/01/2010"); SetupIncludeJourney(); _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(nonIncludedPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.NotIncludable) .Returns([includedSuggestion]); await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Carter, Casey, 01/01/2010"); @@ -688,7 +688,7 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndNoIncludedNameMatch_Proce }; SetupIncludeJourney(); _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(nonIncludedPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Jane Smith", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Jane Smith", PupilFilter.NotIncludable) .Returns([]); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Smith, Jane, 01/01/2010"); @@ -697,6 +697,35 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndNoIncludedNameMatch_Proce Assert.Equal("evidence", redirect.RouteValues!["pageId"]); } + // #711: on 16-19 a student can be included by file and still be includable by the schema's + // rule. An Included lookup would find the selected student and block their own request, so the + // autocomplete guard must ask NotIncludable and never Included. + [Fact] + public async Task PupilSearchPost_WhenIncludeJourneyAndSelectedPupilIsIncludedButIncludable_NeverAsksIncluded() + { + var includableStudent = new PupilDto + { + Id = PrimaryPupilId, Firstname = "Bob", Surname = "Smith", + Sex = "M", DateOfBirth = "01/09/2007", Age = 18, + Cypmd_Id = "500002", Identifier = "9900500002" + }; + SetupIncludeJourney(); + _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(includableStudent); + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Bob Smith", PupilFilter.Included) + .Returns([new PupilSuggestionDto(PrimaryPupilId, "Bob, Smith, (INCLUDED)", "Bob", "Smith", "01/09/2007")]); + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Bob Smith", PupilFilter.NotIncludable) + .Returns([]); + + var result = await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Bob, Smith, (INCLUDED)"); + + var redirect = Assert.IsType(result); + Assert.Equal("evidence", redirect.RouteValues!["pageId"]); + await _pupilDataService.Received() + .GetPupilSuggestionsAsync(WindowId, "Bob Smith", PupilFilter.NotIncludable, Arg.Any(), Arg.Any(), Arg.Any()); + await _pupilDataService.DidNotReceive() + .GetPupilSuggestionsAsync(Arg.Any(), Arg.Any(), PupilFilter.Included, Arg.Any(), Arg.Any(), Arg.Any()); + } + [Fact] public async Task PupilSearchPost_WhenIncludeJourneyAndIncludedLookupFails_ProceedsNormally() { @@ -708,7 +737,7 @@ public async Task PupilSearchPost_WhenIncludeJourneyAndIncludedLookupFails_Proce }; SetupIncludeJourney(); _pupilDataService.GetPupilAsync(WindowId, PrimaryPupilId).Returns(nonIncludedPupil); - _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.Included) + _pupilDataService.GetPupilSuggestionsAsync(WindowId, "Casey Carter", PupilFilter.NotIncludable) .Returns(Task.FromException>(new TimeoutException())); var result = await _sut.PupilSearchPost(WindowId, "select-pupil", PrimaryPupilId.ToString(), "Carter, Casey, 01/01/2010"); @@ -725,7 +754,7 @@ private void SetupIncludeJourney() Pages = [new JourneyPage { Id = "select-pupil", Type = PageType.PupilSearch, - PupilFilter = PupilFilter.NonIncluded, PupilKey = JourneyPage.PrimaryKey, + PupilFilter = PupilFilter.Includable, PupilKey = JourneyPage.PrimaryKey, NextPageId = "evidence" }] }; diff --git a/tests/DfE.CheckPerformanceData.UnitTests/Web/Views/IncludeDecisionViewSourceTests.cs b/tests/DfE.CheckPerformanceData.UnitTests/Web/Views/IncludeDecisionViewSourceTests.cs new file mode 100644 index 000000000..b04c4dc2d --- /dev/null +++ b/tests/DfE.CheckPerformanceData.UnitTests/Web/Views/IncludeDecisionViewSourceTests.cs @@ -0,0 +1,66 @@ +namespace DfE.CheckPerformanceData.Application.UnitTests.Web.Views; + +// #711: the Include journey's two decision pages are shared between key stages. They must take +// the noun from the model ("student" on 16-19) and offer "Start adding" only when the window type +// has an Add flow — 16-19 has none, so the button would only bounce to Check your data. +public sealed class IncludeDecisionViewSourceTests +{ + private static string ReadViewSource(string relativePath) + { + var viewsDir = Path.GetFullPath(Path.Combine( + AppContext.BaseDirectory, + "..", "..", "..", "..", "..", + "src", "DfE.CheckPerformanceData.Web")); + return File.ReadAllText(Path.Combine(viewsDir, relativePath)); + } + + [Theory] + [InlineData("Views/Journey/PupilNotFound.cshtml")] + [InlineData("Views/Journey/AlreadyIncluded.cshtml")] + public void The_page_wraps_the_add_form_in_CanAdd(string view) + { + var source = ReadViewSource(view); + + Assert.Contains("@if (Model.CanAdd)", source); + Assert.Contains("StartAddingPupil", source); + } + + [Theory] + [InlineData("Views/Journey/PupilNotFound.cshtml")] + [InlineData("Views/Journey/AlreadyIncluded.cshtml")] + public void The_page_takes_its_noun_from_the_model_and_never_says_pupil(string view) + { + var source = ReadViewSource(view); + + Assert.Contains("Model.LearnerNoun.", source); + // The rendered sentences. Code may still say "pupil" (the StartAddingPupil action, the + // "select-pupil" page id, the model type), so the check names the old prose, not the word. + foreach (var oldProse in new[] + { + "Pupil not found", "No pupil", "the pupil", "this pupil", "This pupil", "new pupil", + "Matching pupils", "Pupils matching", "non-included pupils", "a pupil matching" + }) + { + Assert.DoesNotContain(oldProse, source); + } + } + + [Theory] + [InlineData("Views/Journey/PupilNotFound.cshtml")] + [InlineData("Views/Journey/AlreadyIncluded.cshtml")] + public void The_title_and_the_heading_are_the_same_words(string view) + { + var source = ReadViewSource(view); + + // docs/accessibility.md: ViewData["Title"] carries the same words as the one

. The + // title is a C# interpolated string and the heading is Razor, so the noun appears as + // {Model.LearnerNoun.X} in one and @Model.LearnerNoun.X in the other. + var title = System.Text.RegularExpressions.Regex.Match(source, @"ViewBag\.Title\s*=\s*(\$?"".*?"");").Groups[1].Value + .Trim('$', '"') + .Replace("{Model.LearnerNoun.SingularCapitalised}", "@Model.LearnerNoun.SingularCapitalised") + .Replace("{Model.LearnerNoun.Singular}", "@Model.LearnerNoun.Singular"); + var heading = System.Text.RegularExpressions.Regex.Match(source, @"]*>(.*?)

").Groups[1].Value.Trim(); + Assert.False(string.IsNullOrEmpty(title)); + Assert.Equal(title, heading); + } +}