[BUG][NRPTI-1233] Amalgamated permits not showing in BCMI #1243
Conversation
|
| @@ -2,7 +2,7 @@ | |||
| name: Bug | |||
| about: An undesirable behaviour that needs correction | |||
| title: '' | |||
| labels: Spike | |||
| labels: Bug | |||
There was a problem hiding this comment.
unrelated minor fix to a github issue template
| @@ -72,7 +72,7 @@ exports.protectedPut = async function (args, res, next) { | |||
|
|
|||
| const sanitizedObj = PutUtils.validateObjectAgainstModel(MineBCMI, incomingObj); | |||
|
|
|||
| if (!sanitizedObj || sanitizedObj === {}) { | |||
| if (!sanitizedObj || Object.keys(sanitizedObj).length === 0) { | |||
There was a problem hiding this comment.
unrelated minor fix. as js compares objects by reference, "sanitizedObj === {}" is always false.
|
sonarcloud is failing due to some var declarations in the migration template and a function we did not write having too high of a "cognitive load". |
| @@ -46,7 +46,9 @@ | |||
| </label> | |||
| <select name="typeCode" id="typeCode" formControlName="typeCode" class="form-control"> | |||
| <option *ngFor="let typeCode of permitTypes" [ngValue]="typeCode"> | |||
| {{ typeCode === 'OGP' ? 'Original Permit' : 'Permit Amendment' }} | |||
| {{ | |||
There was a problem hiding this comment.
I prefer to avoid nested ternary operators. I find they can be confusing to reason about and prefer the clarity of something along the following lines. Not a necessary change though. You'd have to pull it out into a function.
`
const acceptedTypeCodes = ['OGP', 'ALG']
if (acceptedTypeCodes.includes(typeCode)) return 'Permit Amendment';
if (typeCode === 'OGP') return 'Original Permit';
if (typeCode === 'ALG') return 'Amalgamated Permit';
`
| @@ -68,7 +68,9 @@ <h2 class="border-0 mb-0">Edit record</h2> | |||
| <label for="typeCode">Permit Type</label> | |||
| <select name="typeCode" id="typeCode" formControlName="typeCode" class="form-control"> | |||
| <option *ngFor="let typeCode of permitTypes" [ngValue]="typeCode"> | |||
| {{ typeCode === 'OGP' ? 'Original Permit' : 'Permit Amendment' }} | |||
| {{ | |||
There was a problem hiding this comment.
We are repeating here so it could make sense to pull this out into a function referenced above.
| @@ -42,6 +42,8 @@ <h2 class="border-0 mb-0">Record Information</h2> | |||
| ? 'Original Permit' | |||
| : record && record.typeCode && record.typeCode === 'AMD' | |||
| ? 'Permit Amendment' | |||
| : record && record.typeCode && record.typeCode === 'ALG' | |||
| ? 'Amalgamated Permit' | |||
There was a problem hiding this comment.
This ternary could also be addressed if you do decide to pull out a function and go the route of early returns.
| getPermitType(typeCode) { | ||
| switch(typeCode){ | ||
| case 'OGP': | ||
| return 'Permit'; |
There was a problem hiding this comment.
We reference 'Permit' here and 'Original Permit' for the same case it would be best to stick to one name but there may be a reason for that.
Also is there an opportunity to refactor this switch into the function suggested above?
|
acatchpole
deleted the
1233-amalgamated-mines-act-permits-not-showing-in-bcmi
branch
* unrelated minor fixes * Add getPermitType method to PermitUtils * Add support for Amalgamated Permit code * Revert datasource.js * Fix typo * created migration that updates the type field based on the typeCode of the associate records * linting fixes * Replace ternary operators with function calls * Replace ternary operator with function call in add component * moved permitName to utils and refactored to support * removed minePermitNames from individual files as they are now in recordUtils * added minePermitNames to recordUtils * Fix mine permit bugs due to typos * linting fix 3: The Revenge of Lint --------- Co-authored-by: acatchpole <adamantiumite@gmail.com> Co-authored-by: acatchpole <113044739+acatchpole@users.noreply.github.com>
Description
This PR includes the following proposed change(s):