-
Notifications
You must be signed in to change notification settings - Fork 7
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
PRC-921: map new Tantalis statuses + clean up comment periods + update unit tests + update status and subpurpose constants #323
Conversation
case 'ACTIVE': | ||
case 'COMPLETED': | ||
case 'DISPOSITION IN GOOD STANDING': | ||
case 'EXPIRED': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is an expired application really approved? Or is this the retired status, where it was completed, but now is too old?
Also, is there some place where the meaning of these statuses is listed? Or is that a question for the business/POs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quick answer is yes, Expired counts as Decision Approved.
I can't speak to the business directly but Lena did and documented everything in PRC-859 (esp the Tantalis Status Mapping doc).
case this.OFFERED: return this.applicationStatuses[this.OFFERED]; | ||
case this.SUSPENDED: return this.applicationStatuses[this.SUSPENDED]; | ||
case this.UNKNOWN: return this.applicationStatuses[this.UNKNOWN]; | ||
getTantalisStatus(statusCode: string): Array<string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this method also have a case for UNKNOWN?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's not needed -- the switch will fall through and return Null. (It's the opposite of getStatusCode() which returns UNKNOWN if it's a status we don't know/support.) We wouldn't know what Tantalis status(es) to map UNKNOWN to so we don't try. Basically it's an unsupported status for us.
*** Since you mentioned it, I added case UNKNOWN, which explicitly returns null (ie, so it doesn't look like a case was accidentally omitted).
@@ -108,6 +108,7 @@ export class Constants { | |||
}; | |||
|
|||
public static readonly statuses = [ | |||
'ABANDONED', // may not be an actual status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are we unsure of in regards to this status? Is it something that comes from Tantalis (I thought abandoned was a made up internal status that wraps a couple real tantalis statuses)? Unless this is totally unrelated to the Abandoned status in Application service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the status we get from Tantalis/BCGW, not to be confused with our internal Abandoned status. Basically, we couldn't get a straight answer on whether this is an actual Tantalis status or not, so it's in the list and we handle it, but we may never see it. Better safe than sorry, plus a comment to say what we've done :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments that may or may not need changes, but otherwise good to go.
…e unit tests + update status and subpurpose constants
No description provided.