CCM-11966: Change button text and TemplateStatus on frontend #759
Conversation
… reflect new data if routing is enabled
… reflect new data if routing is enabled
| "WAITING_FOR_PROOF", | ||
| "PROOF_AVAILABLE" | ||
| "PROOF_AVAILABLE", | ||
| "TEMPLATE_PROOF_APPROVED" |
There was a problem hiding this comment.
if we want to keep this change to the frontend, we shouldn't be updating lists of backend statuses or any backend code including the event publisher
There was a problem hiding this comment.
Because this is used to generate types. I will be changing this approach as suggested by Michael
| "WAITING_FOR_PROOF", | ||
| "PROOF_AVAILABLE" | ||
| "PROOF_AVAILABLE", | ||
| "TEMPLATE_PROOF_APPROVED" |
There was a problem hiding this comment.
It feels strange having this on the API spec when this isn't a status on the backend. The API will never return or accept this status. I know its used to generate the types. But I wonder if there's a that the status change can be shifted further left or further right.
Further left would be something like we make the API return this status (we wouldn't have to change in the database). Maybe we update the Zod parser so that it transforms the status from SUBMITTED to TEMPLATE_PROOF_APPROVED.
Or further right would be something like ... I will comment in the appropriate place 😅
There was a problem hiding this comment.
I added a comment in utils/utils/src/enum.ts with the shift-right option. I think I prefer that approach rather than shift-left.
utils/utils/src/enum.ts
Outdated
|
|
||
| const templateStatusToDisplayMappingsLetter = (status: TemplateStatus) => | ||
| statusToDisplayMappings[status]; | ||
| const isProofEmpty = ( |
There was a problem hiding this comment.
This returns true if template.files.proofs in non-empty, right?
Should this be called isProofAvailable instead?
utils/utils/src/enum.ts
Outdated
| ? templateStatusToDisplayMappingsLetter( | ||
| template.templateStatus, | ||
| isRoutingEnabled, | ||
| isProofEmpty(template) |
There was a problem hiding this comment.
Shift right option:
You could get rid of templateStatusToDisplayMappingsDigital and templateStatusToDisplayMappingsLetter, you could rework statusToDisplayMapping like this:
It'd mean that you don't need to introduce the TEMPLATE_PROOF_APPROVED status on the open api spec and into the type system.
export const statusToDisplayMapping = (
template: TemplateDto,
isRoutingEnabled: boolean = false
): string => {
const statusToDisplayMappings: Record<TemplateStatus, string> = {
NOT_YET_SUBMITTED:
template.templateType === 'LETTER' ? 'Not yet submitted' : 'Draft',
SUBMITTED:
template.templateType === 'LETTER' &&
isRoutingEnabled &&
isProofAvailable(template)
? 'Template proof approved'
: 'Submitted',
DELETED: '', // will not be shown in the UI
PENDING_PROOF_REQUEST: 'Files uploaded',
PENDING_UPLOAD: 'Checking files',
PENDING_VALIDATION: 'Checking files',
VALIDATION_FAILED: 'Checks failed',
VIRUS_SCAN_FAILED: 'Checks failed',
WAITING_FOR_PROOF: 'Waiting for proof',
PROOF_AVAILABLE: 'Proof available',
} as const;
return statusToDisplayMappings[template.templateStatus];
};3 participants
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.