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.

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

Paginate Operations Form #269

Merged
merged 46 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
32ac6db
refactor: add form base component and default rjsf theme
marcellmueller Nov 3, 2023
287f553
feat: set max page width and center header and footer
marcellmueller Nov 6, 2023
247c0d0
fix: adjust main div padding
marcellmueller Nov 6, 2023
7c50c5d
fix: breadcrumbs hidden due to navidation updates
marcellmueller Nov 6, 2023
f390087
chore: cleanup operations
marcellmueller Nov 10, 2023
db6bd95
refactor: destructure props in operations form
marcellmueller Nov 10, 2023
40de665
feat: create multi step button component
marcellmueller Nov 10, 2023
5d78acb
feat: add multistep header
marcellmueller Nov 10, 2023
51b1df5
fix: add multistep support to operations edit
marcellmueller Nov 10, 2023
f2035b1
refactor: create multistep form base for reusable form pagination
marcellmueller Nov 11, 2023
29b398c
fix: pass form section titles to multistep header
marcellmueller Nov 11, 2023
4474432
fix: move mulistep buttons to formbase children
marcellmueller Nov 11, 2023
90b7c8d
feat: return operation row id from create_operation
marcellmueller Nov 14, 2023
462d79f
refactor: use single page route for creating and viewing operations
marcellmueller Nov 14, 2023
38de8f5
fix: make ui:hidden work correctly with inline field template
marcellmueller Nov 15, 2023
ea5e951
fix: add disabled props to custom widgets
marcellmueller Nov 15, 2023
294ccce
feat: change default operation status and add submit argument
marcellmueller Nov 15, 2023
e7331d2
chore: remove unnecesary fragments
marcellmueller Nov 15, 2023
c1fec7a
fix: add readonly/disabled state to custom widgets
marcellmueller Nov 15, 2023
677c806
fix: multistep form header responsive mobile view
marcellmueller Nov 16, 2023
3fc8cda
feat: style operations submission page
marcellmueller Nov 16, 2023
44303ab
chore: revert status string formatting
marcellmueller Nov 16, 2023
2de2304
fix: use 0 for create operator query param instead of string
marcellmueller Nov 16, 2023
6d2b837
chore: run pre commit on all files
marcellmueller Nov 16, 2023
fd3d738
refactor: operation put api submit query param
marcellmueller Nov 17, 2023
ec29bfe
test: update operation api tests
marcellmueller Nov 17, 2023
3d69ab2
fix: operation put api save
marcellmueller Nov 17, 2023
bbaa6ec
test: add operations put api with submit test
marcellmueller Nov 17, 2023
2f68838
chore: cleanup unused group schema
marcellmueller Nov 17, 2023
6965bb5
fix: combobox enum error
marcellmueller Nov 17, 2023
425f485
feat: add widget error states
marcellmueller Nov 20, 2023
f0330c3
feat: add error state to inline field template
marcellmueller Nov 21, 2023
9659194
refactor: update multistep form base
marcellmueller Nov 21, 2023
6828090
feat: add custom titles to user operator form
marcellmueller Nov 21, 2023
410877e
feat: add tailwind global link colour override
marcellmueller Nov 21, 2023
13824ea
fix: add header icon link and minor style improvements
marcellmueller Nov 21, 2023
636c158
chore: update review button styles
marcellmueller Nov 21, 2023
2a5f7be
fix: add file widget disabled colour
marcellmueller Nov 21, 2023
2406d79
chore: remove commented code in operations schema
marcellmueller Nov 22, 2023
70d5f4b
chore: remove commented code in theme.ts
marcellmueller Nov 22, 2023
d03a2f0
chore: remove unneeded eslint disable rule
marcellmueller Nov 22, 2023
4c1b8a0
feat: remove use client
marcellmueller Nov 22, 2023
a3e766d
refactor: simplify form step math in operation form
marcellmueller Nov 22, 2023
0bd8830
feat: improve form responsive layout
marcellmueller Nov 22, 2023
a63d00d
fix: add min width to multstep header circles
marcellmueller Nov 22, 2023
123048c
test: update endpoint tests
marcellmueller Nov 23, 2023
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
11 changes: 7 additions & 4 deletions bc_obps/registration/api/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,15 @@ def create_operation(request, payload: OperationIn):
delattr(payload, field_name)

