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.

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

17468 Misc cleanup and prep for xpro aml #683

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

severinbeauvais
Copy link
Collaborator

@severinbeauvais severinbeauvais commented Sep 12, 2023

Issue #: bcgov/entity#17468

Note that xpro aml is BROKEN (not yet complete). All pre-existing flows should be OK.

Description of changes:

  • app version = 5.0.43
  • misc cleanup
  • renamed EntityType -> EntityTypes
  • renamed CompanyType -> CompanyTypes
  • cleaned up PickEntityOrConversion.vue
  • added category table for xpro (amalgamation only)
  • moved many NrApprovedGrayBox props into component
  • renamed isSupportedEntity -> isSupportedIncorporationRegistration
  • changed isXProCompany -> isXproEntityType
  • added Alter Now button to NrApprovedGrayBox
  • added template blocks for amalgamation to Search
  • renamed companyRadioBtnApplicable -> showCompanyTypeRadioButtons
  • added amalgamation form URL to Search
  • added AML blurbs to Entity Types Data
  • added "Extraprovincial" to all xpro business labels
  • added XCR and XCP to AML xpro mapping
  • renamed isSupportedEntity -> isSupportedEntityType
  • added isSupportedAmalgamation()
  • added some store getters
  • deleted debugging strings

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namerequest license (Apache 2.0).

@severinbeauvais severinbeauvais self-assigned this Sep 12, 2023
<!-- List Tables -->
<template v-else-if="!showSocietiesInfo">
<v-card-text class="">
<!-- Special case for societies -->
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Sep 12, 2023

Choose a reason for hiding this comment

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

I simply moved this up from below.

(I think it's easier to understand "if true else everything else" instead of "if not true ...".)

this.setEntityTypeCd(value)
}

get locationText (): string {
return (this.getLocationText === 'BC') ? 'British Columbia' : this.getLocationText
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turns out the store getter getLocationText already returns "British Columbia", so this computed is not needed.

const cols = this.tableData.length
const maxThreshold = this.$vuetify.breakpoint.thresholds.sm
const val = (210 * cols > maxThreshold) ? maxThreshold : (210 * cols)
return `${val}px`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just cleanup.

Copy link
Collaborator

Choose a reason for hiding this comment

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

When you added the .sm, you're targeting the threshold for small screens?

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 didn't actually change this code; I just cleaned it up.

But, yes, this.$vuetify.breakpoint.thresholds.sm is the value (960) for "small" screens.

:showAlterNowButton="showAlterNowButton"
:isAllowAlterOnline="isAlterOnline(nr.requestTypeCd)"
:showOpenExternalIcon="showOpenExternalIcon"
:showGoToSocietiesButton="showGoToSocietiesButton"
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 moved these into the sub-component (along with their supporting functions).

- misc cleanup
- renamed EntityType -> EntityTypes
- renamed CompanyType -> CompanyTypes
- cleaned up PickEntityOrConversion.vue
- added category table for xpro (amalgamation only)
- moved many NrApprovedGrayBox props into component
- renamed isSupportedEntity -> isSupportedIncorporationRegistration
- changed isXProCompany -> isXproEntityType
- added Alter Now button to NrApprovedGrayBox
- added template blocks for amalgamation to Search
- renamed companyRadioBtnApplicable -> showCompanyTypeRadioButtons
- added amalgamation form URL to Search
- added AML blurbs to Entity Types Data
- added "Extraprovincial" to all xpro business labels
- added XCR and XCP to AML xpro mapping
- renamed isSupportedEntity -> isSupportedEntityType
- added isSupportedAmalgamation()
- added some store getters
- deleted debugging strings
- removed debugging code
<!-- Xpro/Federal bullets -->
<v-col v-if="isXproFlow && isFederal" cols="12" :md="isSelectedCompanyXPro ? 10 : 8">
<ul class="bullet-points">
<li>Federally incorporated businesses do not need a Name Request.</li>
<li v-if="isRestoration && isSelectedXproAndRestorable">

<li v-if="isAmalgamation">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's a bit of new amalgamation code in this PR but it shouldn't affect the other flows. I'm preparing here for my next refactoring PR...

@@ -770,27 +777,29 @@ export default class Search extends Mixins(CommonMixin, NrAffiliationMixin) {
}
}

