argush3 commented 1 year ago

To Do List:

[x] Display applicable warnings. These are now alerts in their own section above the Todo list.
[x] Inform client/staff about missing mandatory business info in a To Do list. Only staff will see the conversion todo item, which is where the warning messsages are listed.
[x] Verify that regular users can't do anything when there is a new or draft conversion filing.

Questions:

Do we need a Conversion To Do list populated and displayed when there is missing required business info. Yes.
Do we need to lock down all other functionality. No change.
Confirm design. See comments below.

Notes:

When the business endpoint returns a business object with a non-empty warnings array with warningType of "MISSING_REQUIRED_BUSINESS_INFO", the UI should display a conversion to do item for staff. This does not seem be happening currently.

example firms: FM0613561, FM0272480

argush3 commented 1 year ago

cc: @davemck513 @Mihai-QuickSilverDev @jdyck-fw @severinbeauvais

severinbeauvais commented 1 year ago

I only see code to handle compliance warnings, ie, https://github.com/bcgov/business-filings-ui/blob/74fc5d3ff397a7a70347d23c0d83e8604a37c365/src/store/getters.ts#L146

and https://github.com/bcgov/business-filings-ui/blob/74fc5d3ff397a7a70347d23c0d83e8604a37c365/tests/unit/EntityInfo.spec.ts#L224

Also, if a conversion todo item is required then it should be provided by the Legal API in the tasks response -- but I don't see any TodoList handling for it (as in, it ignores it), ie, https://github.com/bcgov/business-filings-ui/blob/74fc5d3ff397a7a70347d23c0d83e8604a37c365/src/components/Dashboard/TodoList.vue#L772

argush3 commented 1 year ago

This is weird. Feels like code has reverted or something.

severinbeauvais commented 1 year ago

I think that, previously, the code did something on all warning, and now it only looks at COMPLIANCE warning.

argush3 commented 1 year ago

now i remember, yes that compliance warning not showing up is correct functionality. the issue is that the UI is not indicating a conversion filing is required.

Mihai-QuickSilverDev commented 1 year ago

Is this the culprit?

Legal api - remove compliance warnings from business endpoint response

13381

From: Argus Chiu @.> Sent: September 9, 2022 3:53 PM To: bcgov/entity @.> Cc: Dinu, Mihai CITZ:EX @.>; Mention @.> Subject: Re: [bcgov/entity] UI no longer flags a business with missing required business info (Issue #13565)

[EXTERNAL] This email came from an external source. Only open attachments or links that you are expecting from a known sender.

now i remember, yes that compliance warning not showing up is correct functionality. the issue is that the UI is not indicating a conversion filing is required.

— Reply to this email directly, view it on GitHubhttps://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fbcgov%2Fentity%2Fissues%2F13565%23issuecomment-1242539942&data=05%7C01%7Cmihai.dinu%40gov.bc.ca%7C670bfc63d8f740f0bc5608da92b5faa5%7C6fdb52003d0d4a8ab036d3685e359adc%7C0%7C0%7C637983607528910499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=rTTzC%2FftnlFKfZAxzrkgluaP3Pvr%2FgnqQ0VSvU%2F%2F%2B2Y%3D&reserved=0, or unsubscribehttps://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAZAUGNJABSFXAD3NHALXLVTV5O5S7ANCNFSM6AAAAAAQI7QTRE&data=05%7C01%7Cmihai.dinu%40gov.bc.ca%7C670bfc63d8f740f0bc5608da92b5faa5%7C6fdb52003d0d4a8ab036d3685e359adc%7C0%7C0%7C637983607529066295%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=91JqIJpDh0cnEoq8Ar%2FVihKJxcbDEHPn%2FiIBl2rktUQ%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>

severinbeauvais commented 1 year ago

The UI does not have code to load a Conversion todo item.

argush3 commented 1 year ago

Relationships did that work to load it though...where did it go? was some kind of decision made to remove that? and just use the record conversion functionality.

severinbeauvais commented 1 year ago

I see no TodoList.vue code to handle a Conversion todo item (and I've gone back to May 24 in the commit history).

argush3 commented 1 year ago

I just remember Faiz working on it

argush3 commented 1 year ago

Is this the culprit? Legal api - remove compliance warnings from business endpoint response #13381

I don't think this is the issue Mihai. I mean we haven't even done the work in #13381

severinbeauvais commented 1 year ago

@Mihai-QuickSilverDev The Legal API correctly provides a "conversion todo" in the tasks response, but the Filings UI does nothing with it.

It could probably be implemented as a 3-pointer. A UI design would be nice but we could copy what we do for the Annual Report todo item.

argush3 commented 1 year ago

@Mihai-QuickSilverDev you should also confirm with the business whether the "conversion todo" is still required.

jdyck-fw commented 1 year ago

Mihai will check with the business on this tomorrow.

severinbeauvais commented 1 year ago

I found this: https://github.com/bcgov/business-filings-ui/blob/3abbe109172f10baa031cc3504254d38d514edae/src/components/Dashboard/TodoList.vue#L671

and this: https://github.com/bcgov/business-filings-ui/blob/main/src/components/Dashboard/ActionRequired.vue

I'm not sure why it's not displaying as Argus expects.

argush3 commented 1 year ago

I think at the time this sort of worked when we were still using the complianceWarning array from the business object. Now we distinguish the warning types and the isNotInCompliance code also checks the warning type which would filter out missing business warning types. I think https://github.com/bcgov/business-filings-ui/blob/3abbe109172f10baa031cc3504254d38d514edae/src/components/Dashboard/TodoList.vue#L671-L674 needs to be updated to something like

  get isActionRequired (): boolean {
    return this.isFirm && this.isNotMissingBusinessInfo
  }

severinbeauvais commented 1 year ago

@argush3 Can you take a shot at listing what needs to be fixed please?

argush3 commented 1 year ago

sure

severinbeauvais commented 1 year ago

There may be crossed-up logic in the Todo List: we started with "not in compliance" and now we're using this for "missing information". @Mihai-QuickSilverDev @argush3 @yuisotozaki Are these the same thing? And only for a firm atm? Or do we need to separate what is displayed for each of these conditions?

If "not in compliance" is now "missing information" then I agree with Argus' proposed solution, ie:

add a new getter, isMissingBusinessInfo, that is similar to isNotInCompliance (which checks for a COMPLIANCE warning) and checks for a MISSING_REQUIRED_BUSINESS_INFO warning
update isActionRequired() to display the ActionRequired component if the business is a firm and is missing business info

Also, we need to implement the Conversion Todo code here. A little UI work would be helpful: what should the title say, the subtitle say, the actions be, should it be staff only?

severinbeauvais commented 1 year ago

Blocked on UI design (Not In Compliance banner + Conversion task display).

cc @yuisotozaki

argush3 commented 1 year ago

@severinbeauvais here's a firm(FM0272480) with more missing business info. believe this firm should be in Dev and Test

"warnings": [
      {
        "code": "NO_BUSINESS_OFFICE",
        "message": "A business office is required.",
        "warningType": "MISSING_REQUIRED_BUSINESS_INFO"
      },
      {
        "code": "NO_BUSINESS_PARTY_MAILING_ADDRESS",
        "message": "Business party mailing address is required.",
        "warningType": "MISSING_REQUIRED_BUSINESS_INFO"
      },
      {
        "code": "NO_COMPLETING_PARTY",
        "message": "A completing party is required.",
        "warningType": "MISSING_REQUIRED_BUSINESS_INFO"
      },
      {
        "code": "NO_START_DATE",
        "message": "A start date is required.",
        "warningType": "MISSING_REQUIRED_BUSINESS_INFO"
      }
    ]

severinbeauvais commented 1 year ago

UI to ignore whatever might be in complianceWarnings array in the todo task
UI to look through warnings array in the todo task for compliance warning type and display appropriate action required card (keep it simple for now since API doesn't return any of these warnings yet)
UI to look through warnings array in the todo task for missing business info and display appropriate action required card
UI to display conversion todo item, with list messages in the details panel (see UI design)
UI to allow conversion filing for staff only

yuisotozaki commented 1 year ago

Designs found.

These designs are out of date and do not take Alert section into account. ~Collapsed: https://invis.io/HN123TTOT95J#/464211674_0-0_SP-DBA-GP_-_TO_DO_Action_Required~ ~Expanded: https://invis.io/HN123TTOT95J#/464211675_0-1_SP-DBA-GP_-_TO_DO_Action_Required_-_Expanded~

Design Reference for the new Alerts section being introduced as part of the Freeze/Unfreeze feature: https://invis.io/TG12Y2HY7XUS#/469515306_03_All_Alerts_Expanded

argush3 commented 1 year ago

@Mihai-QuickSilverDev these are the things the backend is checking to determine if there is missing business info. If any of these are missing, a conversion to do is returned along with these warnings(warning type = missing required business info).

NO_BUSINESS_OFFICE = 'NO_BUSINESS_OFFICE'

NO_BUSINESS_DELIVERY_ADDRESS = 'NO_BUSINESS_DELIVERY_ADDRESS'
NO_PROPRIETOR = 'NO_PROPRIETOR'
NO_PROPRIETOR_PERSON_NAME = 'NO_PROPRIETOR_PERSON_NAME'
NO_PROPRIETOR_ORG_NAME = 'NO_PROPRIETOR_ORG_NAME'
NO_PARTNER = 'NO_PARTNER'
NO_PARTNER_PERSON_NAME = 'NO_PARTNER_PERSON_NAME'
NO_PARTNER_ORG_NAME = 'NO_PARTNER_ORG_NAME'
NO_COMPLETING_PARTY = 'NO_COMPLETING_PARTY'
NO_COMPLETING_PARTY_PERSON_NAME = 'NO_COMPLETING_PARTY_PERSON_NAME'
NO_COMPLETING_PARTY_ORG_NAME = 'NO_COMPLETING_PARTY_ORG_NAME'

NO_BUSINESS_OFFICE_MAILING_ADDRESS = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS'
NO_BUSINESS_OFFICE_MAILING_ADDRESS_STREET = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS_STREET'
NO_BUSINESS_OFFICE_MAILING_ADDRESS_CITY = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS_CITY'
NO_BUSINESS_OFFICE_MAILING_ADDRESS_COUNTRY = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS_COUNTRY'
NO_BUSINESS_OFFICE_MAILING_ADDRESS_POSTAL_CODE = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS_POSTAL_CODE'
NO_BUSINESS_OFFICE_MAILING_ADDRESS_REGION = 'NO_BUSINESS_OFFICE_MAILING_ADDRESS_REGION'

NO_BUSINESS_OFFICE_DELIVERY_ADDRESS = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS'
NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_STREET = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_STREET'
NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_CITY = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_CITY'
NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_COUNTRY = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_COUNTRY'
NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_POSTAL_CODE = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_POSTAL_CODE'
NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_REGION = 'NO_BUSINESS_OFFICE_DELIVERY_ADDRESS_REGION'

NO_BUSINESS_PARTY_MAILING_ADDRESS = 'NO_BUSINESS_PARTY_MAILING_ADDRESS'
NO_BUSINESS_PARTY_MAILING_ADDRESS_STREET = 'NO_BUSINESS_PARTY_MAILING_ADDRESS_STREET'
NO_BUSINESS_PARTY_MAILING_ADDRESS_CITY = 'NO_BUSINESS_PARTY_MAILING_ADDRESS_CITY'
NO_BUSINESS_PARTY_MAILING_ADDRESS_COUNTRY = 'NO_BUSINESS_PARTY_MAILING_ADDRESS_COUNTRY'
NO_BUSINESS_PARTY_MAILING_ADDRESS_POSTAL_CODE = 'NO_BUSINESS_PARTY_MAILING_ADDRESS_POSTAL_CODE'
NO_BUSINESS_PARTY_MAILING_ADDRESS_REGION = 'NO_BUSINESS_PARTY_MAILING_ADDRESS_REGION'

NO_COMPLETING_PARTY_MAILING_ADDRESS = 'NO_COMPLETING_PARTY_MAILING_ADDRESS'
NO_COMPLETING_PARTY_MAILING_ADDRESS_STREET = 'NO_COMPLETING_PARTY_MAILING_ADDRESS_STREET'
NO_COMPLETING_PARTY_MAILING_ADDRESS_CITY = 'NO_COMPLETING_PARTY_MAILING_ADDRESS_CITY'
NO_COMPLETING_PARTY_MAILING_ADDRESS_COUNTRY = 'NO_COMPLETING_PARTY_MAILING_ADDRESS_COUNTRY'
NO_COMPLETING_PARTY_MAILING_ADDRESS_POSTAL_CODE = 'NO_COMPLETING_PARTY_MAILING_ADDRESS_POSTAL_CODE'
NO_COMPLETING_PARTY_MAILING_ADDRESS_REGION = 'NO_COMPLETING_PARTY_MAILING_ADDRESS_REGION'

NO_START_DATE = 'NO_START_DATE'

severinbeauvais commented 1 year ago

^^ If we change some of these checks to return a warning type that includes the word "COMPLIANCE" then we can display the compliance banner instead of, or as well as, the missing info banner.

@Mihai-QuickSilverDev to create a new BE ticket and FE ticket for that if desired. (Currently the BE doesn't label any of these checks with COMPLIANCE, and the FE doesn't handle them differently.)

Mihai-QuickSilverDev commented 1 year ago

@severinbeauvais Could we have all the missing business info in the Conversion To Do list, and then an Alerts tab, which might contain one compliance warning entry, if the proprietor or partner is missing? @argush3

argush3 commented 1 year ago

@Mihai-QuickSilverDev if we want to support compliance warnings, we will need a BE ticket. Specific rules will need to be included. Right now we flag missing names for partners and proprietors and things like if min num of partners is met when doing the required business info check. These details will need to be provided.

yuisotozaki commented 1 year ago

Conversion design updated in UXPin to include Alerts section on the entity dashboard. https://preview.uxpin.com/363a49b33bdb2a314ccac0f18daad8969e93b337#/pages/149272182

severinbeauvais commented 1 year ago

Test Notes

You can use these firms for testing (either as reg user or staff):

https://dev.bcregistry.ca/business/FM0272482 (BCREG0018 or IDIR)
https://dev.bcregistry.ca/business/FM0272479 (BCREG0001 or IDIR)
https://dev.bcregistry.ca/business/FM0613561 (IDIR only - not affiliated)
https://dev.bcregistry.ca/business/FM0272480 (IDIR only)

yuisotozaki commented 1 year ago

A few visual design observations in dev.

font sizes
icon sizes
some spacing (Was the one below the alert title is the one we don't have control over? If so, ignore that one.)

severinbeauvais commented 1 year ago

More Test Notes

This is what has changed:

"View and Change Business Information" button no longer has a warning icon and tooltip when the business is "not in compliance" (where not in compliance means the business has compliance warnings, which is not implemented in the BE).
Businesses with warnings (compliance or missing info) will display an Alerts section above the To Do list. This section will have a "Missing information" or "Not in good standing" item (where not in good standing means the business has compliance warnings, which is not implemented in the BE).
All View/Hide Details buttons in the To Do list are now on the title line instead of the subtitle line.
Businesses with missing required business information (as determined by the BE) will display a record conversion todo item for staff only. The details of this item will show the individual warning messages.
Where there is a missing information alert (and a record conversion todo item if staff), other filings cannot be started -- except staff filings (staff only).
All other rules for what is allowed vs disabled are unchanged. For example, a pending task (in To Do list or Recent Filing History list) will block any other filings.

See PR 407 for some screenshots (or see them in Dev).

bcgov / entity

Filings UI no longer shows conversion todo for business with missing required business info #13565

To Do List:

Questions:

Notes:

13381

Test Notes

More Test Notes