operation = Operation.objects.create(**payload.dict())
return {"name": operation.name}

return {"name": operation.name, "id": operation.id}


##### PUT #####


@router.put("/operations/{operation_id}")
def update_operation(request, operation_id: int, payload: OperationIn):
def update_operation(request, operation_id: int, submit, payload: OperationIn):
operation = get_object_or_404(Operation, id=operation_id)
if "operator" in payload.dict():
operator = payload.dict()["operator"]
Expand Down Expand Up @@ -87,8 +88,10 @@ def update_operation(request, operation_id: int, payload: OperationIn):
for attr, value in payload.dict().items():
if attr not in excluded_fields:
setattr(operation, attr, value)
# set the operation status to 'pending' on update
operation.status = "Pending"
# set the operation status to 'pending' on update
if submit == "true":
Copy link
Contributor

@BCerki BCerki Nov 21, 2023

Choose a reason for hiding this comment

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

What does submit true/false mean? Like if it's the entire form submission vs one of the steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the final step we set ?submit=true so that we can set the status to pending. We want the status to stay as not_registered unless we are submitting.

operation.status = Operation.Statuses.PENDING

operation.save()
return {"name": operation.name}

Expand Down
27 changes: 27 additions & 0 deletions bc_obps/registration/migrations/0002_alter_operation_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Generated by Django 4.2.6 on 2023-11-22 23:59

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
('registration', '0001_initial'),
]

operations = [
migrations.AlterField(
model_name='operation',
name='status',
field=models.CharField(
choices=[
('not_registered', 'Not Registered'),
('pending', 'Pending'),
('approved', 'Approved'),
('rejected', 'Rejected'),
],
db_comment='The status of an operation in the app (e.g. pending review)',
default='not_registered',
max_length=1000,
),
),
]
27 changes: 27 additions & 0 deletions bc_obps/registration/migrations/0003_alter_operation_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Generated by Django 4.2.6 on 2023-11-23 00:11

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
('registration', '0002_alter_operation_status'),
]

operations = [
migrations.AlterField(
model_name='operation',
name='status',
field=models.CharField(
choices=[
('not_registered', 'Not Registered'),
('pending', 'Pending'),
('approved', 'Approved'),
('rejected', 'Rejected'),
],
db_comment='The status of an operation in the app (e.g. pending review)',
default='pending',
max_length=1000,
),
),
]
2 changes: 1 addition & 1 deletion bc_obps/registration/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class Statuses(models.TextChoices):
status = models.CharField(
max_length=1000,
choices=Statuses.choices,
default=Statuses.PENDING,
default=Statuses.NOT_REGISTERED,
db_comment="The status of an operation in the app (e.g. pending review)",
)
# temporary handling, many-to-many handled in #138
Expand Down
57 changes: 48 additions & 9 deletions bc_obps/registration/tests/test_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,11 @@ def test_post_new_operation(self, client):
)
post_response = client.post(self.endpoint, content_type=content_type_json, data=mock_operation.json())
assert post_response.status_code == 200
assert post_response.json() == {"name": "Springfield Nuclear Power Plant"}
assert post_response.json().get('name') == "Springfield Nuclear Power Plant"
assert post_response.json().get('id') is not None
# check that the default status of pending was applied
marcellmueller marked this conversation as resolved.
Show resolved Hide resolved
get_response = client.get(self.endpoint).json()[0]
assert 'status' in get_response and get_response['status'] == 'pending'
assert 'status' in get_response and get_response['status'] == 'not_registered'