/** Returns whether the selected XPRO is restorable. */
/** Whether the selected XPRO is restorable. */
get isSelectedXproAndRestorable (): boolean {
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Sep 12, 2023

Choose a reason for hiding this comment

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

These are getters. They don't "return" anything -- they "are" something.

(FYI, getters aka computed are evaluated the first time they're needed, then they're cached until something in them changes, ie they are reactive. So they can be faster than a function!)

@@ -888,6 +897,7 @@ export default class Search extends Mixins(CommonMixin, NrAffiliationMixin) {

// set default location for requests where there is only one location option
if (this.isNewBcBusiness || this.isContinuationIn || this.isConversion || this.isAmalgamation) {
// *** TODO: set location for amalgamation later (depending on entity type selected)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A note to myself for upcoming xpro amalgamation flow.

export * from './company-type'
export * from './entity-type'
export * from './company-types'
export * from './entity-types'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

First, I prefer enums to be plural (since they are a list of values -- arrays should be plural).

Second, in my next PR, I will have a couple of components named CompanyType and EntityType, so this resolves the name conflict.

@@ -235,13 +235,16 @@ export const EntityTypesBcData: EntityI[] = [
'Has name protection in BC',
'Must use Societies Online to register a name and incorporate'
],
amlBlurbs: [
'Society amalgamation'
],
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Sep 12, 2023

Choose a reason for hiding this comment

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

Without this, you get a blank tooltip.

In Dev:

image

is extraprovincially registered in BC.`
]
`Register an amalgamation or merger that occurred in another jurisdiction where at least one of the companies
is extraprovincially registered in BC.`
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Sep 12, 2023

Choose a reason for hiding this comment

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

We can't distinguish between a Canadian or International amalgamation, so we don't need this array and I combined the verbiage.

Screenshot
image

@@ -280,8 +283,8 @@ export const EntityTypesBcData: EntityI[] = [

export const EntityTypesXproData: EntityI[] = [
{
text: 'Limited Company',
value: EntityType.XCR,
text: 'Extraprovincial Limited Company',
Copy link
Collaborator Author

@severinbeauvais severinbeauvais Sep 12, 2023

Choose a reason for hiding this comment

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

I renamed all the xpro text per UI designs.

See screenshot attached to comment above for example.

]
}

export const XproMapping: MappingI = {
AML: [
EntityTypes.XCR,
EntityTypes.XCP
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just these 2 types for xpro aml.

@severinbeauvais
Copy link
Collaborator Author

/gcbrun

@severinbeauvais severinbeauvais marked this pull request as ready for review September 12, 2023 16:32
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://namerequest-dev--pr-683-scw91tbu.web.app

@severinbeauvais severinbeauvais changed the title 17468 Misc cleanup 17468 Misc cleanup and prep for xpro aml Sep 12, 2023
</v-col>
</v-row>
</v-container>
<!-- Category tables - xpro (amalgamation only) -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the second block for amalgamations only. Note that it displays Xpro data directly. (The block above, for amalgamations, displayed BC data.)

See screenshot attached to comment above for example.

@@ -194,24 +227,25 @@ export default class PickEntityOrConversionDialog extends CommonMixin {
return output
}

get width (): string {
if (this.showSocietiesInfo || this.isConversion) {
get maxWidth (): string {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed for double-wide table layout.

See screenshot attached to comment above for example.

}

/** True if the Incorporate button should be shown. */
get showIncorporateButton (): boolean {
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 it's awesome that these getters are now here instead.

src/components/new-request/search.vue Show resolved Hide resolved
Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

LGTM Sev! 👍

Copy link
Collaborator

@ketaki-deodhar ketaki-deodhar left a comment

Choose a reason for hiding this comment

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

Looks Good!

@severinbeauvais severinbeauvais merged commit 90744cb into bcgov:main Sep 12, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants