217 create operations table #1719
Conversation
BallardRobinett
requested review from
joshgamache,
pbastia and
dleard
and removed request for
joshgamache,
pbastia and
dleard
| OperationRow, | ||
| OperationsSearchParams, | ||
| } from "@/app/components/operations/types"; | ||
| import { OperationRow } from "@shared/components/src/operations/type"; |
There was a problem hiding this comment.
To be consistent with the previous naming
| import { OperationRow } from "@shared/components/src/operations/type"; | |
| import { OperationRow } from "@shared/components/src/operations/types"; |
| export default async function Page({ | ||
| searchParams, | ||
| }: { | ||
| searchParams: OperationsSearchParams; |
There was a problem hiding this comment.
These search params are not the ones matching the table. We should probably not use them here, and implement search on that table at a later point
| // Set flex to 1 to make the column take up all the remaining width if user zooms out | ||
| flex: 0, | ||
| }, | ||
| ] as GridColDef[]; |
There was a problem hiding this comment.
We should type the const variable instead of casting the value, otherwise we don't get a proper type check
const columns: GridColDef[] = [{...}]
| if (isOperatorColumn) { | ||
| columns.splice(OPERATOR_COLUMN_INDEX, 0, { | ||
| field: "operator", | ||
| headerName: "Operator", | ||
| width: 320, | ||
| }); |
There was a problem hiding this comment.
The third column should be "Operation" as per the AC, and we shouldn't make that conditional to whether the user is an operator or not
There was a problem hiding this comment.
just deleting this, Operation column is defined above. This is just something copied over from registration that we don't need
bciers/tsconfig.base.json
Outdated
| @@ -20,7 +20,9 @@ | |||
| "@bciers/components": ["libs/shared/components/src/index.ts"], | |||
| "@bciers/img/*": ["libs/shared/img/*"], | |||
| "@bciers/styles": ["libs/shared/styles/src/index.ts"], | |||
| "@/*": ["apps/registration/*"] | |||
| "@/*": ["apps/registration/*"], | |||
There was a problem hiding this comment.
We might want to reconsider this alias now that the 2 apps live side by side. For tech debt :)
There was a problem hiding this comment.
Yes I think it would make a lot of sense to rename the @/* alias to @registration/*, but I thought maybe it needs to be run by the team first.
| // Show the operator column if the user is CAS internal | ||
| const isOperatorColumn = | ||
| session?.user.app_role?.includes("cas") && | ||
| !session?.user.app_role?.includes("pending"); |
There was a problem hiding this comment.
We can leave this out, it's not needed in the reporting app
| // Render the DataGrid component | ||
| return ( | ||
| <div className="mt-5"> | ||
| <OperationDataGrid |
There was a problem hiding this comment.
Keeping the name will create confusion with the reg app
| <OperationDataGrid | |
| <ReportingOperationDataGrid |
| }: { | ||
| searchParams: OperationsSearchParams; | ||
| }) { | ||
| const session = await getServerSession(authOptions); |
There was a problem hiding this comment.
We don't need the session at this point
|
|
||
| const ReportingOperationDataGrid = ({ | ||
| initialData, | ||
| isOperatorColumn = false, |
There was a problem hiding this comment.
isOperatorColumn is not needed in the reporting context
| const formatTimestamp = (timestamp: string) => { | ||
| if (!timestamp) return undefined; | ||
|
|
||
| const date = new Date(timestamp).toLocaleString("en-CA", { | ||
| month: "short", | ||
| day: "numeric", | ||
| year: "numeric", | ||
| timeZone: "America/Vancouver", | ||
| }); | ||
|
|
||
| const timeWithTimeZone = new Date(timestamp).toLocaleString("en-CA", { | ||
| hour: "numeric", | ||
| minute: "numeric", | ||
| second: "numeric", | ||
| timeZoneName: "short", | ||
| timeZone: "America/Vancouver", | ||
| }); |
There was a problem hiding this comment.
We don't have a timestamp in the table
| return { | ||
| id, | ||
| name, | ||
| bcghg_id, | ||
| operator: operator, | ||
| }; |
There was a problem hiding this comment.
Just for consistency in the coding style
| return { | |
| id, | |
| name, | |
| bcghg_id, | |
| operator: operator, | |
| }; | |
| return { | |
| id, | |
| name, | |
| bcghg_id, | |
| operator, | |
| }; |
There was a problem hiding this comment.
Hey! Sorry just poked my head in here after the demo, hope this was relatively easy to implement from our examples :)
formatOperationRows can be removed altogether, if you notice it's not doing anything (map returning the same values). It's some registration 1 tech debt that we removed in the recent Registration part 2 Operations table.
| } catch (error) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
I think the try block is not useful here if we spit the error right back out. Or is this some javascript weirdness that we try to avoid?
BallardRobinett
force-pushed
the
217-create-operations-table
branch
2 times, most recently
from
a618ae9
to
d07137d
Compare
…perationDataGrid -> ReportingOperationDataGrid for clarity
BallardRobinett
force-pushed
the
217-create-operations-table
branch
from
589eae1
to
a366825
Compare
Card: bcgov/cas-reporting#217
Creating a page showing operators a list of their operations, similar to
OperationsDataGridin registration. Both are relying on the Registration componentDataGrid, which has been moved to a shared component library along with other shared components.The page has replaced the placeholder page showing the operators API call in the reporting app.