def test_post_new_malformed_operation(self, client):
response = client.post(
Expand All @@ -105,7 +106,7 @@ def test_post_new_malformed_operation(self, client):

def test_put_operation_update_status_approved(self, client):
operation = baker.make(Operation)
assert operation.status == Operation.Statuses.PENDING
assert operation.status == Operation.Statuses.NOT_REGISTERED

url = self.build_update_status_url(operation_id=operation.id)

Expand All @@ -128,7 +129,7 @@ def test_put_operation_update_status_approved(self, client):

def test_put_operation_update_status_rejected(self, client):
operation = baker.make(Operation)
assert operation.status == Operation.Statuses.PENDING
assert operation.status == Operation.Statuses.NOT_REGISTERED

url = self.build_update_status_url(operation_id=operation.id)

Expand All @@ -151,7 +152,7 @@ def test_put_operation_update_status_rejected(self, client):

def test_put_operation_not_verified_when_not_registered(self, client):
operation = baker.make(Operation)
assert operation.status == Operation.Statuses.PENDING
assert operation.status == Operation.Statuses.NOT_REGISTERED

url = self.build_update_status_url(operation_id=operation.id)

Expand All @@ -173,7 +174,7 @@ def test_put_operation_not_verified_when_not_registered(self, client):
def test_put_operation_update_status_invalid_data(self, client):
def send_put_invalid_data():
operation = baker.make(Operation)
assert operation.status == Operation.Statuses.PENDING
assert operation.status == Operation.Statuses.NOT_REGISTERED
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check the default status so often?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, I updated the original tests which were checking the default status and kept it in.


url = self.build_update_status_url(operation_id=operation.id)

Expand All @@ -198,7 +199,7 @@ def test_get_method_for_200_status(self, client):
response = client.get(self.endpoint + str(operation.id))
assert response.status_code == 200

def test_put_operation(self, client):
def test_put_operation_without_submit(self, client):
naics_code = baker.make(NaicsCode)
naics_category = baker.make(NaicsCategory)
document = baker.make(Document)
Expand Down Expand Up @@ -229,15 +230,53 @@ def test_put_operation(self, client):
)

response = client.put(
self.endpoint + str(operation.id), content_type=content_type_json, data=mock_operation.json()
self.endpoint + str(operation.id) + "?submit=false",
content_type=content_type_json,
data=mock_operation.json(),
)
assert response.status_code == 200
assert response.json() == {"name": "New name"}

get_response = client.get(self.endpoint + str(operation.id)).json()
assert get_response["status"] == Operation.Statuses.NOT_REGISTERED

def test_put_operation_with_submit(self, client):
naics_code = baker.make(NaicsCode)
naics_category = baker.make(NaicsCategory)
document = baker.make(Document)
contact = baker.make(Contact, postal_code="V1V 1V2")
operator = baker.make(Operator)
activity = baker.make(ReportingActivity)
operation = baker.make(Operation)
operation.reporting_activities.set([activity.id])

mock_operation = OperationIn(
name="New name",
type="Single Facility Operation",
naics_code_id=naics_code.id,
reporting_activities=[1],
regulated_products=[1],
naics_category_id=naics_category.id,
documents=[document.id],
contacts=[contact.id],
operator_id=operator.id,
)

response = client.put(
self.endpoint + str(operation.id) + "?submit=true",
content_type=content_type_json,
data=mock_operation.json(),
)
assert response.status_code == 200
assert response.json() == {"name": "New name"}

get_response = client.get(self.endpoint + str(operation.id)).json()
assert get_response["status"] == Operation.Statuses.PENDING

