Checking Window / Checking Exercise refactor AB#297801

docs/request-journey.md

@@ -122,12 +122,15 @@ All flow configs begin with one or more `PupilSearch` pages. These are full-page
 |---|---|---|---|
 | `pupilFilter` | yes | `"Included"` / `"All"` | `Included` limits results to Pincl codes `[401,403,414,421,431]`; `All` returns every pupil for the school |
 | `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 |
 | `nextPageId` | no | page id string | Absent → redirect to Summary after selection |
 | `validationFailure` | no | string | Error shown when no pupil is submitted. Supports `{pupilName}`. Falls back to `"Enter the name of the pupil"` |
 
-**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All&excludePupilId={guid}`  
+**Suggestions endpoint:** `GET /pupils/suggestions?windowId={id}&query={q}&filter=Included|All&excludePupilId={guid}&requireResults=true`  
 Served by `PupilSuggestionsController`. Queries PostgreSQL via `ICheckYourPupilDataService.GetPupilSuggestionsAsync`. 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 (`IStudentResultsClient.GetStudentIdsWithResultsAsync`, 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.
+
 **On successful pupil selection (`PupilSearchPost`):**
 
 - **`pupilKey: "primary"`** — saves to `SelectedPupil*`, generates the reference number, resets `QuestionAnswers` and `QuestionHistory`. This is the pupil the request is about.

docs/results-enquiry.md

@@ -13,8 +13,8 @@ data), AB#297013.
 | `check-late-results` | `Content` | Guidance: check your second late results file first. Entered by the controller, not the flow's `firstPageId` — see [Late results guidance](#late-results-guidance). |
 | `cohort-scope` | `Question` / Radio | Does the incorrect grade affect the whole cohort? Branches the journey. |
 | `cohort-count` | `Question` / FreeText | How many students (cohort branch only). Validated by the `WholeNumber` format validator. |
