Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Final Framework Changes/Fixes for Regular Amalgamation #608

Merged
merged 7 commits into from
Jan 8, 2024
Merged

Final Framework Changes/Fixes for Regular Amalgamation #608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a0171ac
Added unit tests for amalgamation views + lots of misc. fixes and cle…
JazzarKarim Jan 2, 2024
7a2d299
Added test line for review and confirm page
JazzarKarim Jan 3, 2024
9615151
More and more fixes + cleanup
JazzarKarim Jan 3, 2024
95309f8
Added return to step for business summary table
JazzarKarim Jan 4, 2024
fc70595
Fixed in response to Sev's comments
JazzarKarim Jan 4, 2024
d8552be
updated comment
JazzarKarim Jan 4, 2024
2ded246
Fixed Amalgamation Statement test after rebase + updated package version
JazzarKarim Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "business-create-ui",
"version": "5.6.25",
"version": "5.6.26",
"private": true,
"appName": "Create UI",
"sbcName": "SBC Common Components",
Expand Down
4 changes: 2 additions & 2 deletions src/components/Amalgamation/BusinessStatus.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export default class BusinessStatus extends Vue {
return 'This business is under limited restoration. It cannot be part of an amalgamation ' +
'unless it is fully restored.'

// case AmlStatuses.ERROR_NEED_BC_COMPANY:
// return 'You must add at least one BC company.'
case AmlStatuses.ERROR_NEED_BC_COMPANY:
Copy link
Collaborator Author

@JazzarKarim JazzarKarim Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke to Mihai about this. We indeed need as it is in the legacy system:
MicrosoftTeams-image (1)

I've tested it, it works:
need one bc company

Please note the changes too in the other files accompanying this.

return 'You must add at least one BC company.'

case AmlStatuses.ERROR_NOT_AFFILIATED:
return 'This business is not affiliated with the currently selected BC Registries account. ' +
Expand Down
6 changes: 4 additions & 2 deletions ...s/Incorporation/IncorporationDateTime.vue → src/components/common/EffectiveDateTime.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
sm="3"
class="pr-4"
>
<label>Incorporation Date and Time</label>
<label>{{ label }}</label>
</v-col>

<v-col
Expand Down Expand Up @@ -157,14 +157,16 @@ import { EffectiveDateTimeIF, FormIF } from '@/interfaces'
import { VuetifyRuleFunction } from '@/types'

@Component({})
export default class IncorporationDateTime extends Mixins(DateMixin) {
export default class EffectiveDateTime extends Mixins(DateMixin) {
// Refs
$refs!: {
dateTimeForm: FormIF
}

@Prop({ default: null }) readonly effectiveDateTime!: EffectiveDateTimeIF

@Prop({ default: 'Incorporation Date and Time' }) readonly label!: string

// Local properties
isImmediate = false
isFutureEffective = false
Expand Down
2 changes: 1 addition & 1 deletion src/enums/amalgamationEnums.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export enum AmlStatuses {
ERROR_FUTURE_EFFECTIVE_FILING,
ERROR_HISTORICAL,
ERROR_LIMITED_RESTORATION,
// ERROR_NEED_BC_COMPANY,
ERROR_NEED_BC_COMPANY,
ERROR_NOT_AFFILIATED,
ERROR_NOT_IN_GOOD_STANDING,
ERROR_XPRO_ULC_CCC,
Expand Down
36 changes: 13 additions & 23 deletions src/mixins/amalgamation-mixin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default class AmalgamationMixin extends Vue {
this.cccMismatch,
this.foreignUnlimited2,
this.xproUlcCcc,
// this.needBcCompany,
this.needBcCompany,
this.foreignHorizontal
]

Expand Down Expand Up @@ -119,7 +119,7 @@ export default class AmalgamationMixin extends Vue {
xproUlcCcc (business: AmalgamatingBusinessIF): AmlStatuses {
if (
business.type === AmlTypes.FOREIGN &&
(!this.isTypeBcUlcCompany || !this.isTypeBcCcc)
(this.isTypeBcUlcCompany || this.isTypeBcCcc)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the logic here is incorrect. I've been playing around with this and noticed it's incorrect.

We want to show this rule when the type is ULC or CCC not the other way around. I've been getting this rule when I was on a Limited company amalgamation with Foreign companies.

Please correct me if I'm wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are correct (as per the comment on line 118) 👍

Thanks for playing around and verifying stuff :)

) {
return AmlStatuses.ERROR_XPRO_ULC_CCC
}
Expand All @@ -138,17 +138,13 @@ export default class AmalgamationMixin extends Vue {
return null
}

// NOT CURRENTLY USED
// /**
// * Disallow only foreign businesses (including EPs).
// * (An amalgamation where all TINGs are foreign will be Phase 2.)
// */
// needBcCompany (): AmlStatuses {
// if (this.isAllForeignOrEp) {
// return AmlStatuses.ERROR_NEED_BC_COMPANY
// }
// return null
// }
/** Disallow if there are no BC Companies. */
needBcCompany (): AmlStatuses {
if (!this.isAnyBcCompany) {
return AmlStatuses.ERROR_NEED_BC_COMPANY
}
return null
}

/** Disallow if foreign in a short-form horizontal amalgamation. */
foreignHorizontal (business: AmalgamatingBusinessIF): AmlStatuses {
Expand Down Expand Up @@ -205,9 +201,8 @@ export default class AmalgamationMixin extends Vue {
async refetchAmalgamatingBusinessesInfo (): Promise<void> {
const fetchTingInfo = async (item: any): Promise<AmalgamatingBusinessIF> => {
const tingBusiness = await this.fetchAmalgamatingBusinessInfo(item)
// *** TODO: need to improve this
// (no auth info means not affiliated, which may or may not be foreign)
if (!tingBusiness.authInfo) {
// no auth info and business info means foreign, otherwise LEAR (affiliated or non-affiliated)
if (!tingBusiness.authInfo && !tingBusiness.businessInfo) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I've tested this extensively.

We have two cases now. If the tingBusiness does not have both authInfo and businessInfo, then it's a Foreign. If it does, then it's LEAR. However, please see next comment.

return {
type: AmlTypes.FOREIGN,
role: AmlRoles.AMALGAMATING,
Expand All @@ -221,9 +216,9 @@ export default class AmalgamationMixin extends Vue {
role: AmlRoles.AMALGAMATING,
identifier: tingBusiness.businessInfo.identifier,
name: tingBusiness.businessInfo.legalName,
email: tingBusiness.authInfo.contacts[0].email,
email: tingBusiness.authInfo?.contacts[0].email || null,
legalType: tingBusiness.businessInfo.legalType,
address: tingBusiness.addresses.registeredOffice.mailingAddress,
address: tingBusiness.addresses?.registeredOffice.mailingAddress || null,
isNotInGoodStanding: (tingBusiness.businessInfo.goodStanding === false),
isFutureEffective: (tingBusiness.firstFiling.isFutureEffective === true),
isLimitedRestoration: await this.isLimitedRestoration(tingBusiness)
Expand All @@ -241,11 +236,6 @@ export default class AmalgamationMixin extends Vue {
// (not all are used atm)
//

/** True if all companies in the table are foreign. */
get isAllForeign (): boolean {
return this.getAmalgamatingBusinesses.every(business => (business.type === AmlTypes.FOREIGN))
}

/** True if there a foreign company in the table. */
get isAnyForeign (): boolean {
return this.getAmalgamatingBusinesses.some(business => (business.type === AmlTypes.FOREIGN))
Expand Down
17 changes: 0 additions & 17 deletions src/mixins/filing-template-mixin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,6 @@ export default class FilingTemplateMixin extends Mixins(AmalgamationMixin, DateM
// restore the amalgamating businesses array
if (draftFiling.amalgamationApplication.amalgamatingBusinesses) {
this.setAmalgamatingBusinesses([
// *** TODO: remove static items when they are no longer needed for testing
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obsolete.

// {
// type: AmlTypes.FOREIGN,
// corpNumber: 'XYZ789',
// legalName: 'Ice Cream Sandwich Canada',
// foreignJurisdiction: { region: 'FEDERAL', country: 'CA' },
// role: AmlRoles.AMALGAMATING
// },
// {
// type: AmlTypes.FOREIGN,
// corpNumber: 'ABC123',
// legalName: 'Gingerbread USA',
// foreignJurisdiction: { country: 'US' },
// role: AmlRoles.AMALGAMATING
// },
...draftFiling.amalgamationApplication.amalgamatingBusinesses
])
this.refetchAmalgamatingBusinessesInfo()
Expand All @@ -267,8 +252,6 @@ export default class FilingTemplateMixin extends Mixins(AmalgamationMixin, DateM
}

// restore Office Addresses
// *** TODO: verify whether we need to assign fallback
// *** also fix IAs and registrations the same way?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked into this, and I don't see the benefit of assigning a fallback. Please correct me if I'm wrong.

Copy link
Collaborator

@severinbeauvais severinbeauvais Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was thinking here is that, if the draft filing doesn't contain office addresses, we'd assign a safe fallback value (eg, empty object?). Similarly to what we do if don't have parties or shareClasses to restore.

However, theoretically the store has safe initial values for offices / parties / shareClasses, so the correct code is "set the value in the store only if the draft contains the value", which is what lines 272-273 already do.

But I'm not certain that our code is consistently doing the right thing everywhere. For example:

image

You'd think the fallback to an empty array wouldn't be needed, but in fact parties is initialized to null in the store. It should probably be initialized to an empty array, but I don't know if that will break other filings.

So, to move forward with this, either we try to fix the code now and test well, or we ignore this for now and test well (this is safe but won't resolve the code issues). 🤷‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes a lot of sense. Sev, I'll be tackling and looking into this more in next commit.

if (draftFiling.amalgamationApplication.offices) {
this.setOfficeAddresses(draftFiling.amalgamationApplication.offices)
}
Expand Down
2 changes: 2 additions & 0 deletions src/store/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,10 +1270,12 @@ export const useStore = defineStore('store', {
/** Adds specified item to end of amalgamating businesses list. */
pushAmalgamatingBusiness (item: AmalgamatingBusinessIF) {
this.stateModel.amalgamation.amalgamatingBusinesses.push(item)
if (!this.stateModel.ignoreChanges) this.stateModel.haveChanges = true
},
/** Deletes item at specified index from amalgamating businesses list. */
spliceAmalgamatingBusiness (index: number) {
this.stateModel.amalgamation.amalgamatingBusinesses.splice(index, 1)
if (!this.stateModel.ignoreChanges) this.stateModel.haveChanges = true
},
setAmalgamatingBusinessesValid (valid: boolean) {
this.stateModel.amalgamation.amalgamatingBusinessesValid = valid
Expand Down
2 changes: 1 addition & 1 deletion src/views/AmalgamationRegular/PeopleRoles.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default class AmalgamationRegularPeopleRoles extends Mixins(CommonMixin)

@Watch('$route')
private async scrollToInvalidComponent (): Promise<void> {
if (this.getShowErrors && this.$route.name === RouteNames.INCORPORATION_PEOPLE_ROLES) {
if (this.getShowErrors && this.$route.name === RouteNames.AMALG_REG_PEOPLE_ROLES) {
// scroll to invalid components
await this.$nextTick()
await this.validateAndScroll(
Expand Down
14 changes: 7 additions & 7 deletions src/views/AmalgamationRegular/ReviewConfirm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

<!-- Amalgamating Businesses Information -->
<v-card
id="people-and-roles-vcard"
id="amalgamating-businesses-information-vcard"
flat
class="mt-6"
>
Expand Down Expand Up @@ -97,10 +97,11 @@
</p>
</header>

<IncorporationDateTime
<EffectiveDateTime
class="mt-6"
severinbeauvais marked this conversation as resolved.
Show resolved Hide resolved
:class="{ 'invalid-section': isEffectiveDateTimeInvalid }"
:effectiveDateTime="getEffectiveDateTime"
label="Amalgamation Date and Time"
@valid="setEffectiveDateTimeValid($event)"
@effectiveDate="setEffectiveDate($event)"
@isFutureEffective="setIsFutureEffective($event)"
Expand All @@ -115,7 +116,7 @@
<header>
<h2>Document Delivery</h2>
<p class="mt-4">
Copies of the incorporation documents will be sent to the email addresses listed below.
Copies of the amalgamation documents will be sent to the email addresses listed below.
</p>
</header>

Expand Down Expand Up @@ -270,7 +271,7 @@
import { Component, Vue } from 'vue-property-decorator'
import { Action, Getter } from 'pinia-class'
import { useStore } from '@/store/store'
import { ContactPointIF, CertifyIF, EffectiveDateTimeIF, IncorporationAgreementIF, ShareStructureIF,
import { ContactPointIF, CertifyIF, EffectiveDateTimeIF, ShareStructureIF,
CourtOrderStepIF } from '@/interfaces'
import CardHeader from '@/components/common/CardHeader.vue'
import Certify from '@/components/common/Certify.vue'
Expand All @@ -279,7 +280,7 @@ import { DocumentDelivery } from '@bcrs-shared-components/document-delivery'
import FolioNumber from '@/components/common/FolioNumber.vue'
import BusinessTableSummary from '@/components/Amalgamation/BusinessTableSummary.vue'
import AmalgamationStatement from '@/components/Amalgamation/AmalgamationStatement.vue'
import IncorporationDateTime from '@/components/Incorporation/IncorporationDateTime.vue'
import EffectiveDateTime from '@/components/common/EffectiveDateTime.vue'
import ListPeopleAndRoles from '@/components/common/ListPeopleAndRoles.vue'
import ListShareClass from '@/components/common/ListShareClass.vue'
import SummaryDefineCompany from '@/components/common/SummaryDefineCompany.vue'
Expand All @@ -295,7 +296,7 @@ import { CorpTypeCd, GetCorpFullDescription } from '@bcrs-shared-components/corp
CourtOrderPoa,
DocumentDelivery,
FolioNumber,
IncorporationDateTime,
EffectiveDateTime,
ListPeopleAndRoles,
ListShareClass,
SummaryDefineCompany,
Expand All @@ -313,7 +314,6 @@ export default class AmalgamationRegularReviewConfirm extends Vue {
@Getter(useStore) getEntityType!: CorpTypeCd
@Getter(useStore) getFolioNumber!: string
@Getter(useStore) getFolioNumberValid!: boolean
@Getter(useStore) getIncorporationAgreementStep!: IncorporationAgreementIF
@Getter(useStore) getUserEmail!: string
@Getter(useStore) getValidateSteps!: boolean
@Getter(useStore) isPremiumAccount!: boolean
Expand Down
16 changes: 8 additions & 8 deletions src/views/AmalgamationRegular/ShareStructure.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<h2>1. Create Your Authorized Share Structure</h2>
</header>

<p id="incorporation-share-structure">
<p id="amalgamation-regular-share-structure">
Add at least one class of shares. A share class consists of the name of the class, the
maximum number of shares in the class (including any associated Series), a par value for
the class, and the currency the shares are valued in.
Expand Down Expand Up @@ -35,7 +35,7 @@
</header>

<p>
An incorporated business must issue shares. These shares represent ownership interest in
An amalgamated business must issue shares. These shares represent ownership interest in
the company and give the shareholder a say in how the company is being run. For most
small companies starting out, a simple share structure with just one class of shares (and
no series) is typical.
Expand Down Expand Up @@ -66,11 +66,11 @@
The staff at the Corporate Registry cannot provide advice on how to set up your company's
share structure. If you do not understand what an authorized share structure is or what
its purpose is or believe you need a more complex share structure, you should seek
professional advice or purchase an incorporation guide for detailed information and
professional advice or purchase an amalgamation guide for detailed information and
infrastructure on establishing an authorized share structure.
</p>
<p>
Refer to this <a :href="helpLink">link</a> to obtain more information on incorporating a company.
Refer to this <a :href="helpLink">link</a> to obtain more information on amalgamating a company.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke to Janis about the wording and link here. She says it's OK here.

</p>
<u
class="help-btn"
Expand Down Expand Up @@ -192,7 +192,7 @@ export default class AmalgamationRegularShareStructure extends Mixins(CommonMixi
}]

readonly helpLink = 'https://www2.gov.bc.ca/gov/content/employment-business/business/' +
'managing-a-business/permits-licences/businesses-incorporated-companies'
'managing-a-business/permits-licences/businesses-incorporated-companies/incorporated-companies#amalgamate'

showShareStructureForm = false
currentShareStructure = null as ShareClassIF
Expand Down Expand Up @@ -316,14 +316,14 @@ export default class AmalgamationRegularShareStructure extends Mixins(CommonMixi

@Watch('$route')
private async scrollToInvalidComponent (): Promise<void> {
if (this.getShowErrors && this.$route.name === RouteNames.INCORPORATION_SHARE_STRUCTURE) {
if (this.getShowErrors && this.$route.name === RouteNames.AMALG_REG_SHARE_STRUCTURE) {
// scroll to invalid components
await this.$nextTick()
await this.validateAndScroll(
{
shareStructure: this.getCreateShareStructureStep.valid
},
['incorporation-share-structure']
['amalgamation-regular-share-structure']
)
}
}
Expand All @@ -349,7 +349,7 @@ li {
padding-top: 0.25rem;
}

p{
p {
padding-top: 0.5rem;
}

Expand Down
6 changes: 3 additions & 3 deletions src/views/Incorporation/IncorporationReviewConfirm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
</p>
</header>

<IncorporationDateTime
<EffectiveDateTime
class="mt-6"
:class="{ 'invalid-section': isEffectiveDateTimeInvalid }"
:effectiveDateTime="getEffectiveDateTime"
Expand Down Expand Up @@ -246,7 +246,7 @@ import CardHeader from '@/components/common/CardHeader.vue'
import Certify from '@/components/common/Certify.vue'
import { CourtOrderPoa } from '@bcrs-shared-components/court-order-poa'
import { DocumentDelivery } from '@bcrs-shared-components/document-delivery'
import IncorporationDateTime from '@/components/Incorporation/IncorporationDateTime.vue'
import EffectiveDateTime from '@/components/common/EffectiveDateTime.vue'
import ListPeopleAndRoles from '@/components/common/ListPeopleAndRoles.vue'
import ListShareClass from '@/components/common/ListShareClass.vue'
import SummaryDefineCompany from '@/components/common/SummaryDefineCompany.vue'
Expand All @@ -262,7 +262,7 @@ import StaffPayment from '@/components/common/StaffPayment.vue'
Certify,
CourtOrderPoa,
DocumentDelivery,
IncorporationDateTime,
EffectiveDateTime,
ListPeopleAndRoles,
ListShareClass,
SummaryDefineCompany,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/AgreementType.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Vue from 'vue'
import { wrapperFactory } from '../jest-wrapper-factory'
import { wrapperFactory } from '../vitest-wrapper-factory'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the name here as that's been done in other UIs.

import AgreementType from '@/components/common/AgreementType.vue'
import { CorpTypeCd, GetCorpFullDescription } from '@bcrs-shared-components/corp-type-module'

Expand Down
Loading
Loading