Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

392 button tech debt #2519

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

392 button tech debt #2519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcc45f1
feat: Add reporting step buttons component for reporting forms, separ…
gdalcengio Nov 21, 2024
7510c25
chore: Add reporting step buttons to facility review page
gdalcengio Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 16 additions & 28 deletions bciers/apps/reporting/src/app/components/facility/FacilityReview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
import React, { ChangeEvent, useEffect, useState } from "react";
import {
Box,
Button,
Alert,
Checkbox,
Typography,
FormControlLabel,
CircularProgress,
} from "@mui/material";
import MultiStepHeader from "@bciers/components/form/components/MultiStepHeader";
import FormBase from "@bciers/components/form/FormBase";
Expand All @@ -21,8 +19,9 @@ import { RJSFSchema } from "@rjsf/utils";
import { actionHandler } from "@bciers/actions";
import { UUID } from "crypto";
import { IChangeEvent } from "@rjsf/core";
import { useRouter, useSearchParams } from "next/navigation";
import { useSearchParams } from "next/navigation";
import serializeSearchParams from "@bciers/utils/src/serializeSearchParams";
import ReportingStepButtons from "@bciers/components/form/components/ReportingStepButtons";

interface Props {
version_id: number;
Expand All @@ -47,7 +46,7 @@ const getAllActivities = async () => {
};

const FacilityReview: React.FC<Props> = ({ version_id, facility_id }) => {
const [isLoading, setIsLoading] = useState(false);
const [isSaving, setIsSaving] = useState(false);
const [errorList, setErrorList] = useState<string[]>([]);
const [isSuccess, setIsSuccess] = useState(false);
const [formData, setFormData] = useState<any>({});
Expand All @@ -57,7 +56,9 @@ const FacilityReview: React.FC<Props> = ({ version_id, facility_id }) => {
properties: {},
});
const [activityList, setActivityList] = useState<Activity[]>([]);
const router = useRouter();
const queryString = serializeSearchParams(useSearchParams());
const reportsTitle = useSearchParams().get('reports_title');
const backUrl = `/reports/${version_id}/person-responsible?reports_title=${reportsTitle}`;

const customStepNames = [
"Operation Information",
Expand Down Expand Up @@ -119,9 +120,8 @@ const FacilityReview: React.FC<Props> = ({ version_id, facility_id }) => {
setFormData(event.formData);
};

const queryString = serializeSearchParams(useSearchParams());
const handleSave = async () => {
setIsLoading(true); // Start loading when save is clicked
setIsSaving(true); // Start loading when save is clicked
const updatedFacility = {
...formData,
activities: Object.keys(activities).filter((id) => activities[+id]),
Expand All @@ -142,22 +142,13 @@ const FacilityReview: React.FC<Props> = ({ version_id, facility_id }) => {
console.error("Error updating facility:", error);
setErrorList([error.message || "An error occurred"]);
} finally {
setIsLoading(false);
setIsSaving(false);
setTimeout(() => {
setIsSuccess(false);
}, 3000);
router.push(`activities${queryString}`);
}
};

const buttonContent = isLoading ? (
<CircularProgress data-testid="progressbar" role="progress" size={24} />
) : isSuccess ? (
"✅ Success"
) : (
"Save & Continue"
);

return (
<Box sx={{ p: 3 }}>
<div className="container mx-auto p-4" data-testid="facility-review">
Expand Down Expand Up @@ -218,17 +209,14 @@ const FacilityReview: React.FC<Props> = ({ version_id, facility_id }) => {
</Box>
</div>
)}
<div className="flex justify-end gap-3">
<Button
variant="contained"
type="submit"
aria-disabled={isLoading}
disabled={isLoading}
onClick={handleSave}
>
{buttonContent}
</Button>
</div>
<ReportingStepButtons
allowBackNavigation={true}
backUrl={backUrl}
continueUrl={`activities${queryString}`}
isSaving={isSaving}
isSuccess={isSuccess}
saveButtonDisabled={false}
/>
</FormBase>
</div>
</div>
Expand Down
13 changes: 6 additions & 7 deletions bciers/apps/reporting/src/app/components/operations/OperationReview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,17 @@ export default function OperationReview({
allActivities,
allRegulatedProducts,
}: Props) {
const router = useRouter();
const [schema, setSchema] = useState<RJSFSchema>(operationReviewSchema);
const [formDataState, setFormDataState] = useState<any>(formData);
const queryString = serializeSearchParams(useSearchParams());
const saveAndContinueUrl = `/reports/${version_id}/person-responsible${queryString}`;
const continueUrl = `/reports/${version_id}/person-responsible${queryString}`;

const reportingWindowEnd = formatDate(
reportingYear.reporting_window_end,
"MMM DD YYYY",
);

const submitHandler = async (
const saveHandler = async (
data: { formData?: any },
reportVersionId: number,
) => {
Expand Down Expand Up @@ -91,9 +90,7 @@ export default function OperationReview({
body: JSON.stringify(preparedData),
});

if (response) {
router.push(saveAndContinueUrl); // Navigate on success
}
return response;
};

useEffect(() => {
Expand Down Expand Up @@ -175,7 +172,9 @@ export default function OperationReview({
formData={formDataState}
baseUrl={baseUrl}
cancelUrl={cancelUrl}
onSubmit={(data) => submitHandler(data, version_id)}
backUrl={cancelUrl}
continueUrl={continueUrl}
onSubmit={(data) => saveHandler(data, version_id)}
/>
);
}
17 changes: 8 additions & 9 deletions bciers/apps/reporting/src/app/components/operations/personResponsible/PersonResponsible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ const PersonResponsible = ({ version_id }: Props) => {
const router = useRouter();

const queryString = serializeSearchParams(useSearchParams());
const saveAndContinueUrl =
const continueUrl =
operationType === "Linear Facility Operation"
? `/reports/${version_id}/facilities/lfo-facilities${queryString}`
: `/reports/${version_id}/facilities/${facilityId}/review${queryString}&facilities_title=Facility`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issue here with this path. When queryString is null, we get:
.../review&facilities_title=Facility, which doesn't parse properly because we don't have the ? before the query params.

A potential solution (which would need to be investigated for side effects) is to change the serializeSearchParams utility function to return ? when there are no params & append & to the end of the querystring when there are.

We'd have to look at where serializeSearchParams is used & make sure everything still works as expected where it's implemented

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reproduce the bug, start a report & get to the Person Responsible page, once you enter data there & click Save&Continue, you'll get a 404 error because the params are missing the ? to start them off

const backUrl = `/reports/${version_id}/review-operator-data${queryString}`

const taskListElements: TaskListElement[] = [
{
Expand All @@ -70,7 +71,7 @@ const PersonResponsible = ({ version_id }: Props) => {
{
type: "Page",
title: "Review facilities",
link: saveAndContinueUrl,
link: continueUrl,
},
],
},
Expand Down Expand Up @@ -171,7 +172,7 @@ const PersonResponsible = ({ version_id }: Props) => {
}
}, 300);

const handleSubmit = async () => {
const handleSave = async () => {
const endpoint = `reporting/report-version/${version_id}/report-contact`;
const method = "POST";
const payload = {
Expand All @@ -182,10 +183,6 @@ const PersonResponsible = ({ version_id }: Props) => {
const response = await actionHandler(endpoint, method, endpoint, {
body: JSON.stringify(payload),
});

if (response) {
router.push(`${saveAndContinueUrl}`);
}
};

const handleSync = async () => {
Expand Down Expand Up @@ -215,6 +212,8 @@ const PersonResponsible = ({ version_id }: Props) => {
"Sign-off & Submit",
]}
cancelUrl={"/reports"}
backUrl={backUrl}
continueUrl={continueUrl}
taskListElements={taskListElements}
schema={schema}
uiSchema={{
Expand All @@ -228,8 +227,8 @@ const PersonResponsible = ({ version_id }: Props) => {
}}
formData={formData}
onChange={handleContactSelect}
onSubmit={handleSubmit}
submitButtonDisabled={!selectedContactId}
onSubmit={handleSave}
saveButtonDisabled={!selectedContactId}
/>
</>
);
Expand Down
22 changes: 12 additions & 10 deletions bciers/libs/components/src/form/MultiStepFormWithTaskList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe("MultiStepFormWithTaskList", () => {
uiSchema={uiSchema}
formData={formData}
onSubmit={mockOnSubmit}
continueUrl="www.test.com/continue"
/>,
);

Expand All @@ -57,10 +58,8 @@ describe("MultiStepFormWithTaskList", () => {
expect(screen.getByLabelText("Name")).toBeVisible();
expect(screen.getByLabelText("Age")).toBeVisible();

// Verify save button is present
expect(
screen.getByRole("button", { name: "Save and Continue" }),
).toBeVisible();
// Verify save button from reportingStepButton component is present
expect(screen.getByRole("button", { name: "Save" })).toBeVisible();
});

it("handles form submission", async () => {
Expand All @@ -73,6 +72,7 @@ describe("MultiStepFormWithTaskList", () => {
uiSchema={uiSchema}
formData={formData}
onSubmit={mockOnSubmit}
continueUrl="www.test.com/continue"
/>,
);

Expand All @@ -83,7 +83,7 @@ describe("MultiStepFormWithTaskList", () => {
fireEvent.change(screen.getByLabelText("Age"), { target: { value: 35 } });

// Simulate form submission
fireEvent.click(screen.getByRole("button", { name: "Save and Continue" }));
fireEvent.click(screen.getByRole("button", { name: "Save" }));

// Assert that the submission function was called with correct data
await waitFor(() => expect(mockOnSubmit).toHaveBeenCalled());
Expand All @@ -103,17 +103,18 @@ describe("MultiStepFormWithTaskList", () => {
formData={formData}
onSubmit={mockOnSubmit}
cancelUrl="http://example.com"
continueUrl="www.test.com/continue"
/>,
);

// Check if the cancel button is present and has the correct href
expect(screen.getByRole("button", { name: "Cancel" })).toBeVisible();
// Check if the back button is present and has the correct href
expect(screen.getByRole("button", { name: "Back" })).toBeVisible();
expect(
screen.getByRole("button", { name: "Cancel" }).closest("a"),
screen.getByRole("button", { name: "Back" }).closest("a"),
).toHaveAttribute("href", "http://example.com");
});

it("does not render cancel button if cancelUrl is not provided", () => {
it("does not render back button if cancelUrl is not provided", () => {
render(
<MultiStepFormWithTaskList
initialStep={0}
Expand All @@ -123,9 +124,10 @@ describe("MultiStepFormWithTaskList", () => {
uiSchema={uiSchema}
formData={formData}
onSubmit={mockOnSubmit}
continueUrl="www.test.com/continue"
/>,
);

expect(screen.queryByRole("button", { name: "Cancel" })).toBeNull();
expect(screen.queryByRole("button", { name: "Back" })).toBeNull();
});
});
59 changes: 28 additions & 31 deletions bciers/libs/components/src/form/MultiStepFormWithTaskList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { TaskListElement } from "@bciers/components/navigation/reportingTaskList
import ReportingTaskList from "@bciers/components/navigation/reportingTaskList/ReportingTaskList";
import { FormBase } from "@bciers/components/form/index";
import { RJSFSchema } from "@rjsf/utils";
import { Box, Button } from "@mui/material";
import Link from "next/link";
import { Box } from "@mui/material";
import ReportingStepButtons from "./components/ReportingStepButtons";

interface Props {
initialStep: number;
Expand All @@ -18,10 +18,12 @@ interface Props {
formData: any;
baseUrl?: string;
cancelUrl?: string;
backUrl: string;
continueUrl: string;
onSubmit: (data: any) => Promise<void>;
buttonText?: string;
onChange?: (data: any) => void;
submitButtonDisabled?: boolean;
saveButtonDisabled?: boolean;
}

const MultiStepFormWithTaskList: React.FC<Props> = ({
Expand All @@ -31,18 +33,24 @@ const MultiStepFormWithTaskList: React.FC<Props> = ({
schema,
uiSchema,
formData,
cancelUrl,
backUrl,
continueUrl,
onSubmit,
buttonText,
onChange,
submitButtonDisabled,
saveButtonDisabled,
}) => {
const [isSubmitting, setIsSubmitting] = useState(false);
const [isSaving, setIsSaving] = useState(false);
const [isSuccess, setIsSuccess] = useState(false);

const handleFormSubmit = async (data: any) => {
setIsSubmitting(true);
await onSubmit(data);
setIsSubmitting(false);
const handleFormSave = async (data: any) => {
setIsSaving(true);
try {
await onSubmit(data);
setIsSuccess(true);
} catch {
setIsSuccess(false);
}
setIsSaving(false);
};

return (
Expand All @@ -59,29 +67,18 @@ const MultiStepFormWithTaskList: React.FC<Props> = ({
<FormBase
schema={schema}
uiSchema={uiSchema}
onSubmit={handleFormSubmit}
onSubmit={handleFormSave}
formData={formData}
onChange={onChange}
>
<Box display="flex" justifyContent="space-between" mt={3}>
{cancelUrl && (
<Link href={cancelUrl} passHref>
<Button variant="outlined">Cancel</Button>
</Link>
)}
<Button
variant="contained"
color="primary"
type="submit"
disabled={isSubmitting || submitButtonDisabled}
>
{isSubmitting
? "Saving..."
: buttonText
? buttonText
: "Save and Continue"}
</Button>
</Box>
<ReportingStepButtons
allowBackNavigation={true}
backUrl={backUrl}
continueUrl={continueUrl}
isSaving={isSaving}
isSuccess={isSuccess}
saveButtonDisabled={saveButtonDisabled}
/>
</FormBase>
</div>
</div>
Expand Down
Loading