feat(ui): 1534 clearer host dashboard#1693
Conversation
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
|
||
| /** Returns true when an application row is in draft status. */ | ||
| const isDraftApplication = (row: any): boolean => | ||
| row.statusKey === ApplicationStatus.DRAFT || row.status === 'Draft' |
There was a problem hiding this comment.
since the param type is any do we have to worry about undefined values sneaking in
There was a problem hiding this comment.
So Jimmy, I fixed most of the any types. I felt we've been relying on any a lot and it would be nice to take some time and fix all of them. Should be good now.
| /** Redirects the user to payment for a payment due application. */ | ||
| async function handlePayNow (row: any) { | ||
| let paymentToken = row.paymentToken | ||
| if (!paymentToken) { |
There was a problem hiding this comment.
why would a row not have payment token? The fallback is to get the individual application. Is that good fallback mechanism? if it doesnt come up in search applicaiton api call then how would it show up in the get app by id api call
There was a problem hiding this comment.
Makes sense. Fixed, removed this fallback mechanism.
| } | ||
| return registration.header.applications[0].applicationNumber | ||
| return registration.header.applications.some((app: any) => | ||
| app.applicationType === 'renewal' && app.applicationStatus === 'PAYMENT_DUE' |
There was a problem hiding this comment.
same comment here about the type any for the param
There was a problem hiding this comment.
instead of using any do you think its a good idea to fix the types?
There was a problem hiding this comment.
Same comment as before, fixed all these any types. Used interfaces and enums now.
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
dimak1
left a comment
dimak1
left a comment
There was a problem hiding this comment.
Approved and added some non-blocking comments.
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
|
@jimmypalelil All good for me to merge? |
|
|
Temporary Url for review: https://strr-hosts-dev--pr-1693-de1mqi9m.web.app |
4 participants
Issue:
Description of changes:
Registrations Table:
Applications Table:
I noticed also there were no unit tests for those two tables, added those.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the BC Registry and Digital Services BSD 3-Clause License