24549 Added missing NR request types #3090
Merged
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
severinbeauvais
force-pushed
the
24549
branch
from
bc3b5ce
to
5a1ee1f
Compare
severinbeauvais
requested review from
vysakh-menon-aot,
leodube-aot and
ArwenQin
|
I'm still working through the unit tests, but please review the code changes and lmk if OK. |
vysakh-menon-aot
approved these changes
Nov 25, 2024
ArwenQin
approved these changes
Nov 25, 2024
severinbeauvais
force-pushed
the
24549
branch
4 times, most recently
from
161a3df
to
5572114
Compare
severinbeauvais
commented
Nov 26, 2024
| @@ -154,7 +190,7 @@ def test_alteration(session, use_nr, new_name, legal_type, new_legal_type, nr_ty | |||
|
|
|||
| f['filing']['alteration']['nameRequest']['nrNumber'] = identifier | |||
| f['filing']['alteration']['nameRequest']['legalName'] = new_name | |||
| f['filing']['alteration']['nameRequest']['legalType'] = legal_type | |||
| f['filing']['alteration']['nameRequest']['legalType'] = new_legal_type | |||
There was a problem hiding this comment.
So... this and line 204, below, were the reasons my initial tests failed 🤦♂️
severinbeauvais
commented
Nov 26, 2024
| @@ -244,7 +302,7 @@ def test_validate_numbered_name(session, test_name, legal_type, new_legal_type, | |||
| # check that validation failed | |||
| assert err | |||
| assert HTTPStatus.BAD_REQUEST == err.code | |||
| assert err.msg[0]['error'] == err_msg | |||
| assert err_msg in err.msg[0]['error'] | |||
There was a problem hiding this comment.
I changed this so we don't have to use the full, 2-line, error messages in the test parameters.
severinbeauvais
force-pushed
the
24549
branch
from
5572114
to
05d9e5c
Compare
|
@vysakh-menon-aot Are you OK with my latest changes (second commit)? |
added 2 commits
November 26, 2024 15:27
- added comments - updated an error message - moved request types together in mapping array - added some missing types - updated unit tests
severinbeauvais
force-pushed
the
24549
branch
from
05d9e5c
to
835e1cf
Compare
|
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.
Issue #: bcgov/entity#24549
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).