def test_put_malformed_operation(self, client):
operation = baker.make(Operation)
response = client.put(
self.endpoint + str(operation.id),
self.endpoint + str(operation.id) + "?submit=false",
content_type=content_type_json,
data={"garbage": "i am bad data"},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import Loading from "@/app/components/loading/SkeletonForm";

export default function Page({ params }: { params: { operation: number } }) {
return (
<>
<Suspense fallback={<Loading />}>
<Operation numRow={params.operation} />
</Suspense>
</>
<Suspense fallback={<Loading />}>
<Operation numRow={params.operation} />
</Suspense>
);
}
11 changes: 0 additions & 11 deletions client/app/(authenticated)/dashboard/operations/create/page.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion client/app/(authenticated)/dashboard/operations/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default function Page() {
return (
<>
<h1>Operations List</h1>
<Link href="/dashboard/operations/create">
<Link href="/dashboard/operations/0/1">
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the 0 is unclear because it looks like a real ID (especially because it shows in the breadcrumb:
image
)

It would be nice if we could create the operation and ID when someone clicks the Add Operation button. But I assume that doesn't work because there are mandatory fields for creating an operation that the user has to fill out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've discussed the breadcrumbs bugs a bit with Shon and this will be fixed. Though it's out of scope for this ticket.

<Button variant="contained">Add Operation</Button>
</Link>
<Suspense fallback={<Loading />}>
Expand Down
1 change: 0 additions & 1 deletion client/app/(authenticated)/dashboard/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export default function Page() {
fetchData();
}
}, [role]); // dependencies array

return (
<div>
{/* 🪜 create a grid layout using container prop to create a container that wraps the grid items */}
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/datagrid/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const DataGrid: React.FC<Props> = ({ rows, columns, cntxt }) => {
renderCell: (params: GridRenderCellParams) => (
<div>
{/* 🔗 Add reg or details link */}
<Link href={`operations/${params.row.id}`}>
<Link href={`operations/${params.row.id}/1`}>
<Button variant="contained">
{params.row.status === "Not Registered"
? "Start Registration"
Expand Down
72 changes: 72 additions & 0 deletions client/app/components/form/MultiStepButtons.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"use client";

import { Button } from "@mui/material";
import { useFormStatus } from "react-dom";
import Link from "next/link";

interface SubmitButtonProps {
baseUrl: string;
cancelUrl: string;
classNames?: string;
step: number;
steps: string[];
submitEveryStep?: boolean;
}

const SubmitButton: React.FunctionComponent<SubmitButtonProps> = ({
baseUrl,
step,
steps,
cancelUrl,
classNames,
submitEveryStep,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm having trouble with the submitEveryStep variable. Does it mean that all steps are mandatory? That a user has or hasn't filled all the steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

submitEveryStep means the form submits between pages, if not it uses a button without type="submit" so it won't trigger RJSF validation.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's about validation, I'd suggest changing the variable name to something about validation or adding the explanation above in a comment. (Sidenote, why would we ever not want to validate?)

}) => {
const { pending } = useFormStatus();
const isFinalStep = step === steps.length - 1;
return (
<div className={`flex w-full mt-8 justify-between ${classNames}`}>
{cancelUrl && (
<Link href={cancelUrl}>
<Button variant="outlined">Cancel</Button>
</Link>
)}
<div>
{step !== 0 ? (
<Link href={`${baseUrl}/${step}`}>
<Button
variant="contained"
type="button"
disabled={step === 0}
className="mr-4"
>
Back
</Button>
</Link>
) : (
<Button variant="contained" type="button" disabled className="mr-4">
Back
</Button>
)}
{!isFinalStep && !submitEveryStep && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Since I don't understand what submitEveryStep is, I'm having trouble following the logic here and below

<Link href={`${baseUrl}/${step + 2}`}>
<Button variant="contained" type="button" disabled={isFinalStep}>
Next
</Button>
</Link>
)}
{isFinalStep && !submitEveryStep && (
<Button type="submit" aria-disabled={pending} variant="contained">
Submit
</Button>
)}
{submitEveryStep && (
<Button type="submit" aria-disabled={pending} variant="contained">
{!isFinalStep ? "Next" : "Submit"}
</Button>
)}
</div>
</div>
);
};

export default SubmitButton;
Loading
Loading