-| `select-student-cohort` | `PupilSearch` | One student as an example (cohort branch). |
-| `select-student-single` | `PupilSearch` | The affected student (single branch). |
+| `select-student-cohort` | `PupilSearch` | One student as an example (cohort branch). Lists only students who hold results — see [Only students who hold results](#only-students-who-hold-results). |
+| `select-student-single` | `PupilSearch` | The affected student (single branch). Same restriction. |
 | `select-result` | `ResultSearch` | Which of the student's results is wrong. |
 | `grade-details` | `ResultDetails` | Shows the chosen result; asks for the revised grade. |
 | `additional-info` | `Question` / TextArea | Optional comments, 250 characters. |
@@ -63,6 +63,14 @@ decides what "the second late results file has landed" means.
 Container `{windowId}`, blob `results-enquiry/data/{laestab}_results.json`. One merged array per
 school across all six supplier files, each row stamped with its source tag.
 
+Written by the results-enquiry checking exercise's own ingress run (#324): one dataset slot per
+source file, each stamping its `SOURCE` tag onto every record it contributes, all merged into one
+file per school in a single run. `SeedStudentResults` still writes the same blob in development, so
+a developer needs no supplier files. Only the main file is required to validate the exercise — the
+late, revised and retention files are optional slots, because they land weeks apart and one may
+never land, and each run rewrites the school's whole file from the slots that are filled. The supplier CSVs must carry a `LAESTAB` column — that is what
+splits one file into one blob per school — and a file without one fails the run by name.
+
 The `results-enquiry/` prefix is deliberate: per consequence #2 of `docs/16-19-window-model.md` each
 checking exercise owns its own blob prefix, so when ingress becomes per-exercise no migration is
 needed and one exercise's sweep cannot destroy another's output. Pupil-data checking keeps its bare
@@ -104,6 +112,40 @@ The checked-in seed holds the three AB#297130 examples plus the dev QANs. The IB
 scale is derived from the ticket (44 pass: `24B`/`24D` … `45B`/`45D`; 49 fail: `00F`–`45F`, `R`, `U`,
 `X`) and is what gives the tests their `24F`-vs-`24D` case.
 
+## Only students who hold results
+
+Both `PupilSearch` pages set `"requireResults": true`. A student with no result has no grade to
+correct, so they are not a candidate, and offering them leads only to a dead end.
+
+How it is wired, layer by layer:
+
+1. `IStudentResultsClient.GetStudentIdsWithResultsAsync(windowId, laestab)` returns the school's CYPMD
+   ids, case-insensitively, from the **already cached** results file — an autocomplete keystroke costs
+   no download.
+2. `CheckYourPupilDataService.GetPupilSuggestionsAsync(..., requireResults)` resolves that set only
+   when asked, and hands it to the repository. Every other journey passes null and searches the whole
+   roll.
+3. `CheckYourPupilDataRepository.SearchPupilsAsync(..., cypmdIdAllowList)` applies it **before** the
+   ten-suggestion cap. Filtering after the cap would drop the one student who holds results whenever
+   ten who do not sort ahead of them.
+
+Persistence never learns what a result is — it receives a set of ids.
+
+The restriction is a search restriction, never a permission. It only ever narrows a search that is
+already scoped to the signed-in school's own file, so a request that forges or omits
+`requireResults=true` reaches nothing new.
+
+**Because it hides students, the pages say so.** The `subheading` ends "You can only search for
+students who have results", and the autocomplete's no-match text becomes "No students found with
+results" rather than the component's default "No results found" — otherwise a school cannot tell a
+typo from a student who holds nothing. Copy on both is FLAGGED for content sign-off.
+
+`select-result` keeps its own empty state for the cases the restriction cannot cover — back
+navigation, a stale session, or a results file that changes mid-journey. Rather than an autocomplete
+that can never answer, it states that we hold no results for the student and links back to the
+student search. It renders instead of the control and the Continue button, which could only ever
+fail validation.
+
 ## Revised-grade rules
 
 Server-authoritative, in this order (`JourneyValidationService.ValidateGradeSelect`):
@@ -218,13 +260,21 @@ Validation failures flow through the existing `validation_error` event; `GradeSe
 
 ## Local development
 
-`SeedStudentResults` writes results for Kingsmead (`860/4070`) in the seeded Post16 window. Three
-students, mixed `16to19_MAIN` / `16to19_LR1` tags, one qualification held twice in different sessions,
-and **no `16to19_LR2` rows** so the interstitial is on the happy path.
+`SeedStudentResults` writes results for Kingsmead (`860/4070`) in the seeded Post16 window: mixed
+`16to19_MAIN` / `16to19_LR1` tags, one qualification held twice in different sessions, and **no
+`16to19_LR2` rows** so the interstitial is on the happy path.
+
+Three students (`500001`–`500003`) carry the Figma screens' own qualification fixtures. The CYPMD ids
+are the ones `SeedPupilData` actually generates — a result keyed to Figma's own id would belong to no
+selectable student and dead-end the journey. E2E drives `500001` by name, so those three rows are
+pinned by `SeedStudentResultsTests`.
 
-The qualification fixtures come from the Figma screens, but the CYPMD ids are the ones
-`SeedPupilData` actually generates (`500001`–`500003`) — a result keyed to Figma's own id would belong
-to no selectable student and dead-end the journey.
+The rest is generated across both populations (every third included student, every fifth
+non-included), giving roughly a quarter of the school. That is deliberate on both sides: with the
+search restricted to students who hold results, three students leave a manual tester unable to
+exercise a common-surname search or the ten-suggestion cap, while seeding *everyone* would hide both
+the restriction and the empty state behind data that never exercises them. Generated qualifications
+come from the seeded grade reference, so the revised-grade picker can always list grades.
 
 ```
 docker compose --profile web --profile database --profile storage up -d --build
@@ -250,7 +300,8 @@ states, dataset reparenting, per-exercise ingress, draft-across-boundary rules.
 ## Deliberately out of scope
 
 The "Review exam results" / Results / Late-results tab pages and CSV/ZIP downloads (entry-point
-ticket); the six-file ingestion pipeline (FACT tickets — this feature seeds the blobs it reads);
+ticket); the six-file ingestion pipeline itself (FACT tickets — the portal side of it, the admin upload and
+ingress run that fill these blobs, is #324);
 missing-qualification and result-does-not-belong-to-student flows (sibling tickets); drafts (decided
 against); duplicate-enquiry blocking (the spec allows multiples).
 

src/DfE.CheckPerformanceData.Application/AmendmentRequests/AmendmentRequestsResult.cs

@@ -1,9 +1,28 @@
+using DfE.CheckPerformanceData.Domain.Enums;
+
 namespace DfE.CheckPerformanceData.Application.AmendmentRequests;
 
 public sealed class AmendmentRequestsResult
 {
-    public required DateTime WindowEndDate { get; init; }
     public required string WindowTitle { get; init; }
+
+    /// <summary>
+    /// One deadline per checking exercise the window runs, in sort order (#320). The page used to
+    /// print the outer window's end date, which on a 16-19 window is the results-enquiry close —
+    /// months after pupil data checking shuts. Since the grid holds both populations, one date
+    /// could not be right for both.
+    /// </summary>
+    public required IReadOnlyList<ExerciseDeadlineDto> Deadlines { get; init; }
     public required IReadOnlyList<AmendmentRequestDto> Rows { get; init; }
     public required IReadOnlyList<SubmittedRequestDto> SubmittedRows { get; init; }
 }
+
+/// <summary>When one of the window's checking exercises closes, and whether it still has.</summary>
+public sealed class ExerciseDeadlineDto
+{
+    public required CheckingExerciseType Exercise { get; init; }
+    public required DateTime EndDate { get; init; }
+
+    /// <summary>False once the deadline has passed, so the page can say so in the past tense.</summary>
+    public required bool IsOpen { get; init; }
+}

src/DfE.CheckPerformanceData.Application/AmendmentRequests/AmendmentRequestsService.cs

@@ -1,12 +1,14 @@
 using DfE.CheckPerformanceData.Application.CheckYourPupilData;
 using DfE.CheckPerformanceData.Application.CurrentUser;
 using DfE.CheckPerformanceData.Application.RequestSubmission;
+using DfE.CheckPerformanceData.Application.WindowManagement;
 
 namespace DfE.CheckPerformanceData.Application.AmendmentRequests;
 
 public sealed class AmendmentRequestsService(
     ICheckYourPupilDataService checkYourPupilDataService,
     IRequestRepository requestRepository,
+    ICheckingExerciseService checkingExercises,
     ICurrentUserService currentUserService) : IAmendmentRequestsService
 {
     public async Task<AmendmentRequestsResult> GetAmendmentRequestsAsync(Guid windowId)
@@ -18,8 +20,20 @@ public async Task<AmendmentRequestsResult> GetAmendmentRequestsAsync(Guid window
 
         return new AmendmentRequestsResult
         {
-            WindowEndDate = window.EndDate,
             WindowTitle = window.Title,
+            // #320: a deadline per exercise the window runs, not the outer window's end date. The
+            // grid lists both populations, and on a 16-19 window pupil data checking shuts months
+            // before results enquiry does — one date could only ever be right for one of them.
+            Deadlines = window.Exercises
+                .OrderBy(e => e.SortOrder)
+                .Select(e => new ExerciseDeadlineDto
+                {
+                    Exercise = e.ExerciseType,
+                    EndDate = e.EndDate,
+                    // The clock lives in one place, so "has this closed" is asked, never computed.
+                    IsOpen = checkingExercises.IsOpen(window.Exercises, e.ExerciseType)
+                })
+                .ToList(),
             Rows = requests.Select(r => new AmendmentRequestDto
             {
                 PupilName = PupilNameFormatter.Format(r.PupilFirstname, r.PupilSurname),

src/DfE.CheckPerformanceData.Application/CheckYourPupilData/CheckYourPupilDataService.cs

@@ -2,12 +2,14 @@
 using DfE.CheckPerformanceData.Application.CurrentUser;
 using DfE.CheckPerformanceData.Application.Journey;
 using DfE.CheckPerformanceData.Application.LandingPage;
+using DfE.CheckPerformanceData.Application.ResultsEnquiry;
 
 namespace DfE.CheckPerformanceData.Application.CheckYourPupilData;
 
 public sealed class CheckYourPupilDataService(
     ICheckYourPupilDataRepository repository,
-    ICurrentUserService currentUserService) : ICheckYourPupilDataService
+    ICurrentUserService currentUserService,
+    IStudentResultsClient studentResultsClient) : ICheckYourPupilDataService
 {
     public async Task<(PupilTable Table, int TotalCount)> GetPupilTableAsync(Guid windowId, bool included, string? search, int page, int pageSize)
     {
@@ -30,11 +32,19 @@ public async Task<PupilTable> GetPupilCsvAsync(Guid windowId, bool included)
     public Task<CheckingWindowDto> GetCheckingWindowAsync(Guid windowId)
         => repository.GetCheckingWindowAsync(windowId);
 
-    public async Task<IReadOnlyList<PupilSuggestionDto>> GetPupilSuggestionsAsync(Guid windowId, string query, PupilFilter filter, Guid? excludeId = null)
+    public async Task<IReadOnlyList<PupilSuggestionDto>> GetPupilSuggestionsAsync(Guid windowId, string query, PupilFilter filter, Guid? excludeId = null, bool requireResults = false)
     {
         var laestab = currentUserService.OrganisationLaestab;
         var urn = currentUserService.OrganisationUrn;
-        return await repository.SearchPupilsAsync(windowId, laestab, urn, query, filter, excludeId);
+
+        // A results enquiry names a student whose grade is wrong, so a student with no result is
+        // not a candidate. The set comes from the same cached school file the enquiry itself reads,
+        // and is resolved only when asked for — every other journey searches the whole roll.
+        var withResults = requireResults
+            ? await studentResultsClient.GetStudentIdsWithResultsAsync(windowId, laestab)
+            : null;
+
+        return await repository.SearchPupilsAsync(windowId, laestab, urn, query, filter, excludeId, withResults);
     }
 
     public async Task<PupilDto> GetPupilAsync(Guid windowId, Guid pupilId)

src/DfE.CheckPerformanceData.Application/CheckYourPupilData/ICheckYourPupilDataRepository.cs

@@ -13,6 +13,16 @@ public interface ICheckYourPupilDataRepository
     Task<IReadOnlyList<IPupilRecord>> GetAllPupilsAsync(Guid windowId, string laestab, bool included);
 
     Task<CheckingWindowDto> GetCheckingWindowAsync(Guid windowId);
-    Task<IReadOnlyList<PupilSuggestionDto>> SearchPupilsAsync(Guid windowId, string laestab, string urn, string query, PupilFilter filter, Guid? excludeId = null);
+
+    /// <summary>
+    /// Autocomplete suggestions for the pupil search, capped at ten.
+    ///
+    /// <paramref name="cypmdIdAllowList"/> restricts the search to a set of students, and is how a
+    /// results enquiry keeps a school from naming a student who holds no result. Null means no
+    /// restriction (every other journey); an empty set correctly matches nobody. It is applied
+    /// before the cap, so a student who does hold results is never crowded out by ten who do not.
+    /// </summary>
+    Task<IReadOnlyList<PupilSuggestionDto>> SearchPupilsAsync(Guid windowId, string laestab, string urn, string query, PupilFilter filter, Guid? excludeId = null, IReadOnlySet<string>? cypmdIdAllowList = null);
+
     Task<PupilDto> GetPupilAsync(Guid windowId, string laestab, Guid pupilId);
 }

src/DfE.CheckPerformanceData.Application/CheckYourPupilData/ICheckYourPupilDataService.cs

@@ -14,8 +14,15 @@ public interface ICheckYourPupilDataService
     Task<PupilTable> GetPupilCsvAsync(Guid windowId, bool included);
 
     Task<CheckingWindowDto> GetCheckingWindowAsync(Guid windowId);
+    /// <summary>
+    /// Autocomplete suggestions for the pupil search.
+    ///
+    /// <paramref name="requireResults"/> limits the search to students the school holds a result
+    /// for — a results enquiry has nothing to correct otherwise. It costs a read of the school's
+    /// (cached) results file, so it is opt-in rather than the default.
+    /// </summary>
     Task<IReadOnlyList<PupilSuggestionDto>> GetPupilSuggestionsAsync(Guid windowId, string query,
-        PupilFilter filter, Guid? excludeId = null);
+        PupilFilter filter, Guid? excludeId = null, bool requireResults = false);
 
     Task<PupilDto> GetPupilAsync(Guid windowId, Guid pupilId);
 }

src/DfE.CheckPerformanceData.Application/CheckYourPupilData/INextStepsService.cs

@@ -0,0 +1,44 @@
+using DfE.CheckPerformanceData.Application.WindowManagement;
+using DfE.CheckPerformanceData.Domain.Enums;
+
+namespace DfE.CheckPerformanceData.Application.CheckYourPupilData;
+
+/// <summary>
+/// What the check-your-pupil-data page may offer right now. The page offers whatever is open, for
+/// any number of exercises, so the options follow the exercise dates rather than the window type
+/// (#317).
+/// </summary>
+public interface INextStepsService
+{
+    /// <summary>Next-step options for the exercises open right now, in display order.</summary>
+    IReadOnlyList<NextSteps> GetAvailableSteps(IReadOnlyList<CheckingExerciseDto> exercises);
+}
+
+/// <inheritdoc />
+/// <remarks>
+/// The mapping is domain knowledge, so it lives here rather than in the controller. Adding a future
+/// exercise type must mean adding a row to <see cref="StepsByExercise"/>, never editing branching
+/// logic — and no branch here may look at <c>CheckingWindowType</c>. Which exercises are open is
+/// never decided here either: that is <see cref="ICheckingExerciseService"/>'s single job, and it
+/// owns the only clock in this path.
+/// </remarks>
+public sealed class NextStepsService(ICheckingExerciseService checkingExercises) : INextStepsService
+{
+    /// <summary>
+    /// One entry per exercise type. RequestChange and Confirm both belong to PupilData, so they
+    /// appear and disappear together when that exercise opens and closes.
+    /// </summary>
+    private static readonly Dictionary<CheckingExerciseType, NextSteps[]> StepsByExercise = new()
+    {
+        [CheckingExerciseType.PupilData] = [NextSteps.RequestChange, NextSteps.Confirm],
+        [CheckingExerciseType.ResultsEnquiry] = [NextSteps.ResultsEnquiry]
+    };
+
+    public IReadOnlyList<NextSteps> GetAvailableSteps(IReadOnlyList<CheckingExerciseDto> exercises) =>
+        checkingExercises.OpenCheckingExercises(exercises)
+            // An exercise with no mapping contributes nothing. Fail closed rather than throw: it
+            // must not offer a journey with nothing behind it, but nor should one unmapped row take
+            // the whole page down. NextStepsServiceTests pins that every type that exists is mapped.
+            .SelectMany(e => StepsByExercise.TryGetValue(e, out var steps) ? steps : [])
+            .ToList();
+}