19036 Added "must include" checklist + refactored validation #635
Conversation
| @@ -1,29 +1,81 @@ | |||
| <template> | |||
| <div id="amalgamating-businesses"> | |||
| <v-btn | |||
There was a problem hiding this comment.
I recommend you go to the ⚙️ icon and hide whitespace, to better see what I did here.
| <span class="rule-item-txt">Amalgamating businesses</span> | ||
| </li> | ||
| </ul> | ||
| </section> |
There was a problem hiding this comment.
Example for regular amalgamation:
There was a problem hiding this comment.
Example for short-form (horizontal) amalgamation:
| @Getter(useStore) getShowErrors!: boolean | ||
| // @Getter(useStore) isAmalgamationFilingHorizontal!: boolean | ||
| // @Getter(useStore) isAmalgamationFilingRegular!: boolean | ||
| // @Getter(useStore) isAmalgamationFilingVertical!: boolean |
There was a problem hiding this comment.
As in other places, these are imported via some mixins, but I like to have the declarations here anyways.
There was a problem hiding this comment.
See screenshots above for regular/short-form updated layouts.
- added checklist section + computeds - changed Business Table "valid" to "allOk" - refactored Amalgamating Businesses validation - cleaned up Information page layout - fixed bug where holding company is changed to amalgamating - fixed/updated unit tests
severinbeauvais
force-pushed
the
19036
branch
2 times, most recently
from
b04b344
to
4eba431
Compare
| corpNumber: item.corpNumber, | ||
| legalName: item.legalName, | ||
| foreignJurisdiction: item.foreignJurisdiction | ||
| } as AmalgamatingBusinessIF | ||
| } else { | ||
| return { | ||
| type: AmlTypes.LEAR, | ||
| role: AmlRoles.AMALGAMATING, | ||
| role: item.role, // amalgamating or holding |
There was a problem hiding this comment.
Use the role from the item (which is what was restored from the draft).
There was a problem hiding this comment.
Example after fix:
| ] | ||
|
|
||
| for (let i = 0; i < businessTableValidTests.length; i++) { | ||
| it.skip(`computes businessTableValid correctly - test ${i}`, () => { |
There was a problem hiding this comment.
In progress - I will submit another PR.
| await wrapper.setData({ businessTableValid, isAddingAmalgamatingBusiness, isAddingAmalgamatingForeignBusiness }) | ||
| expect(store.stateModel.amalgamation.amalgamatingBusinessesValid).toBe(expected) | ||
| for (let i = 0; i < amalgamatingBusinessesValidTests.length; i++) { | ||
| it.skip(`validates component - test ${i}`, async () => { |
There was a problem hiding this comment.
In progress - I will submit another PR.
|
/gcbrun |
severinbeauvais
requested review from
jamespaologarcia,
ketaki-deodhar,
JazzarKarim and
PaulGarewal
|
Temporary Url for review: https://business-create-dev--pr-635-7gpbp65t.web.app |
* - app version = 5.8.1 - added checklist section + computeds - changed Business Table "valid" to "allOk" - refactored Amalgamating Businesses validation - cleaned up Information page layout - fixed bug where holding company is changed to amalgamating - fixed/updated unit tests * wip --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca>
Issue #: bcgov/entity#19036
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 bcrs-entities-create-ui license (Apache 2.0).