[BUG][NRPTI-1242] Add support for creating & updating permits from multiple permit codes #1248
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LocalNewsTV
reviewed
Jun 27, 2024
Comment on lines
280
to
283
| if (nrptiRecord.name === 'Lumby Mine' && permit.permit_no === 'G-4-352') { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Since the if determines the return, couldn't the return just be the !inverse of the if.
If the intent isn't clear it could be assigned to a variable before the return
There was a problem hiding this comment.
It sure can! The sonarcloud robots agree with you too ;)
Comment on lines
270
to
276
| return permits.filter(p => this.isValidPermit(p,nrptiRecord)); | ||
| } | ||
|
|
||
| isValidPermit(permit,nrptiRecord){ | ||
| //Mine must not be historical which is indicated by an authorized year of '9999' on the latest amendment. | ||
| if((permit.permit_amendments.length && !permit.permit_amendments[0].authorization_end_date) | ||
| || permit.permit_status_code === 'O'){ |
There was a problem hiding this comment.
Just minor formatting inconsistencies to the rest of the file with regards to whitespace
There was a problem hiding this comment.
Good spotting! I did see this when I reviewed the PR too, surprised the linter didn't catch it
|
acatchpole
approved these changes
Jun 27, 2024
sggerard
added a commit
that referenced
this pull request
Jul 17, 2024
…ltiple permit codes (#1248) * Add support for creating permits from multiple permit codes * Fix test method calls * Fix whitespace and replace if-then-else with single return statement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Standards
Description
Previously only the most recent valid permit was selected for a mine and records processed for the permit. However requirements changed and now both M and MX records need to be publicly available. A hotfix hotfix was deployed but did not address the underlying problem leading to syncing issues. This PR attempts to properly implement a solution.
This PR includes the following proposed change(s):
Replaces getMinePermit() with getMinePermits() and separates the logic out into multiple methods so that createMinePermit() and updateMinePermit() can process multiple mine permits instead of only the currently selected valid Mine Permit. This allows both MX and M permits and collections for a single mine to be processed during the cron run.