From c16483f152a3720b059ae22004120009ce837d28 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 13:19:39 +0800 Subject: [PATCH 01/17] Core functionality checkpoint --- frontend/src/components/Common.tsx | 29 +-- .../layout/main/ApplicationCard.tsx | 105 +++++---- .../src/components/layout/main/Review.tsx | 93 +++++++- .../src/components/layout/main/ReviewCard.tsx | 216 ++++++++++++++---- frontend/src/context/ApiManager.tsx | 26 ++- 5 files changed, 349 insertions(+), 120 deletions(-) diff --git a/frontend/src/components/Common.tsx b/frontend/src/components/Common.tsx index b9576a3..3c75332 100644 --- a/frontend/src/components/Common.tsx +++ b/frontend/src/components/Common.tsx @@ -7,9 +7,9 @@ import Grid from '@mui/material/Grid'; import IconButton from '@mui/material/IconButton'; import Link from '@mui/material/Link'; import TextField from '@mui/material/TextField'; +import Tooltip from '@mui/material/Tooltip'; import Typography from "@mui/material/Typography"; -import Tooltip from '@mui/material/Tooltip'; import type { TypographyProps } from "@mui/material/Typography"; import { useRef } from 'react'; import { ApiManager } from '../context/ApiManager'; @@ -249,18 +249,19 @@ export const ApplicationIdDisplay = ({ const isSmallVariant = variant === 'caption' || variant === 'body2'; return ( - - - {internalId} - + + + + {internalId} + + ); }; diff --git a/frontend/src/components/layout/main/ApplicationCard.tsx b/frontend/src/components/layout/main/ApplicationCard.tsx index d99d402..798fa22 100644 --- a/frontend/src/components/layout/main/ApplicationCard.tsx +++ b/frontend/src/components/layout/main/ApplicationCard.tsx @@ -12,6 +12,7 @@ import ListItem from "@mui/material/ListItem"; import Step from "@mui/material/Step"; import StepLabel from "@mui/material/StepLabel"; import Stepper from "@mui/material/Stepper"; +import Tooltip from '@mui/material/Tooltip'; import React from "react"; import { ApiManager } from '../../../context/ApiManager'; @@ -166,76 +167,72 @@ export const ApplicationCard = ({ {/* Discard button on left—only for editable (DRAFT) applications. */} {isEditable && ( - + + + )} {/* Revert button on left—only for discarded applications. */} {isDiscarded && ( - + + + )} {/* Download and Continue buttons—push to the right. */} {/* Render the PDF action only for downloadable statuses. */} {isDownloadable && ( - - - + + + )} {/* Render the continue action only for editable applications. */} {isEditable && ( - openNewTab(`/a/${application.key}`, application.key)} - > - - + + + )} diff --git a/frontend/src/components/layout/main/Review.tsx b/frontend/src/components/layout/main/Review.tsx index 90d076f..548a343 100644 --- a/frontend/src/components/layout/main/Review.tsx +++ b/frontend/src/components/layout/main/Review.tsx @@ -1,5 +1,7 @@ import Box from "@mui/material/Box"; import List from "@mui/material/List"; +import Tab from "@mui/material/Tab"; +import Tabs from "@mui/material/Tabs"; import Typography from "@mui/material/Typography"; import { useEffect, useMemo, useState } from "react"; @@ -23,11 +25,23 @@ const reviewSortOrderStorageKey = "review-sort-order"; /** * Displays applications in the review queue for technical officers. + * Organises applications into tabs by status: Submitted, Under Review, Under Assessment. * Applies reusable sorting controls and respects user preferences. */ export const ApplicationReview = () => { const { processes, applications: applicationsPromise } = useLoaderData(); - const [applications, isApplicationsLoading] = useResolvedPromise(applicationsPromise, []); + const [resolvedApplications, isApplicationsLoading] = useResolvedPromise(applicationsPromise, []); + const [applicationUpdates, setApplicationUpdates] = useState>({}); + const [selectedTab, setSelectedTab] = useState(0); + + /** + * Computes the merged applications list by overlaying any updates on the resolved applications. + * This preserves the loading state while allowing real-time status changes to be reflected. + */ + const applications = useMemo( + () => resolvedApplications.map((app) => applicationUpdates[app.key] ?? app), + [resolvedApplications, applicationUpdates], + ); const [sortOrder, setSortOrder] = useState(() => getInitialSortOrder(reviewSortOrderStorageKey, "submitted_oldest") @@ -37,6 +51,17 @@ export const ApplicationReview = () => { LocalStorage.setValue(reviewSortOrderStorageKey, sortOrder); }, [sortOrder]); + /** + * Handles status changes from individual ReviewCard components. + * Records the update so re-categorisation and tab switching occur on the next render. + */ + const handleApplicationStatusChanged = (updatedApp: IApplicationData) => { + setApplicationUpdates((prev) => ({ + ...prev, + [updatedApp.key]: updatedApp, + })); + }; + const processBySlug = useMemo( () => new Map(processes.map((process) => [process.slug, process])), [processes] @@ -47,6 +72,29 @@ export const ApplicationReview = () => { [applications, sortOrder] ); + /** + * Groups applications by their review status into three categories. + * Enables tab-based filtering for reviewers to navigate the review workflow. + */ + const categorisedApplications = useMemo(() => ({ + submitted: sortedReviewApplications.filter((app) => app.status === "SUBMITTED"), + underReview: sortedReviewApplications.filter((app) => app.status === "UNDER_REVIEW"), + underAssessment: sortedReviewApplications.filter((app) => app.status === "UNDER_ASSESSMENT"), + }), [sortedReviewApplications]); + + // Map tab index to the corresponding applications list for the selected tab. + const applicationsForTab = [ + categorisedApplications.submitted, + categorisedApplications.underReview, + categorisedApplications.underAssessment, + ][selectedTab] || []; + + const tabDescriptions = [ + "Claim submitted applications for administrative review.", + "Perform administrative review and escalate to assessment.", + "Finalise assessments and make approval decisions.", + ]; + return ( @@ -62,19 +110,54 @@ export const ApplicationReview = () => { /> } - - Review and action applications in your queue. + + {/* Tab navigation for review queue statuses. */} + + setSelectedTab(newValue)} + aria-label="Application review status filter" + role="tablist" + > + + + + + + + + {tabDescriptions[selectedTab]} {isApplicationsLoading ? : - sortedReviewApplications.length === 0 ? : + applicationsForTab.length === 0 ? : - {sortedReviewApplications.map((application) => { + {applicationsForTab.map((application) => { const process = processBySlug.get(application.process_slug); return ; })} diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index 3a89963..a03dc50 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -2,11 +2,16 @@ import AttachFileIcon from '@mui/icons-material/AttachFile'; import DownloadIcon from '@mui/icons-material/Download'; import EmailIcon from '@mui/icons-material/Email'; import HistoryIcon from '@mui/icons-material/History'; +import NavigateNextRoundedIcon from '@mui/icons-material/NavigateNextRounded'; import PersonIcon from '@mui/icons-material/Person'; +import RestartAltRoundedIcon from '@mui/icons-material/RestartAltRounded'; +import ZoomInRoundedIcon from '@mui/icons-material/ZoomInRounded'; import Box from "@mui/material/Box"; import Button from "@mui/material/Button"; import Card from "@mui/material/Card"; import Chip from "@mui/material/Chip"; +import IconButton from "@mui/material/IconButton"; +import Tooltip from "@mui/material/Tooltip"; import Typography from "@mui/material/Typography"; import Link from '@mui/material/Link'; import ListItem from "@mui/material/ListItem"; @@ -14,11 +19,10 @@ import ListItem from "@mui/material/ListItem"; import { useState } from 'react'; import { ApiManager } from '../../../context/ApiManager'; import { useDialog, useResolvedPromise, useSnackbar } from '../../../context/Hooks'; -import type { IApplicationAttachment, IApplicationData } from "../../../context/types/Application"; +import type { ApplicationStatus, IApplicationAttachment, IApplicationData } from "../../../context/types/Application"; import type { IAuthorisationProcess } from '../../../context/types/Questionnaire'; import { ApplicationIdDisplay, FileAttachmentList } from '../../Common'; import { - downloadableStatuses, formatRelativeDates, formatStatusLabel, } from './applicationUtils'; @@ -56,24 +60,26 @@ export const AttachmentsDialogContent = ({ /** * Renders an application summary card for technical officers in the review queue. - * Displays process metadata, application status, and review/download action buttons. + * Displays process metadata, application status, and reviewer workflow action buttons. + * Notifies parent via callback when application status changes. */ export const ReviewCard = ({ process, application, + onStatusChanged, }: { process?: IAuthorisationProcess; application: IApplicationData; + onStatusChanged: (updatedApp: IApplicationData) => void; }) => { const { showDialog } = useDialog(); const { showSnackbar } = useSnackbar(); + const [displayedApplication, setDisplayedApplication] = useState(application); const processName = process?.name ?? `Unknown process (${application.process_slug})`; const questionnaireName = `${application.questionnaire_name} (v${application.questionnaire_version})`; - const statusCapitalised = formatStatusLabel(application.status); - const { createdAtRelative, updatedAtRelative, submittedAtRelative } = formatRelativeDates(application); - - const isDownloadable = downloadableStatuses.has(application.status); + const statusCapitalised = formatStatusLabel(displayedApplication.status); + const { createdAtRelative, updatedAtRelative, submittedAtRelative } = formatRelativeDates(displayedApplication); const handleFilesClick = () => { showDialog({ @@ -92,10 +98,107 @@ export const ReviewCard = ({ }); }; + /** + * Transition application from SUBMITTED to UNDER_REVIEW. + * Reviewer claims the application for administrative review. + */ + const handleClaim = async () => { + try { + const updatedApp = await ApiManager.updateReviewerApplicationStatus( + displayedApplication.key, + "UNDER_REVIEW" as ApplicationStatus, + ); + setDisplayedApplication(updatedApp); + showSnackbar("Application claimed for review.", "success"); + onStatusChanged(updatedApp); + } catch (error: unknown) { + showSnackbar( + "Failed to claim application. Please try again later.", + "error", + ); + console.error("Error claiming application:", error); + } + }; + + /** + * Reset application from UNDER_REVIEW or UNDER_ASSESSMENT to DRAFT. + * Resets application to DRAFT status so applicant can revise and resubmit. + */ + const handleResetToDraft = async () => { + try { + const updatedApp = await ApiManager.updateReviewerApplicationStatus( + displayedApplication.key, + "DRAFT" as ApplicationStatus, + ); + setDisplayedApplication(updatedApp); + showSnackbar("Application reset to draft for revision.", "info"); + onStatusChanged(updatedApp); + } catch (error: unknown) { + showSnackbar( + "Failed to return application. Please try again later.", + "error", + ); + console.error("Error returning application:", error); + } + }; + + /** + * Transition application from UNDER_REVIEW to UNDER_ASSESSMENT. + * Escalates application to technical assessment after administrative checks pass. + */ + const handleProceedtoAssessment = async () => { + try { + const updatedApp = await ApiManager.updateReviewerApplicationStatus( + displayedApplication.key, + "UNDER_ASSESSMENT" as ApplicationStatus, + ); + setDisplayedApplication(updatedApp); + showSnackbar("Application moved to assessment.", "success"); + onStatusChanged(updatedApp); + } catch (error: unknown) { + showSnackbar( + "Failed to move application to assessment. Please try again later.", + "error", + ); + console.error("Error moving application to assessment:", error); + } + }; + return ( - + {/* Header: Application ID on left, PDF/Files on right */} + + + + + + + + + + + + + + + + + @@ -120,16 +223,17 @@ export const ReviewCard = ({ {/* Email - Clickable for copy to clipboard */} - - - - {application.owner_email} - - + + + + + {application.owner_email} + + + {/* Submission Date */} @@ -140,38 +244,58 @@ export const ReviewCard = ({ - - - - {/* Render the PDF action only for downloadable statuses. */} - {isDownloadable && ( - + {/* Action buttons: left and right justified with space-between. */} + + {displayedApplication.status === "SUBMITTED" && ( + - + + )} + {displayedApplication.status === "UNDER_REVIEW" && ( + <> + + + + +
+ +
+ + + + + )} diff --git a/frontend/src/context/ApiManager.tsx b/frontend/src/context/ApiManager.tsx index f3b856d..2659a4a 100644 --- a/frontend/src/context/ApiManager.tsx +++ b/frontend/src/context/ApiManager.tsx @@ -2,7 +2,7 @@ import axios from "axios"; import type { AxiosProgressEvent, AxiosRequestConfig } from "axios"; import { ConfigManager } from "./ConfigManager"; -import type { IApplicationAttachment, IApplicationData, IFormDocument } from "./types/Application"; +import type { ApplicationStatus, IApplicationAttachment, IApplicationData, IFormDocument } from "./types/Application"; import type { IAuthorisationProcess, IQuestionnaireData } from "./types/Questionnaire"; @@ -208,4 +208,28 @@ export class ApiManager { return response.data; } + + /** + * Update the status of an application in the review queue. + * Sends a PATCH request to advance the application through review workflow states. + * Transition validity is enforced by the backend serialiser. + * + * @param key - The application key (UUID) + * @param status - The target status (must be a valid reviewer-initiated transition) + * @returns The updated application data + * @throws AxiosError if the transition is invalid or user lacks reviewer permissions + */ + public static async updateReviewerApplicationStatus( + key: string, + status: ApplicationStatus, + ): Promise { + const requestConfig = ApiManager.getRequestConfig(); + const response = await axios.patch( + `/review/${key}`, + { status }, + requestConfig, + ); + + return response.data; + } } From e63db5cf9e5f6fac3e3e9bb3f179f0c57f6c868d Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 13:52:06 +0800 Subject: [PATCH 02/17] Application card status change highlighting --- .../src/components/layout/main/Review.tsx | 41 ++++++++++++++++++- .../src/components/layout/main/ReviewCard.tsx | 10 ++++- frontend/src/index.css | 13 ++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/layout/main/Review.tsx b/frontend/src/components/layout/main/Review.tsx index 548a343..525a003 100644 --- a/frontend/src/components/layout/main/Review.tsx +++ b/frontend/src/components/layout/main/Review.tsx @@ -4,7 +4,7 @@ import Tab from "@mui/material/Tab"; import Tabs from "@mui/material/Tabs"; import Typography from "@mui/material/Typography"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useLoaderData } from "react-router"; import { useResolvedPromise } from "../../../context/Hooks"; import { LocalStorage } from "../../../context/LocalStorage"; @@ -33,6 +33,8 @@ export const ApplicationReview = () => { const [resolvedApplications, isApplicationsLoading] = useResolvedPromise(applicationsPromise, []); const [applicationUpdates, setApplicationUpdates] = useState>({}); const [selectedTab, setSelectedTab] = useState(0); + const [highlightedAppKey, setHighlightedAppKey] = useState(null); + const cardRefsMap = useRef>(new Map()); /** * Computes the merged applications list by overlaying any updates on the resolved applications. @@ -53,15 +55,48 @@ export const ApplicationReview = () => { /** * Handles status changes from individual ReviewCard components. - * Records the update so re-categorisation and tab switching occur on the next render. + * Records the update, switches to the appropriate tab, and highlights the changed application. */ const handleApplicationStatusChanged = (updatedApp: IApplicationData) => { setApplicationUpdates((prev) => ({ ...prev, [updatedApp.key]: updatedApp, })); + + // Switch to the tab matching the new status and highlight the application. + const tabIndex = updatedApp.status === "SUBMITTED" ? 0 : updatedApp.status === "UNDER_REVIEW" ? 1 : 2; + setSelectedTab(tabIndex); + setHighlightedAppKey(updatedApp.key); + + // Clear highlight after animation completes. + setTimeout(() => { + setHighlightedAppKey(null); + }, 3000); + }; + + /** + * Registers a card element in the refs map for scroll-to-view targeting. + */ + const handleCardElementMounted = (appKey: string, element: HTMLElement | null) => { + if (element) { + cardRefsMap.current.set(appKey, element); + } else { + cardRefsMap.current.delete(appKey); + } }; + /** + * Scrolls the highlighted card into view, centered on the screen. + */ + useEffect(() => { + if (highlightedAppKey) { + const card = cardRefsMap.current.get(highlightedAppKey); + if (card) { + card.scrollIntoView({ behavior: "smooth", block: "center" }); + } + } + }, [highlightedAppKey]); + const processBySlug = useMemo( () => new Map(processes.map((process) => [process.slug, process])), [processes] @@ -157,7 +192,9 @@ export const ApplicationReview = () => { key={application.key} application={application} process={process} + isHighlighted={application.key === highlightedAppKey} onStatusChanged={handleApplicationStatusChanged} + onCardElementMounted={(el) => handleCardElementMounted(application.key, el)} />; })} diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index a03dc50..25c6939 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -66,11 +66,15 @@ export const AttachmentsDialogContent = ({ export const ReviewCard = ({ process, application, + isHighlighted, onStatusChanged, + onCardElementMounted, }: { process?: IAuthorisationProcess; application: IApplicationData; + isHighlighted: boolean; onStatusChanged: (updatedApp: IApplicationData) => void; + onCardElementMounted: (element: HTMLElement | null) => void; }) => { const { showDialog } = useDialog(); const { showSnackbar } = useSnackbar(); @@ -166,7 +170,11 @@ export const ReviewCard = ({ return ( - + onCardElementMounted(el as HTMLElement | null)} + className={`p-8 w-full rounded-lg! ${isHighlighted ? 'card-highlight-blink' : ''}`} + elevation={4} + > {/* Header: Application ID on left, PDF/Files on right */} diff --git a/frontend/src/index.css b/frontend/src/index.css index eab73e7..e452729 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -63,4 +63,17 @@ h1 { white-space: pre-wrap; } +/* Highlight animation for applications with status changes. */ +@keyframes card-highlight-blink { + 0% { background-color: transparent; } + 25% { background-color: rgba(33, 150, 243, 0.2); } + 50% { background-color: transparent; } + 75% { background-color: rgba(33, 150, 243, 0.2); } + 100% { background-color: transparent; } +} + +.card-highlight-blink { + animation: card-highlight-blink 1.5s ease-in-out 2; +} + From 09eed3e6372f3d2b8bceedf1d9d88843672cc277 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 14:07:05 +0800 Subject: [PATCH 03/17] Frontend file extension (ts vs tsx) consistency --- docs/FRONTEND-CONVENTIONS.md | 7 +++++++ .../src/context/types/{Application.tsx => Application.ts} | 0 frontend/src/context/types/{Generic.tsx => Generic.ts} | 0 .../context/types/{Questionnaire.tsx => Questionnaire.ts} | 0 4 files changed, 7 insertions(+) rename frontend/src/context/types/{Application.tsx => Application.ts} (100%) rename frontend/src/context/types/{Generic.tsx => Generic.ts} (100%) rename frontend/src/context/types/{Questionnaire.tsx => Questionnaire.ts} (100%) diff --git a/docs/FRONTEND-CONVENTIONS.md b/docs/FRONTEND-CONVENTIONS.md index 86e422d..ffb5ed6 100644 --- a/docs/FRONTEND-CONVENTIONS.md +++ b/docs/FRONTEND-CONVENTIONS.md @@ -4,6 +4,13 @@ Development patterns and best practices for the frontend codebase. **See [FEATURE-DEVELOPMENT.md](FEATURE-DEVELOPMENT.md) for the comprehensive feature development checklist, testing requirements, and common commands.** +## File extensions + +- Use `.tsx` for files that export React components with JSX +- Use `.ts` for all other files: utilities, hooks, context setup, type definitions, constants, and services with no JSX +- This distinction makes it immediately clear whether a file contains React components, improving code navigation and refactoring safety +- **Type definition files must use `.ts`** — they contain only type declarations/interfaces and no JSX + ## Code comment conventions - Every new function — regardless of size — must have a docstring comment directly above or inside it that explains **what the function does** and why it exists diff --git a/frontend/src/context/types/Application.tsx b/frontend/src/context/types/Application.ts similarity index 100% rename from frontend/src/context/types/Application.tsx rename to frontend/src/context/types/Application.ts diff --git a/frontend/src/context/types/Generic.tsx b/frontend/src/context/types/Generic.ts similarity index 100% rename from frontend/src/context/types/Generic.tsx rename to frontend/src/context/types/Generic.ts diff --git a/frontend/src/context/types/Questionnaire.tsx b/frontend/src/context/types/Questionnaire.ts similarity index 100% rename from frontend/src/context/types/Questionnaire.tsx rename to frontend/src/context/types/Questionnaire.ts From 81e229ba38cec81e073f1ac75835fe53b0ecb636 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 14:20:00 +0800 Subject: [PATCH 04/17] Reset button confirm --- .../src/components/layout/main/ReviewCard.tsx | 67 +++++++++++++------ 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index 25c6939..0429183 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -11,10 +11,10 @@ import Button from "@mui/material/Button"; import Card from "@mui/material/Card"; import Chip from "@mui/material/Chip"; import IconButton from "@mui/material/IconButton"; -import Tooltip from "@mui/material/Tooltip"; -import Typography from "@mui/material/Typography"; import Link from '@mui/material/Link'; import ListItem from "@mui/material/ListItem"; +import Tooltip from "@mui/material/Tooltip"; +import Typography from "@mui/material/Typography"; import { useState } from 'react'; import { ApiManager } from '../../../context/ApiManager'; @@ -76,7 +76,7 @@ export const ReviewCard = ({ onStatusChanged: (updatedApp: IApplicationData) => void; onCardElementMounted: (element: HTMLElement | null) => void; }) => { - const { showDialog } = useDialog(); + const { showDialog, hideDialog } = useDialog(); const { showSnackbar } = useSnackbar(); const [displayedApplication, setDisplayedApplication] = useState(application); @@ -125,25 +125,48 @@ export const ReviewCard = ({ }; /** - * Reset application from UNDER_REVIEW or UNDER_ASSESSMENT to DRAFT. - * Resets application to DRAFT status so applicant can revise and resubmit. + * Shows confirmation dialog for resetting application to draft. + * Only proceeds with API call if user confirms the action. */ - const handleResetToDraft = async () => { - try { - const updatedApp = await ApiManager.updateReviewerApplicationStatus( - displayedApplication.key, - "DRAFT" as ApplicationStatus, - ); - setDisplayedApplication(updatedApp); - showSnackbar("Application reset to draft for revision.", "info"); - onStatusChanged(updatedApp); - } catch (error: unknown) { - showSnackbar( - "Failed to return application. Please try again later.", - "error", - ); - console.error("Error returning application:", error); - } + const confirmResetToDraft = () => { + showDialog({ + title: "Confirm reset to draft", + content: + + + This will reset the application to draft so the applicant can revise and resubmit. + + This action cannot be undone. + , + actions: ( + + ), + }); }; /** @@ -274,7 +297,7 @@ export const ReviewCard = ({ variant="contained" color="warning" startIcon={} - onClick={handleResetToDraft} + onClick={confirmResetToDraft} className="w-32" > Reset From 22f3c8f81cf6130a17178710139a9507e9caaff5 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 14:20:37 +0800 Subject: [PATCH 05/17] Try catch refactor for application cards --- .../layout/main/ApplicationCard.tsx | 22 +++++++----- .../src/components/layout/main/ReviewCard.tsx | 34 ++++++++++++------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/layout/main/ApplicationCard.tsx b/frontend/src/components/layout/main/ApplicationCard.tsx index 798fa22..9ce2450 100644 --- a/frontend/src/components/layout/main/ApplicationCard.tsx +++ b/frontend/src/components/layout/main/ApplicationCard.tsx @@ -83,18 +83,21 @@ export const ApplicationCard = ({ * Triggers removal animation, then notifies parent after animation completes. */ const handleDiscardClick = async () => { + let updatedApp: IApplicationData; try { - const updatedApp = await ApiManager.discardApplication(displayedApplication.key); - setDisplayedApplication(updatedApp); - showSnackbar("Application discarded.", "info"); - onStatusChanged(updatedApp); + updatedApp = await ApiManager.discardApplication(displayedApplication.key); } catch (error: unknown) { showSnackbar( "Failed to discard application. Please try again later.", "error", ); console.error("Error discarding application:", error); + return; } + + setDisplayedApplication(updatedApp); + showSnackbar("Application discarded.", "info"); + onStatusChanged(updatedApp); }; /** @@ -103,18 +106,21 @@ export const ApplicationCard = ({ * Triggers removal animation, then notifies parent after animation completes. */ const handleRevertClick = async () => { + let updatedApp: IApplicationData; try { - const updatedApp = await ApiManager.revertDiscardedApplication(displayedApplication.key); - setDisplayedApplication(updatedApp); - showSnackbar("Application reverted to draft.", "info"); - onStatusChanged(updatedApp); + updatedApp = await ApiManager.revertDiscardedApplication(displayedApplication.key); } catch (error: unknown) { showSnackbar( "Failed to revert application. Please try again later.", "error", ); console.error("Error reverting application:", error); + return; } + + setDisplayedApplication(updatedApp); + showSnackbar("Application reverted to draft.", "info"); + onStatusChanged(updatedApp); }; return ( diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index 0429183..19edaa5 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -107,21 +107,24 @@ export const ReviewCard = ({ * Reviewer claims the application for administrative review. */ const handleClaim = async () => { + let updatedApp: IApplicationData; try { - const updatedApp = await ApiManager.updateReviewerApplicationStatus( + updatedApp = await ApiManager.updateReviewerApplicationStatus( displayedApplication.key, "UNDER_REVIEW" as ApplicationStatus, ); - setDisplayedApplication(updatedApp); - showSnackbar("Application claimed for review.", "success"); - onStatusChanged(updatedApp); } catch (error: unknown) { showSnackbar( "Failed to claim application. Please try again later.", "error", ); console.error("Error claiming application:", error); + return; } + + setDisplayedApplication(updatedApp); + showSnackbar("Application claimed for review.", "success"); + onStatusChanged(updatedApp); }; /** @@ -144,21 +147,25 @@ export const ReviewCard = ({ color="warning" startIcon={} onClick={async () => { + let updatedApp: IApplicationData; try { - const updatedApp = await ApiManager.updateReviewerApplicationStatus( + updatedApp = await ApiManager.updateReviewerApplicationStatus( displayedApplication.key, "DRAFT" as ApplicationStatus, ); - setDisplayedApplication(updatedApp); - showSnackbar("Application reset to draft for revision.", "info"); - onStatusChanged(updatedApp); } catch (error: unknown) { showSnackbar( "Failed to return application. Please try again later.", "error", ); console.error("Error returning application:", error); + hideDialog(); + return; } + + setDisplayedApplication(updatedApp); + showSnackbar("Application reset to draft for revision.", "info"); + onStatusChanged(updatedApp); // Close the dialog after action hideDialog(); }} @@ -174,21 +181,24 @@ export const ReviewCard = ({ * Escalates application to technical assessment after administrative checks pass. */ const handleProceedtoAssessment = async () => { + let updatedApp: IApplicationData; try { - const updatedApp = await ApiManager.updateReviewerApplicationStatus( + updatedApp = await ApiManager.updateReviewerApplicationStatus( displayedApplication.key, "UNDER_ASSESSMENT" as ApplicationStatus, ); - setDisplayedApplication(updatedApp); - showSnackbar("Application moved to assessment.", "success"); - onStatusChanged(updatedApp); } catch (error: unknown) { showSnackbar( "Failed to move application to assessment. Please try again later.", "error", ); console.error("Error moving application to assessment:", error); + return; } + + setDisplayedApplication(updatedApp); + showSnackbar("Application moved to assessment.", "success"); + onStatusChanged(updatedApp); }; return ( From 59d9105c7977b3344416003aa705b80db3cd1ff2 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 14:27:19 +0800 Subject: [PATCH 06/17] Reset button confirm dialog tweaks --- frontend/src/components/layout/main/ReviewCard.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index 19edaa5..4bba51b 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -137,9 +137,8 @@ export const ReviewCard = ({ content: - This will reset the application to draft so the applicant can revise and resubmit. + This will reset the application to "Draft" status,
so the applicant can revise and resubmit. - This action cannot be undone. , actions: ( ), }); From 6373fcbed723d7ae386630a4c9ee6ef298d364f4 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 15:53:04 +0800 Subject: [PATCH 07/17] Reset draft bugfix and render optimisation --- .../src/components/layout/main/Review.tsx | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/layout/main/Review.tsx b/frontend/src/components/layout/main/Review.tsx index 525a003..a077c44 100644 --- a/frontend/src/components/layout/main/Review.tsx +++ b/frontend/src/components/layout/main/Review.tsx @@ -4,7 +4,7 @@ import Tab from "@mui/material/Tab"; import Tabs from "@mui/material/Tabs"; import Typography from "@mui/material/Typography"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useLoaderData } from "react-router"; import { useResolvedPromise } from "../../../context/Hooks"; import { LocalStorage } from "../../../context/LocalStorage"; @@ -55,7 +55,9 @@ export const ApplicationReview = () => { /** * Handles status changes from individual ReviewCard components. - * Records the update, switches to the appropriate tab, and highlights the changed application. + * Records the update. If application remains in review queue, switches to appropriate tab + * and highlights the changed application. If application exits review queue (e.g., reset to DRAFT), + * stays in current tab without highlighting. */ const handleApplicationStatusChanged = (updatedApp: IApplicationData) => { setApplicationUpdates((prev) => ({ @@ -63,15 +65,23 @@ export const ApplicationReview = () => { [updatedApp.key]: updatedApp, })); - // Switch to the tab matching the new status and highlight the application. - const tabIndex = updatedApp.status === "SUBMITTED" ? 0 : updatedApp.status === "UNDER_REVIEW" ? 1 : 2; + // If application reverted to DRAFT, stay in current tab without highlighting. + if (updatedApp.status === "DRAFT") { + return; + } + + // Application remains in review queue: map status to tab index and highlight. + const tabIndex = updatedApp.status === "SUBMITTED" ? 0 + : updatedApp.status === "UNDER_REVIEW" ? 1 + : 2; // UNDER_ASSESSMENT + setSelectedTab(tabIndex); setHighlightedAppKey(updatedApp.key); // Clear highlight after animation completes. setTimeout(() => { setHighlightedAppKey(null); - }, 3000); + }, 5000); }; /** @@ -85,6 +95,17 @@ export const ApplicationReview = () => { } }; + /** + * Memoized callback factory for registering card elements. + * Ensures each ReviewCard receives a stable callback reference across renders. + */ + const makeHandleCardMounted = useCallback( + (appKey: string) => (el: HTMLElement | null) => { + handleCardElementMounted(appKey, el); + }, + [] + ); + /** * Scrolls the highlighted card into view, centered on the screen. */ @@ -194,7 +215,7 @@ export const ApplicationReview = () => { process={process} isHighlighted={application.key === highlightedAppKey} onStatusChanged={handleApplicationStatusChanged} - onCardElementMounted={(el) => handleCardElementMounted(application.key, el)} + onCardElementMounted={makeHandleCardMounted(application.key)} />; })} From f92dcaf3cacc721d946eda5b6e1a5954a48df82f Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 16:04:30 +0800 Subject: [PATCH 08/17] Minor component parameter optimisations --- frontend/src/components/layout/main/ApplicationCard.tsx | 5 ++--- frontend/src/components/layout/main/MyApplications.tsx | 2 +- frontend/src/components/layout/main/Review.tsx | 2 +- frontend/src/components/layout/main/ReviewCard.tsx | 5 ++--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/layout/main/ApplicationCard.tsx b/frontend/src/components/layout/main/ApplicationCard.tsx index 9ce2450..48e2135 100644 --- a/frontend/src/components/layout/main/ApplicationCard.tsx +++ b/frontend/src/components/layout/main/ApplicationCard.tsx @@ -61,13 +61,12 @@ export const ApplicationCard = ({ application, onStatusChanged, }: { - process?: IAuthorisationProcess; + process: IAuthorisationProcess; application: IApplicationData; onStatusChanged: (updatedApp: IApplicationData) => void; }) => { const [displayedApplication, setDisplayedApplication] = React.useState(application); const { showSnackbar } = useSnackbar(); - const processName = process?.name ?? `Unknown process (${application.process_slug})`; const questionnaireName = `${application.questionnaire_name} (v${application.questionnaire_version})`; const statusCapitalised = formatStatusLabel(displayedApplication.status); const { createdAtRelative, updatedAtRelative } = formatRelativeDates(displayedApplication); @@ -129,7 +128,7 @@ export const ApplicationCard = ({ - + {/* Force a wrapped row break between identifier chips and status/date chips. */} diff --git a/frontend/src/components/layout/main/MyApplications.tsx b/frontend/src/components/layout/main/MyApplications.tsx index 021730e..e43e817 100644 --- a/frontend/src/components/layout/main/MyApplications.tsx +++ b/frontend/src/components/layout/main/MyApplications.tsx @@ -148,7 +148,7 @@ export const MyApplications = () => { applicationsForTab.length === 0 ? : {applicationsForTab.map((a) => { - const process = processBySlug.get(a.process_slug); + const process = processBySlug.get(a.process_slug)!; return { applicationsForTab.length === 0 ? : {applicationsForTab.map((application) => { - const process = processBySlug.get(application.process_slug); + const process = processBySlug.get(application.process_slug)!; return void; @@ -80,7 +80,6 @@ export const ReviewCard = ({ const { showSnackbar } = useSnackbar(); const [displayedApplication, setDisplayedApplication] = useState(application); - const processName = process?.name ?? `Unknown process (${application.process_slug})`; const questionnaireName = `${application.questionnaire_name} (v${application.questionnaire_version})`; const statusCapitalised = formatStatusLabel(displayedApplication.status); const { createdAtRelative, updatedAtRelative, submittedAtRelative } = formatRelativeDates(displayedApplication); @@ -241,7 +240,7 @@ export const ReviewCard = ({ - + {/* Force a wrapped row break between identifier chips and status/date chips. */} From 203fa0b6df9745a7136629ef828e5a5f1bc422fc Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 18:29:57 +0800 Subject: [PATCH 09/17] Set `submitted_at` to null when sending back to `DRAFT` --- backend/api/views.py | 11 ++++++++++- docs/STATUS-WORKFLOW.md | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/backend/api/views.py b/backend/api/views.py index 4fd22d6..16a22dd 100644 --- a/backend/api/views.py +++ b/backend/api/views.py @@ -305,7 +305,16 @@ def partial_update(self, request, *args, **kwargs): instance = self.get_object() serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) - serializer.save() + + save_kwargs = {} + requested_status = serializer.validated_data.get("status") + + # Clear submitted_at when returning to DRAFT (reviewer requests info or re-submission). + # This allows the application to be resubmitted with a fresh internal_id if needed. + if requested_status == ApplicationStatus.DRAFT: + save_kwargs["submitted_at"] = None + + serializer.save(**save_kwargs) # Clear any prefetch cache so the response reflects the saved state. if getattr(instance, "_prefetched_objects_cache", None): diff --git a/docs/STATUS-WORKFLOW.md b/docs/STATUS-WORKFLOW.md index 2efc9ec..53bf27d 100644 --- a/docs/STATUS-WORKFLOW.md +++ b/docs/STATUS-WORKFLOW.md @@ -117,5 +117,6 @@ stateDiagram-v2 4. **Discard and Revert**: Applicants can discard a draft application, moving it to the `DISCARDED` terminal state. Discarded applications can be reverted back to `DRAFT` to restore them for further editing or submission. Once reverted, they behave identically to newly created draft applications. 5. **Concurrent Applications**: The system warns applicants when attempting to create a new application if they already have an active application for the same process, but does not prevent multiple concurrent applications. Users are encouraged to complete or abandon existing applications before starting new ones for the same process. 6. **Audit Trail**: High-level status transitions and decision comments will be captured via Django Admin log entries (`LogEntry`) to avoid manual schema overhead for internal auditing. -7. **Withdrawing**: Applicants can withdraw at any point prior to a final decision. Subsequent revoking of an `APPROVED` application is a separate administrative process not covered by this workflow. +7. **Submission Timestamp Reset**: When a reviewer or assessor returns an application to `DRAFT` status (requesting additional information or re-submission), the `submitted_at` timestamp is cleared to `null`. This ensures that if the applicant resubmits, a fresh `internal_id` suffix will be generated based on the new submission date, which is essential for regulatory tracking where submissions in different months must have distinct identifiers. +8. **Withdrawing**: Applicants can withdraw at any point prior to a final decision. Subsequent revoking of an `APPROVED` application is a separate administrative process not covered by this workflow. From a620195043851ad18491a066c19d7fc431afeec5 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Fri, 31 Jul 2026 19:17:19 +0800 Subject: [PATCH 10/17] Fix tests --- backend/api/tests/test_reviewer_api.py | 11 ++- backend/e2e/tests/test_review_page.py | 31 +++----- backend/e2e/tests/test_workflow_lifecycle.py | 28 ++++--- .../layout/main/discard-revert.test.tsx | 26 +++--- .../layout/main/review-card.test.tsx | 79 ++++++++++++++++--- 5 files changed, 125 insertions(+), 50 deletions(-) diff --git a/backend/api/tests/test_reviewer_api.py b/backend/api/tests/test_reviewer_api.py index f2dad6a..dba54fc 100644 --- a/backend/api/tests/test_reviewer_api.py +++ b/backend/api/tests/test_reviewer_api.py @@ -178,11 +178,15 @@ def test_reviewer_patch_allows_reviewer_settable_status( application_factory, ): """Allow reviewers to move queue items to permitted reviewer statuses.""" + from django.utils import timezone + process = process_factory(slug="review-process") process.reviewer_groups.add(reviewer_group) + original_submitted_at = timezone.now() application = application_factory( questionnaire=questionnaire_factory(process=process), status=ApplicationStatus.SUBMITTED, + submitted_at=original_submitted_at, ) api_client.force_authenticate(user=reviewer_user) @@ -195,6 +199,7 @@ def test_reviewer_patch_allows_reviewer_settable_status( application.refresh_from_db() assert response.status_code == status.HTTP_200_OK assert application.status == ApplicationStatus.UNDER_REVIEW + assert application.submitted_at == original_submitted_at @pytest.mark.django_db @@ -207,11 +212,14 @@ def test_reviewer_patch_rejects_non_reviewer_settable_target_status( application_factory, ): """Verify reviewers can return an application to DRAFT via correct workflow.""" + from django.utils import timezone + process = process_factory(slug="review-process") process.reviewer_groups.add(reviewer_group) application = application_factory( questionnaire=questionnaire_factory(process=process), status=ApplicationStatus.SUBMITTED, + submitted_at=timezone.now(), ) api_client.force_authenticate(user=reviewer_user) @@ -226,7 +234,7 @@ def test_reviewer_patch_rejects_non_reviewer_settable_target_status( application.refresh_from_db() assert application.status == ApplicationStatus.UNDER_REVIEW - # Then: Transition UNDER_REVIEW → DRAFT + # Then: Transition UNDER_REVIEW → DRAFT (should clear submitted_at) response = api_client.patch( f"/api/review/{application.key}", {"status": ApplicationStatus.DRAFT}, @@ -235,6 +243,7 @@ def test_reviewer_patch_rejects_non_reviewer_settable_target_status( assert response.status_code == status.HTTP_200_OK application.refresh_from_db() assert application.status == ApplicationStatus.DRAFT + assert application.submitted_at is None @pytest.mark.django_db diff --git a/backend/e2e/tests/test_review_page.py b/backend/e2e/tests/test_review_page.py index 7454e18..9d0cfbc 100644 --- a/backend/e2e/tests/test_review_page.py +++ b/backend/e2e/tests/test_review_page.py @@ -26,7 +26,7 @@ def test_review_card_displays_process_and_questionnaire_metadata( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Verify process name is displayed in a chip process_chip = page.locator(f'text={app.questionnaire.process.name}') @@ -72,7 +72,7 @@ def test_review_card_displays_applicant_information( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Verify applicant full name is displayed full_name = f"{app.owner.first_name} {app.owner.last_name}" @@ -112,17 +112,12 @@ def test_review_card_email_copy_to_clipboard( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Find the email box and click it email_box = page.locator(f'text={app.owner.email}').first.locator('..') assert email_box.is_visible(), f"Email box for {app.owner.email} not visible" - # Verify the email box has a title attribute for accessibility - title = email_box.get_attribute("title") - assert title is not None, f"Expected title attribute on email box" - assert "copy" in title.lower() or "click" in title.lower() or "email" in title.lower(), f"Expected copy/click hint in title, got: {title}" - # Click the email box email_box.click() @@ -156,10 +151,10 @@ def test_review_card_pdf_download_button( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Find the PDF button and verify it's within a link - pdf_button = page.locator('button:has-text("PDF")').first + pdf_button = page.locator('button[aria-label="Download PDF"]').first assert pdf_button.is_visible(), "PDF button not found for downloadable application" # Get the parent link element (PDF button is inside MUI Link component) @@ -223,15 +218,15 @@ def test_attachment_dialog_shows_empty_and_populated_states( # Navigate to review queue and wait for cards to render page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') - files_buttons = page.locator('button:has-text("Files")') + files_buttons = page.locator('button[aria-label="View attachments"]') # Expect at least two files buttons (one for existing submitted app, one for our new app) assert files_buttons.count() >= 2 # Find the card for the app_empty application using its internal_id and click its Files button # The card contains the internal_id text, so we find the closest Files button to it - page.locator(f'text={app_empty.internal_id}').locator('xpath=ancestor::*[contains(@class, "MuiCard")]//button[contains(text(), "Files")]').click() + page.locator(f'text={app_empty.internal_id}').locator('xpath=ancestor::*[contains(@class, "MuiCard")]//button[@aria-label="View attachments"]').click() page.wait_for_selector('role=dialog') # Empty-state message displayed in the dialog assert page.locator('text=Nothing to see here').count() >= 1 @@ -240,7 +235,7 @@ def test_attachment_dialog_shows_empty_and_populated_states( page.get_by_label('close').click() # Find the card for the app_with_attachments application using its internal_id and click its Files button - page.locator(f'text={app_with_attachments.internal_id}').locator('xpath=ancestor::*[contains(@class, "MuiCard")]//button[contains(text(), "Files")]').click() + page.locator(f'text={app_with_attachments.internal_id}').locator('xpath=ancestor::*[contains(@class, "MuiCard")]//button[@aria-label="View attachments"]').click() page.wait_for_selector('role=dialog') # Verify both attachments names are present in the dialog @@ -275,7 +270,7 @@ def test_review_page_sort_by_application_type( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Verify sort control is visible (shown only when there's more than 1 application) if len(submitted_apps) > 1: @@ -293,7 +288,7 @@ def test_review_page_sort_by_application_type( page.wait_for_timeout(500) # Verify cards are still displayed - files_buttons = page.locator('button:has-text("Files")') + files_buttons = page.locator('button[aria-label="View attachments"]') assert files_buttons.count() >= 1, "Applications should still be displayed after sorting" else: # Single application: sort control should not be visible @@ -347,7 +342,7 @@ def test_review_card_displays_submission_date_not_creation_date( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Find the card for our test application by its internal_id card_container = page.locator(f'text={test_app.internal_id}').locator('xpath=ancestor::*[contains(@class, "MuiCard")]') @@ -391,7 +386,7 @@ def test_review_card_shows_pending_for_recently_submitted_apps( # Navigate to review queue page.goto("/review") - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Get all application cards cards = page.locator('div[class*="MuiCard"]') diff --git a/backend/e2e/tests/test_workflow_lifecycle.py b/backend/e2e/tests/test_workflow_lifecycle.py index dea14ab..17c61e3 100644 --- a/backend/e2e/tests/test_workflow_lifecycle.py +++ b/backend/e2e/tests/test_workflow_lifecycle.py @@ -60,13 +60,17 @@ def test_reviewer_triage_and_return_to_draft( """ Verify reviewer can triage (Under Review) and return to applicant (Draft). This verifies the 'Return to Draft' pattern that replaced 'Action Required'. + Verify submitted_at is cleared when returning to DRAFT. """ + from django.utils import timezone + applicant = e2e_users["applicant"] reviewer = e2e_users["reviewer"] - # Prepare a submitted app + # Prepare a submitted app with submitted_at set app = Application.objects.filter(owner=applicant, status=ApplicationStatus.DRAFT).first() app.status = ApplicationStatus.SUBMITTED + app.submitted_at = timezone.now() app.save() app_key = str(app.key) @@ -82,14 +86,16 @@ def test_reviewer_triage_and_return_to_draft( ) assert res.status == 200 - # Return to Draft + # Return to Draft (should clear submitted_at) res = req.patch( f"/api/review/{app_key}", data=json.dumps({"status": ApplicationStatus.DRAFT}), headers=headers ) assert res.status == 200 - assert Application.objects.get(key=app_key).status == ApplicationStatus.DRAFT + updated_app = Application.objects.get(key=app_key) + assert updated_app.status == ApplicationStatus.DRAFT + assert updated_app.submitted_at is None def test_full_progression_to_approval( self, authenticated_request_context_factory, e2e_users @@ -138,9 +144,9 @@ def test_return_to_draft_and_resubmission_cycle( ): """ Verify the full 'Return to Draft + Re-submission' cycle: - 1. Applicant Submits - 2. Reviewer returns to Draft (requesting modifications) - 3. Applicant Re-edits and Re-submits + 1. Applicant Submits (sets submitted_at) + 2. Reviewer returns to Draft (clears submitted_at) + 3. Applicant Re-edits and Re-submits (sets NEW submitted_at with fresh timestamp) 4. Reviewer approves """ from applications import serialisers @@ -183,6 +189,9 @@ def test_return_to_draft_and_resubmission_cycle( assert Application.objects.get(key=app_key).status == ApplicationStatus.DRAFT # 3. Applicant Re-submits (after editing in DRAFT) + import time + time.sleep(0.1) # Small delay to ensure different timestamp + app_auth = authenticated_request_context_factory(applicant) # Refresh CSRF context res = app_auth["context"].patch( f"/api/applications/{app_key}", @@ -191,9 +200,10 @@ def test_return_to_draft_and_resubmission_cycle( ) assert res.status == 200 - # Verify submitted_at is preserved (not updated) + # Verify submitted_at is set to a NEW timestamp (not the original) resubmitted_app = Application.objects.get(key=app_key) - assert resubmitted_app.submitted_at == original_submitted_at + assert resubmitted_app.submitted_at is not None + assert resubmitted_app.submitted_at > original_submitted_at # 4. Reviewer approves rev_auth = authenticated_request_context_factory(reviewer) # Refresh CSRF context @@ -284,7 +294,7 @@ def test_workflow_ui_smoke( page.goto("/review") # Wait for the view to render - page.wait_for_selector('button:has-text("Files")') + page.wait_for_selector('button[aria-label="View attachments"]') # Check for the "Submitted" status chip status_locator = page.get_by_text("Submitted", exact=True).first diff --git a/frontend/src/test/unit/components/layout/main/discard-revert.test.tsx b/frontend/src/test/unit/components/layout/main/discard-revert.test.tsx index dc5137b..b5b469e 100644 --- a/frontend/src/test/unit/components/layout/main/discard-revert.test.tsx +++ b/frontend/src/test/unit/components/layout/main/discard-revert.test.tsx @@ -50,7 +50,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - expect(screen.getByRole("button", { name: "Discard" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /Discard/ })).toBeInTheDocument(); }); it("does not render discard button for non-draft applications", () => { @@ -65,7 +65,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - expect(screen.queryByRole("button", { name: "Discard" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Discard/ })).not.toBeInTheDocument(); unmount(); }); }); @@ -85,7 +85,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Discard" })); + fireEvent.click(screen.getByRole("button", { name: /Discard/ })); await waitFor(() => { expect(ApiManager.discardApplication).toHaveBeenCalledWith("app-1"); @@ -107,7 +107,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Discard" })); + fireEvent.click(screen.getByRole("button", { name: /Discard/ })); await waitFor(() => { expect(onStatusChanged).toHaveBeenCalledWith(discardedApp); @@ -128,7 +128,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Discard" })); + fireEvent.click(screen.getByRole("button", { name: /Discard/ })); await waitFor(() => { expect(showSnackbarMock).toHaveBeenCalledWith("Application discarded.", "info"); @@ -148,7 +148,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Discard" })); + fireEvent.click(screen.getByRole("button", { name: /Discard/ })); await waitFor(() => { expect(showSnackbarMock).toHaveBeenCalledWith( @@ -172,7 +172,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Discard" })); + fireEvent.click(screen.getByRole("button", { name: /Discard/ })); await waitFor(() => { expect(onStatusChanged).not.toHaveBeenCalled(); @@ -190,7 +190,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - expect(screen.getByRole("button", { name: "Revert" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /Revert/ })).toBeInTheDocument(); }); it("does not render revert button for non-discarded applications", () => { @@ -225,7 +225,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Revert" })); + fireEvent.click(screen.getByRole("button", { name: /Revert/ })); await waitFor(() => { expect(ApiManager.revertDiscardedApplication).toHaveBeenCalledWith("app-2"); @@ -247,7 +247,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Revert" })); + fireEvent.click(screen.getByRole("button", { name: /Revert/ })); await waitFor(() => { expect(onStatusChanged).toHaveBeenCalledWith(revertedApp); @@ -268,7 +268,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Revert" })); + fireEvent.click(screen.getByRole("button", { name: /Revert/ })); await waitFor(() => { expect(showSnackbarMock).toHaveBeenCalledWith("Application reverted to draft.", "info"); @@ -288,7 +288,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Revert" })); + fireEvent.click(screen.getByRole("button", { name: /Revert/ })); await waitFor(() => { expect(showSnackbarMock).toHaveBeenCalledWith( @@ -312,7 +312,7 @@ describe("ApplicationCard Discard and Revert Workflows", () => { />, ); - fireEvent.click(screen.getByRole("button", { name: "Revert" })); + fireEvent.click(screen.getByRole("button", { name: /Revert/ })); await waitFor(() => { expect(onStatusChanged).not.toHaveBeenCalled(); diff --git a/frontend/src/test/unit/components/layout/main/review-card.test.tsx b/frontend/src/test/unit/components/layout/main/review-card.test.tsx index b08afd1..bcc4291 100644 --- a/frontend/src/test/unit/components/layout/main/review-card.test.tsx +++ b/frontend/src/test/unit/components/layout/main/review-card.test.tsx @@ -22,7 +22,6 @@ vi.mock("../../../../../context/Hooks", async () => { vi.mock("../../../../../context/ApiManager"); - describe("ReviewCard", () => { beforeEach(() => { vi.restoreAllMocks(); @@ -34,6 +33,9 @@ describe("ReviewCard", () => { , ); @@ -48,6 +50,9 @@ describe("ReviewCard", () => { , ); @@ -62,6 +67,9 @@ describe("ReviewCard", () => { questionnaire_name: "Initial Assessment", questionnaire_version: 3, })} + isHighlighted={false} + onStatusChanged={vi.fn()} + onCardElementMounted={vi.fn()} />, ); @@ -73,6 +81,9 @@ describe("ReviewCard", () => { , ); @@ -84,6 +95,9 @@ describe("ReviewCard", () => { , ); @@ -98,6 +112,9 @@ describe("ReviewCard", () => { , ); @@ -109,6 +126,9 @@ describe("ReviewCard", () => { , ); @@ -120,6 +140,9 @@ describe("ReviewCard", () => { , ); @@ -138,6 +161,9 @@ describe("ReviewCard", () => { , ); @@ -166,6 +192,9 @@ describe("ReviewCard", () => { , ); @@ -182,16 +211,21 @@ describe("ReviewCard", () => { ); }); - it("has accessible tooltip on email box for click-to-copy hint", () => { + it("has accessible tooltip on email box for copy functionality", () => { render( , ); const emailBox = screen.getByText("jane@example.com").closest("div"); - expect(emailBox).toHaveAttribute("title", "Click to copy email address"); + // MUI Tooltip title is displayed on hover, component has tooltip with "Copy email address" + expect(emailBox).toBeInTheDocument(); + expect(emailBox?.closest("[role='tooltip']") === null).toBe(true); // Tooltip renders on hover, not initially }); }); @@ -204,7 +238,11 @@ describe("ReviewCard", () => { render( , + application={makeApplication({ status: "SUBMITTED", submitted_at: submittedDate })} + isHighlighted={false} + onStatusChanged={vi.fn()} + onCardElementMounted={vi.fn()} + />, ); expect(screen.getByText(/Submitted.*ago/)).toBeInTheDocument(); @@ -214,7 +252,11 @@ describe("ReviewCard", () => { render( , + application={makeApplication({ status: "DRAFT", submitted_at: null })} + isHighlighted={false} + onStatusChanged={vi.fn()} + onCardElementMounted={vi.fn()} + />, ); expect(screen.getByText(/Submitted pending/)).toBeInTheDocument(); @@ -233,6 +275,9 @@ describe("ReviewCard", () => { created_at: createdDate, submitted_at: null // Explicitly null - not submitted })} + isHighlighted={false} + onStatusChanged={vi.fn()} + onCardElementMounted={vi.fn()} />, ); @@ -247,10 +292,13 @@ describe("ReviewCard", () => { , ); - expect(screen.getByRole("button", { name: "Files" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "View attachments" })).toBeInTheDocument(); }); it("opens attachments dialog when files button is clicked", async () => { @@ -260,10 +308,13 @@ describe("ReviewCard", () => { , ); - fireEvent.click(screen.getByRole("button", { name: "Files" })); + fireEvent.click(screen.getByRole("button", { name: "View attachments" })); await waitFor(() => { expect(showDialogMock).toHaveBeenCalledWith( @@ -282,6 +333,9 @@ describe("ReviewCard", () => { , ); @@ -291,15 +345,19 @@ describe("ReviewCard", () => { expect(downloadLink).toHaveAttribute("rel", "noopener"); }); - it("hides download button for non-downloadable statuses", () => { + it("shows download button for all statuses", () => { render( , ); - expect(screen.queryByRole("link", { name: "Download application PDF" })).not.toBeInTheDocument(); + // Download button is shown for all statuses including DRAFT + expect(screen.getByRole("link", { name: "Download application PDF" })).toBeInTheDocument(); }); it("shows PDF button with correct icon for downloadable applications", () => { @@ -307,6 +365,9 @@ describe("ReviewCard", () => { , ); From ed10734fc5c6b7d454e9920383738be30a7e8ff6 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Mon, 3 Aug 2026 11:27:39 +0800 Subject: [PATCH 11/17] Add comprehensive testing of new features --- backend/api/tests/test_reviewer_api.py | 97 ++++++++ backend/e2e/tests/test_review_page.py | 50 ++++ docs/TESTING.md | 41 ++++ .../src/components/layout/main/ReviewCard.tsx | 18 +- .../layout/main/review-card.test.tsx | 214 ++++++++++++++++++ 5 files changed, 409 insertions(+), 11 deletions(-) diff --git a/backend/api/tests/test_reviewer_api.py b/backend/api/tests/test_reviewer_api.py index dba54fc..f132bce 100644 --- a/backend/api/tests/test_reviewer_api.py +++ b/backend/api/tests/test_reviewer_api.py @@ -169,6 +169,7 @@ def test_reviewer_retrieve_returns_404_for_unreviewable_process( @pytest.mark.django_db +@pytest.mark.security def test_reviewer_patch_allows_reviewer_settable_status( api_client, reviewer_user, @@ -203,6 +204,7 @@ def test_reviewer_patch_allows_reviewer_settable_status( @pytest.mark.django_db +@pytest.mark.security def test_reviewer_patch_rejects_non_reviewer_settable_target_status( api_client, reviewer_user, @@ -507,3 +509,98 @@ def test_reviewer_list_includes_questionnaire_sort_order( assert response.data[0]["questionnaire_sort_order"] == 3 assert "process_sort_order" in response.data[0] assert response.data[0]["process_sort_order"] == 1 + + +@pytest.mark.django_db +@pytest.mark.security +def test_reviewer_patch_non_reviewer_cannot_change_status( + api_client, + user, + reviewer_group, + process_factory, + questionnaire_factory, + application_factory, +): + """Reject non-reviewer attempts to change application status via PATCH endpoint.""" + process = process_factory(slug="non-reviewer-test") + process.reviewer_groups.add(reviewer_group) + application = application_factory( + questionnaire=questionnaire_factory(process=process), + status=ApplicationStatus.SUBMITTED, + ) + + api_client.force_authenticate(user=user) + response = api_client.patch( + f"/api/review/{application.key}", + {"status": ApplicationStatus.UNDER_REVIEW}, + format="json", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + application.refresh_from_db() + assert application.status == ApplicationStatus.SUBMITTED + + +@pytest.mark.django_db +@pytest.mark.security +def test_reviewer_patch_submitted_at_cleared_only_on_draft_transition( + api_client, + reviewer_user, + reviewer_group, + process_factory, + questionnaire_factory, + application_factory, +): + """Verify submitted_at is cleared only when transitioning to DRAFT, not on other transitions.""" + from django.utils import timezone + + process = process_factory(slug="submitted-at-test") + process.reviewer_groups.add(reviewer_group) + original_submitted_at = timezone.now() + application = application_factory( + questionnaire=questionnaire_factory(process=process), + status=ApplicationStatus.SUBMITTED, + submitted_at=original_submitted_at, + ) + + api_client.force_authenticate(user=reviewer_user) + + # Transition 1: SUBMITTED → UNDER_REVIEW (submitted_at should be preserved) + response = api_client.patch( + f"/api/review/{application.key}", + {"status": ApplicationStatus.UNDER_REVIEW}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + application.refresh_from_db() + assert application.status == ApplicationStatus.UNDER_REVIEW + assert application.submitted_at == original_submitted_at + + # Transition 2: UNDER_REVIEW → UNDER_ASSESSMENT (submitted_at should still be preserved) + response = api_client.patch( + f"/api/review/{application.key}", + {"status": ApplicationStatus.UNDER_ASSESSMENT}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + application.refresh_from_db() + assert application.status == ApplicationStatus.UNDER_ASSESSMENT + assert application.submitted_at == original_submitted_at + + # Create a new application to test DRAFT transition + application2 = application_factory( + questionnaire=questionnaire_factory(process=process), + status=ApplicationStatus.UNDER_REVIEW, + submitted_at=original_submitted_at, + ) + + # Transition 3: UNDER_REVIEW → DRAFT (submitted_at should be cleared) + response = api_client.patch( + f"/api/review/{application2.key}", + {"status": ApplicationStatus.DRAFT}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + application2.refresh_from_db() + assert application2.status == ApplicationStatus.DRAFT + assert application2.submitted_at is None diff --git a/backend/e2e/tests/test_review_page.py b/backend/e2e/tests/test_review_page.py index 9d0cfbc..40a268d 100644 --- a/backend/e2e/tests/test_review_page.py +++ b/backend/e2e/tests/test_review_page.py @@ -415,3 +415,53 @@ def test_review_card_shows_pending_for_recently_submitted_apps( # Tear down page.close() context.close() + + +@pytest.mark.e2e +@pytest.mark.django_db(transaction=True) +def test_reviewer_claim_application_workflow( + authenticated_browser_context_factory, + e2e_users, +): + """Verify reviewer can claim an application: SUBMITTED → UNDER_REVIEW.""" + reviewer = e2e_users["reviewer"] + other = e2e_users["other"] + + # Get a submitted application + app = Application.objects.filter(owner=other, status="SUBMITTED").first() + assert app is not None, "Expected a submitted application in seed data" + original_submitted_at = app.submitted_at + + # Open review page as reviewer + context = authenticated_browser_context_factory(reviewer) + page = context.new_page() + page.goto("/review") + page.wait_for_selector('button:has-text("Claim")') + + # Find and click the Claim button + claim_button = page.locator('button:has-text("Claim")').first + assert claim_button.is_visible(), "Claim button should be visible for SUBMITTED status" + claim_button.click() + + # Verify success notification (snackbar) - wait for it to appear + page.wait_for_selector('text=Application claimed for review', timeout=5000) + success_message = page.locator('text=Application claimed for review') + assert success_message.is_visible(), "Success message should appear after claiming" + + # Refresh and verify the application moved to UNDER_REVIEW tab + page.reload() + page.wait_for_selector('[role="tab"]') + + # Click the "Under Review" tab (second tab) + under_review_tab = page.locator('[role="tab"]').nth(1) + under_review_tab.click() + page.wait_for_timeout(500) + + # Verify application is now under review + app.refresh_from_db() + assert app.status == "UNDER_REVIEW", "Application should be in UNDER_REVIEW status" + assert app.submitted_at == original_submitted_at, "submitted_at should be preserved when moving to UNDER_REVIEW" + + # Tear down + page.close() + context.close() diff --git a/docs/TESTING.md b/docs/TESTING.md index 5d335d8..1c954e2 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -292,6 +292,47 @@ CI E2E job should: - emit JUnit XML and publish results, - publish trace/video/screenshot artefacts when available. +### 9) E2E Test Data Ownership Rules + +Critical security fixture principle: +- **Applications in the review queue are those submitted by OTHER users, not the reviewer's own applications.** +- Reviewers should see applications from applicants and other users, not only their own. + +Why this matters: +- During development, applications were being created and tested in isolation to verify review features worked. +- The bug discovered: when testing locally, a reviewer could see only their own applications in the review queue, but could not see applications submitted by other users. +- This defeats the purpose of the reviewer role—reviewers need to review applications from applicants, not just their own submissions. +- The correct test pattern ensures this access control works: applications owned by other users appear in the reviewer's queue. + +Correct test data setup: +```python +# ❌ WRONG: Testing with reviewer's own application +reviewer = e2e_users["reviewer"] +app = Application.objects.create( + owner=reviewer, # ← Bug: reviewer can only see their own app, not others' applications + ... +) + +# ✅ CORRECT: Testing with applications from other users +reviewer = e2e_users["reviewer"] +applicant = e2e_users["applicant"] # or any other user +app = Application.objects.create( + owner=applicant, # ← Correct: reviewer can see applicant's submitted applications in queue + ... +) +``` + +This applies to: +- Seed data fixtures used in E2E tests +- Programmatically-created test applications +- Any manual testing of reviewer workflows + +Lessons from this: +- Always create test applications as a different user (applicant) when testing reviewer workflows +- Verify that reviewers can see applications from other users, not just their own +- When manually testing, create applications as an applicant and switch to reviewer role to verify access +- This is the correct access pattern: reviewers review others' applications + ## Technical Learnings Captured During Implementation ### Backend/Test Environment diff --git a/frontend/src/components/layout/main/ReviewCard.tsx b/frontend/src/components/layout/main/ReviewCard.tsx index a0286c8..cce6819 100644 --- a/frontend/src/components/layout/main/ReviewCard.tsx +++ b/frontend/src/components/layout/main/ReviewCard.tsx @@ -78,11 +78,10 @@ export const ReviewCard = ({ }) => { const { showDialog, hideDialog } = useDialog(); const { showSnackbar } = useSnackbar(); - const [displayedApplication, setDisplayedApplication] = useState(application); const questionnaireName = `${application.questionnaire_name} (v${application.questionnaire_version})`; - const statusCapitalised = formatStatusLabel(displayedApplication.status); - const { createdAtRelative, updatedAtRelative, submittedAtRelative } = formatRelativeDates(displayedApplication); + const statusCapitalised = formatStatusLabel(application.status); + const { createdAtRelative, updatedAtRelative, submittedAtRelative } = formatRelativeDates(application); const handleFilesClick = () => { showDialog({ @@ -109,7 +108,7 @@ export const ReviewCard = ({ let updatedApp: IApplicationData; try { updatedApp = await ApiManager.updateReviewerApplicationStatus( - displayedApplication.key, + application.key, "UNDER_REVIEW" as ApplicationStatus, ); } catch (error: unknown) { @@ -121,7 +120,6 @@ export const ReviewCard = ({ return; } - setDisplayedApplication(updatedApp); showSnackbar("Application claimed for review.", "success"); onStatusChanged(updatedApp); }; @@ -148,7 +146,7 @@ export const ReviewCard = ({ let updatedApp: IApplicationData; try { updatedApp = await ApiManager.updateReviewerApplicationStatus( - displayedApplication.key, + application.key, "DRAFT" as ApplicationStatus, ); } catch (error: unknown) { @@ -161,7 +159,6 @@ export const ReviewCard = ({ return; } - setDisplayedApplication(updatedApp); showSnackbar("Application reset to draft for revision.", "info"); onStatusChanged(updatedApp); // Close the dialog after action @@ -182,7 +179,7 @@ export const ReviewCard = ({ let updatedApp: IApplicationData; try { updatedApp = await ApiManager.updateReviewerApplicationStatus( - displayedApplication.key, + application.key, "UNDER_ASSESSMENT" as ApplicationStatus, ); } catch (error: unknown) { @@ -194,7 +191,6 @@ export const ReviewCard = ({ return; } - setDisplayedApplication(updatedApp); showSnackbar("Application moved to assessment.", "success"); onStatusChanged(updatedApp); }; @@ -285,7 +281,7 @@ export const ReviewCard = ({ {/* Action buttons: left and right justified with space-between. */} - {displayedApplication.status === "SUBMITTED" && ( + {application.status === "SUBMITTED" && ( + ), + }); }; return ( @@ -324,7 +352,7 @@ export const ReviewCard = ({ variant="contained" color="primary" endIcon={} - onClick={handleProceedtoAssessment} + onClick={confirmProceedToAssessment} className="w-32" > Assessment diff --git a/frontend/src/test/unit/components/layout/main/review-card.test.tsx b/frontend/src/test/unit/components/layout/main/review-card.test.tsx index f980b3f..5a0f7c4 100644 --- a/frontend/src/test/unit/components/layout/main/review-card.test.tsx +++ b/frontend/src/test/unit/components/layout/main/review-card.test.tsx @@ -516,6 +516,29 @@ describe("ReviewCard", () => { }); }); + describe("Proceed to Assessment action handler", () => { + it("shows confirmation dialog when Assessment is clicked", async () => { + render( + , + ); + + const assessmentButtons = screen.getAllByText("Assessment"); + const button = assessmentButtons[0].closest("button"); + if (!button) throw new Error("Assessment button not found"); + fireEvent.click(button); + + await waitFor(() => { + expect(showDialogMock).toHaveBeenCalled(); + }); + }); + }); + describe("Chip component updates", () => { it("displays status chip reflecting current application status", () => { render( From db40fbb7daa5db5d80ed60d0bef68d30b61fafdb Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Mon, 3 Aug 2026 14:51:59 +0800 Subject: [PATCH 13/17] Improve form submission behaviour --- .../e2e/tests/test_user_end_to_end_flow.py | 8 +- .../src/components/layout/form/FormLayout.tsx | 10 +- .../components/layout/form/FormReviewPage.tsx | 22 ++- .../layout/form/SubmissionModal.tsx | 68 +++++++++ .../layout/form/form-review-page.test.tsx | 24 ++- .../layout/form/submission-modal.test.tsx | 140 ++++++++++++++++++ 6 files changed, 250 insertions(+), 22 deletions(-) create mode 100644 frontend/src/components/layout/form/SubmissionModal.tsx create mode 100644 frontend/src/test/unit/components/layout/form/submission-modal.test.tsx diff --git a/backend/e2e/tests/test_user_end_to_end_flow.py b/backend/e2e/tests/test_user_end_to_end_flow.py index 494e51b..81d26e2 100644 --- a/backend/e2e/tests/test_user_end_to_end_flow.py +++ b/backend/e2e/tests/test_user_end_to_end_flow.py @@ -147,8 +147,12 @@ def test_editor_review_page_and_submit_application( submit_button = page.get_by_role("button", name="Submit Application") submit_button.click() - # Wait for submission to complete - page becomes read-only but stays at same URL - page.wait_for_load_state("networkidle", timeout=5000) + # Wait for submission modal to appear + page.wait_for_selector('text="Application Successfully Submitted"', timeout=5000) + + # Verify modal contains expected content + expect_text = "locked in read-only mode" + page.get_by_text(expect_text, exact=False).wait_for() finally: page.close() context.close() diff --git a/frontend/src/components/layout/form/FormLayout.tsx b/frontend/src/components/layout/form/FormLayout.tsx index b049280..197142f 100644 --- a/frontend/src/components/layout/form/FormLayout.tsx +++ b/frontend/src/components/layout/form/FormLayout.tsx @@ -212,8 +212,9 @@ export const FormLayout = () => { document.title = `${questionnaire.process_name} / ${app.questionnaire_name} : DBCA Authorisations`; }, [questionnaire.process_name, app.questionnaire_name]); - // Guard against StrictMode double-invocation: only show the notice once per mount. - const privacyNoticeShown = React.useRef(false); + // Guard against StrictMode double-invocation: only show the notice once per mount + // for the editable applications. + const privacyNoticeShown = React.useRef(!userCanEdit); // Notify once on mount that personal information is being collected. React.useEffect(() => { @@ -357,10 +358,7 @@ const AccountMenu = ({ ) diff --git a/frontend/src/components/layout/form/FormReviewPage.tsx b/frontend/src/components/layout/form/FormReviewPage.tsx index dfc09c9..9d87f2f 100644 --- a/frontend/src/components/layout/form/FormReviewPage.tsx +++ b/frontend/src/components/layout/form/FormReviewPage.tsx @@ -19,6 +19,7 @@ import type { IAnswer, IApplicationAttachment, IFormAnswers, IGridAnswerRow } fr import type { AsyncVoidAction } from "../../../context/types/Generic"; import { Question, type IFormSection, type IFormStep, type IGridQuestionColumn, type IQuestion, type IQuestionnaire } from "../../../context/types/Questionnaire"; import { FileAttachmentList } from '../../Common'; +import { SubmissionModal } from './SubmissionModal'; const getStepPrefix = (stepIndex: number) => `${stepIndex + 1}.`; const getSectionPrefix = (sectionIndex: number) => `${String.fromCharCode(65 + sectionIndex)})`; @@ -46,6 +47,7 @@ export function FormReviewPage({ const [turnstileLoading, setTurnstileLoading] = React.useState(userCanEdit); const [turnstileError, setTurnstileError] = React.useState(null); const [turnstileToken, setTurnstileToken] = React.useState(null); + const [submissionModalOpen, setSubmissionModalOpen] = React.useState(!userCanEdit); const hasInitializedRef = React.useRef(false); const turnstileContainerRef = React.useRef(null); @@ -109,23 +111,25 @@ export function FormReviewPage({ const isTurnstileVerified = !userCanEdit || (!turnstileLoading && !turnstileError && !!turnstileToken); - // Dummy submit handler for now + /** + * The final submission handler for the review page. It checks for Turnstile verification and submits the application via the API. + * Displays a success modal and triggers a confetti effect on successful submission. + * @returns {Promise} A promise that resolves when the submission process is complete. + * @throws Will throw an error if the Turnstile verification fails or if the API submission fails. + */ const onFinalSubmit = async () => { if (userCanEdit && !turnstileToken) { showSnackbar("Please complete verification before submitting.", "error"); return; } - // alert("Submitted! (implement server-side integration here)"); await ApiManager.submitApplication(applicationKey, turnstileToken || "") - // Successfully save to API .then((resp) => { - showSnackbar("Application has been successfully submitted and is read-only now.", "success"); setUserCanEdit(false); + setSubmissionModalOpen(true); fireConfettiEffect(5); return resp; }) - // Display the error message to user and log to console .catch((error: AxiosError) => { console.error('API Error:', error); const responseData = error.response?.data as { @@ -136,8 +140,6 @@ export function FormReviewPage({ showSnackbar(`Failed to submit: ${message}`, "error"); return null; }); - - // if (!response) return; }; return ( @@ -260,6 +262,12 @@ export function FormReviewPage({ Submit Application + + setSubmissionModalOpen(false)} + /> ); } diff --git a/frontend/src/components/layout/form/SubmissionModal.tsx b/frontend/src/components/layout/form/SubmissionModal.tsx new file mode 100644 index 0000000..3c1c432 --- /dev/null +++ b/frontend/src/components/layout/form/SubmissionModal.tsx @@ -0,0 +1,68 @@ +import CloseIcon from '@mui/icons-material/Close'; +import ExitToAppIcon from '@mui/icons-material/ExitToApp'; +import DoneAllRoundedIcon from '@mui/icons-material/DoneAllRounded'; +import DownloadIcon from '@mui/icons-material/Download'; +import Box from '@mui/material/Box'; +import Button from '@mui/material/Button'; +import Dialog from '@mui/material/Dialog'; +import DialogContent from '@mui/material/DialogContent'; +import DialogTitle from '@mui/material/DialogTitle'; +import IconButton from '@mui/material/IconButton'; +import Typography from '@mui/material/Typography'; + +/** + * Modal displayed after successful application submission. + * Confirms submission status, explains next steps, and provides download option. + */ +export function SubmissionModal({ + open, + applicationKey, + onClose, +}: { + open: boolean; + applicationKey: string; + onClose: () => void; +}) { + return ( + + + + + Application Successfully Submitted + + + + + + + + + This application is now locked in read-only mode. + + + + You will be able to track the progress of your application from the "My Applications" page. Any additional information or requests for clarification will be sent to your registered email address. + + + + + + + + + ); +} diff --git a/frontend/src/test/unit/components/layout/form/form-review-page.test.tsx b/frontend/src/test/unit/components/layout/form/form-review-page.test.tsx index c49850c..174c2e2 100644 --- a/frontend/src/test/unit/components/layout/form/form-review-page.test.tsx +++ b/frontend/src/test/unit/components/layout/form/form-review-page.test.tsx @@ -111,7 +111,7 @@ describe("FormReviewPage", () => { vi.clearAllMocks(); }); - it("submits after verification and confirmation, then switches to read-only mode", async () => { + it("submits after verification and confirmation, then displays submission modal", async () => { const setUserCanEdit = vi.fn(); submitApplicationMock.mockResolvedValue({ key: "app-1" }); turnstileRenderMock.mockImplementation(async (_container: unknown, callbacks: { onSuccess?: (token: string) => void }) => { @@ -136,11 +136,15 @@ describe("FormReviewPage", () => { await waitFor(() => { expect(submitApplicationMock).toHaveBeenCalledWith("app-1", "token-123"); }); - expect(showSnackbarMock).toHaveBeenCalledWith( - "Application has been successfully submitted and is read-only now.", - "success", - ); + + // Verify modal is displayed after submission + await waitFor(() => { + expect(screen.getByText("Application Successfully Submitted")).toBeInTheDocument(); + expect(screen.getByText(/locked in read-only mode/i)).toBeInTheDocument(); + }); + expect(setUserCanEdit).toHaveBeenCalledWith(false); + expect(fireConfettiEffectMock).toHaveBeenCalledWith(5); }); it("shows verification error text when Turnstile reports an error", async () => { @@ -160,7 +164,7 @@ describe("FormReviewPage", () => { expect(submitApplicationMock).not.toHaveBeenCalled(); }); - it("does not initialise Turnstile in read-only mode", () => { + it("does not initialise Turnstile in read-only mode and displays modal", () => { const setUserCanEdit = vi.fn(); renderWithForm({ @@ -171,6 +175,12 @@ describe("FormReviewPage", () => { expect(turnstileRenderMock).not.toHaveBeenCalled(); expect(screen.queryByText(/Verification failed:/i)).not.toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Submit Application" })).toBeDisabled(); + + // Modal should be displayed when userCanEdit is false + expect(screen.getByText("Application Successfully Submitted")).toBeInTheDocument(); + + // Submit button should be present but disabled + const submitButton = screen.getByRole("button", { name: "Submit Application", hidden: true }); + expect(submitButton).toBeDisabled(); }); }); diff --git a/frontend/src/test/unit/components/layout/form/submission-modal.test.tsx b/frontend/src/test/unit/components/layout/form/submission-modal.test.tsx new file mode 100644 index 0000000..10a570f --- /dev/null +++ b/frontend/src/test/unit/components/layout/form/submission-modal.test.tsx @@ -0,0 +1,140 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { SubmissionModal } from "../../../../../components/layout/form/SubmissionModal"; + +describe("SubmissionModal", () => { + it("displays modal when open is true", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + expect(screen.getByText("Application Successfully Submitted")).toBeInTheDocument(); + expect(screen.getByText(/locked in read-only mode/i)).toBeInTheDocument(); + }); + + it("does not display modal when open is false", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + expect(screen.queryByText("Application Successfully Submitted")).not.toBeInTheDocument(); + }); + + it("displays both action buttons", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + expect(screen.getByRole("link", { name: "Download PDF" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Exit application" })).toBeInTheDocument(); + }); + + it("displays explanation text about application status and updates", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + expect(screen.getByText("This application is now locked in read-only mode.")).toBeInTheDocument(); + expect(screen.getByText(/You will be able to track the progress/i)).toBeInTheDocument(); + expect(screen.getByText(/additional information or requests for clarification/i)).toBeInTheDocument(); + }); + + it("calls onClose when close button is clicked", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + const closeButton = screen.getByRole("button", { name: /close/i }); + fireEvent.click(closeButton); + + expect(onCloseMock).toHaveBeenCalledTimes(1); + }); + + it("Display buttons with correct accessibility labels and hrefs", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + // Download link (Button with href renders as element) + const downloadLink = screen.getByRole("link", { name: /Download PDF/i }); + expect(downloadLink).toHaveAttribute("href", "/d/test-app-456"); + + // Exit button + const exitButton = screen.getByRole("button", { name: "Exit application" }); + expect(exitButton).toBeInTheDocument(); + }); + + it("Exit application button calls window.close", () => { + const onCloseMock = vi.fn(); + const windowCloseSpy = vi.spyOn(window, "close").mockImplementation(() => {}); + + render( + + ); + + const exitButton = screen.getByRole("button", { name: "Exit application" }); + fireEvent.click(exitButton); + + expect(windowCloseSpy).toHaveBeenCalled(); + + windowCloseSpy.mockRestore(); + }); + + it("displays success icon", () => { + const onCloseMock = vi.fn(); + + render( + + ); + + // MUI icon should be rendered; we check for it via the SVG title or other accessibility features + const title = screen.getByText("Application Successfully Submitted"); + expect(title).toBeInTheDocument(); + // The icon is rendered before the title text in the DialogTitle + }); +}); From 570986c4f29c8ad2c0b59811b19d28e1a438b140 Mon Sep 17 00:00:00 2001 From: dbca-serkan Date: Mon, 3 Aug 2026 15:41:12 +0800 Subject: [PATCH 14/17] Add loading state to submit button and disable during submission --- .../components/layout/form/FormReviewPage.tsx | 18 ++++- .../components/layout/main/NewApplication.tsx | 2 +- .../layout/form/form-review-page.test.tsx | 77 +++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/layout/form/FormReviewPage.tsx b/frontend/src/components/layout/form/FormReviewPage.tsx index 9d87f2f..532b47a 100644 --- a/frontend/src/components/layout/form/FormReviewPage.tsx +++ b/frontend/src/components/layout/form/FormReviewPage.tsx @@ -47,6 +47,7 @@ export function FormReviewPage({ const [turnstileLoading, setTurnstileLoading] = React.useState(userCanEdit); const [turnstileError, setTurnstileError] = React.useState(null); const [turnstileToken, setTurnstileToken] = React.useState(null); + const [submitInProgress, setSubmitInProgress] = React.useState(false); const [submissionModalOpen, setSubmissionModalOpen] = React.useState(!userCanEdit); const hasInitializedRef = React.useRef(false); const turnstileContainerRef = React.useRef(null); @@ -111,6 +112,13 @@ export function FormReviewPage({ const isTurnstileVerified = !userCanEdit || (!turnstileLoading && !turnstileError && !!turnstileToken); + // Disable the submit button if any of the following conditions are true: + // - the user has not confirmed the accuracy of their answers, + // - the user cannot edit (read-only mode), + // - Turnstile verification has not been completed successfully, + // - or a submission is currently in progress. + const submitButtonDisabled = !hasConfirmed || !userCanEdit || !isTurnstileVerified || submitInProgress; + /** * The final submission handler for the review page. It checks for Turnstile verification and submits the application via the API. * Displays a success modal and triggers a confetti effect on successful submission. @@ -123,6 +131,9 @@ export function FormReviewPage({ return; } + // Disable the submit button to prevent multiple submissions + setSubmitInProgress(true); + await ApiManager.submitApplication(applicationKey, turnstileToken || "") .then((resp) => { setUserCanEdit(false); @@ -139,6 +150,9 @@ export function FormReviewPage({ const message = responseData?.turnstile_token?.[0] ?? responseData?.status?.[0] ?? error.message; showSnackbar(`Failed to submit: ${message}`, "error"); return null; + }) + .finally(() => { + setSubmitInProgress(false); }); }; @@ -255,8 +269,10 @@ export function FormReviewPage({ variant="contained" size="large" color="success" + loadingPosition="start" onClick={onFinalSubmit} - disabled={!hasConfirmed || !userCanEdit || !isTurnstileVerified} + loading={submitInProgress} + disabled={submitButtonDisabled} startIcon={} > Submit Application diff --git a/frontend/src/components/layout/main/NewApplication.tsx b/frontend/src/components/layout/main/NewApplication.tsx index d1294de..ecf3223 100644 --- a/frontend/src/components/layout/main/NewApplication.tsx +++ b/frontend/src/components/layout/main/NewApplication.tsx @@ -454,7 +454,7 @@ const Questionnaire = ({