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

Edit UI: full restoration bugs (date fields, approval type) #16500

Closed
8 tasks done
severinbeauvais opened this issue May 25, 2023 · 27 comments
Closed
8 tasks done

Edit UI: full restoration bugs (date fields, approval type) #16500

severinbeauvais opened this issue May 25, 2023 · 27 comments
Assignees
Labels
bug Something isn't working Entities - Olga A label to filter on the tickets for the Entities based team that Olga is PO for. ENTITY Business Team Priority2

Comments

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented May 25, 2023

This is related to #16081 and #16155.

If the todos below get done quickly, see also bugs in #16079 and #16080.

  • determine if Limited Restoration Conversion to Full is setting dates correctly depending on approval type (eg, registrar)
  • if limited restoration was done by court order, the conversion can only be done by court order
  • check whether approval fields are properly saved and resumed
  • check that validation enforces mandatory fields
  • check that first invalid section is scrolled to
  • check that empty fields match schema (eg, field may need to be undefined if empty)
  • fix any issues
  • update unit tests

UXPin: https://preview.uxpin.com/306da47387722817e24fc77fd71476a1869a05fe#/pages/160541351/simulate/sitemap

@severinbeauvais severinbeauvais added bug Something isn't working ENTITY Business Team labels May 25, 2023
@severinbeauvais
Copy link
Collaborator Author

Comment from Argus on May 4, 2023:


@Mihai-QuickSilverDev @severinbeauvais Just did a test for this in DEV and verified that the UI is not passing the gazette publish and application mailed date that were filled out in the conversion application.

FE Http Request Body

PUT https://legal-api-dev.apps.silver.devops.gov.bc.ca/api/v2/businesses/BC0871238/filings/144874
Note that values were passed for parties and offices in the body payload but replaced with empty list to help with readability.

{
  "filing": {
    "business": {
      "foundingDate": "2022-11-21T08:04:10.034456+00:00",
      "identifier": "BC0871238",
      "legalName": "0871238 B.C. COMMUNITY CONTRIBUTION COMPANY LTD.",
      "legalType": "CC"
    },
    "header": {
      "certifiedBy": "asdf",
      "date": "2023-05-04",
      "folioNumber": "",
      "name": "restoration",
      "priority": false,
      "waiveFees": true
    },
    "restoration": {
      "approvalType": "registrar",
      "business": {
        "identifier": "BC0871238",
        "legalType": "CC"
      },
      "contactPoint": {
        "email": "snikker298@gmail.com",
        "phone": "(911) 033-7861"
      },
      "nameRequest": {
        "legalName": "0871238 B.C. COMMUNITY CONTRIBUTION COMPANY LTD.",
        "legalType": "CC"
      },
      "offices": [], 
      "parties": [],
      "relationships": [
        "Heir or Legal Representative",
        "Officer"
      ],
      "type": "limitedRestorationToFull"
    }
  }
}

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Sep 11, 2024

I may have fixed these issues in #16079.

@severinbeauvais severinbeauvais removed their assignment Sep 11, 2024
@ArwenQin ArwenQin self-assigned this Sep 11, 2024
@ArwenQin
Copy link
Collaborator

@severinbeauvais
For limited restore by Registrar, then convert to full restore, the approval type section shows both types:
image.png
But if I choose by court order, while there shows no error, after I click "file and pay", it shows error saying "must choose registrar"
image.png
Just to confirm is it a bug, or expected? I assume it's a bug.

@ArwenQin
Copy link
Collaborator

@severinbeauvais
Just to confirm, Approval Type approved by registrar, can the date the Notice of the Application and date of mailed be in future?
image.png
Currently, both dates can be in the future.

@severinbeauvais
Copy link
Collaborator Author

@severinbeauvais Just to confirm, Approval Type approved by registrar, can the date the Notice of the Application and date of mailed be in future? Currently, both dates can be in the future.

If you don't see a requirement for this in the UI designs then it's OK.

@severinbeauvais
Copy link
Collaborator Author

@severinbeauvais For limited restore by Registrar, then convert to full restore, the approval type section shows both types:
But if I choose by court order, while there shows no error, after I click "file and pay", it shows error saying "must choose registrar"
Just to confirm is it a bug, or expected? I assume it's a bug.

@Mihai-QuickSilverDev , what is the requirement here -- if the limited restoration was done by registrar (not court order), does the conversion to full restoration have to be done by registrar also, or can the conversion be done by court order?

I think the UI design says that the conversion can be done by court order even if the limited restoration was done by registrar. If this is true then this is a Legal API validation bug (new ticket).

@Mihai-QuickSilverDev
Copy link
Collaborator

Mihai-QuickSilverDev commented Sep 11, 2024

Hi Sev, the only restriction is this:

  • For a Full Restoration - the person requesting the restoration needs to be affiliated in an official position with the company
  • For a Limited Restoration - the person requesting the restoration does not need to be affiliated in an official position with the company

So, going back to your question, if a company was limited restored by Registrar, that restoration could be converted to a Full one by Court Order, and vice-versa.

@severinbeauvais
Copy link
Collaborator Author

So, going back to your question, if a company was limited restored by Registrar, that restoration could be converted to a Full one by Court Order, and vice-versa.

According to the UI design, if the limited restoration was done by Court Order then the conversion must also be done by Court Order. Can you confirm this?

However, it seems accurate that, if the limited restoration was done by Registrar then the conversion could be done by Registrar or Court Order. Because of this, I think we need that Legal API validation ticket. Arwen, can you please create it?

@ArwenQin
Copy link
Collaborator

ArwenQin commented Sep 11, 2024

Because of this, I think we need that Legal API validation ticket. Arwen, can you please create it?

@severinbeauvais
Yes, I just created # 23251

@ArwenQin
Copy link
Collaborator

ArwenQin commented Sep 11, 2024

I verified the following:

Full Restore Application:

Limited Restore Application, by court order:

  • limited restoration was done by court order, the conversion can only be done by court order
  • approval fields are properly saved and resumed
  • checked that validation enforces mandatory fields
  • first invalid section is scrolled to
  • Bug to be fixed: Section 2 Limited Restoration time, when choose " __month" but didn't enter the number, the error didn't show (see below)

Limited Restore Application, by Registrar:

Limited Restoration Conversion to Full

  • approval fields are properly saved and resumed
  • checked that validation enforces mandatory fields
  • first invalid section is scrolled to
  • Bug to be fixed: if we edited person, save and resume, the undo is not working (see below)

Test Limited Restoration Extension - verified, no bug
View wrapper component - verified, no bug

  • Schema: The person schema doesn't have taxId, while the UI files them; the middleName (UI) doesn't match middleInitial (schema) - ok

Will fix the 2 small bugs mentioned above in this ticket:

  1. In the Limited Restoration Application Page, Section 2 Limited Restoration time,
    when choose " __month" but didn't enter the number, the error didn't show, all fields are green

image.png

@severinbeauvais This bug is in create UI, do we fix it here?
  1. Conversion to Full Restoration or Limited Restoration Extension page, when we edited the Applicant Person's information, click "Save and Resume", the "Undo" function doesn't work anymore

image.png

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Sep 11, 2024

Sounds good.

For the first bug, you'll need to fix the component in the bcrs-shared-components repo. I can help you publish it. see next comment

@severinbeauvais
Copy link
Collaborator Author

I am already working on the first bug, in Create UI, in #16700.

Try to fix the second bug in this ticket. Thanks!

@ArwenQin
Copy link
Collaborator

  1. The Edit UI bug mentioned above is fixed in this PR: 16500 - Fixed Restoration People Section Bug business-edit-ui#588
  2. I found another 2 bugs, which I believe are with Legal API issue:
  • For Limited Restoration Extension/Conversion, if we replace the original person/business with a new business, the process fails

image.png

File and Pay process fails:
image.png

  • If we remove the original person/business and replace it with a new person - processed successfully.
    However, after it, when we open a new extension/conversion file, the new added person is not updated, and the removed old person is still there.

image.png

@severinbeauvais

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Sep 12, 2024

@Mihai-QuickSilverDev Please look at Arwen's findings above regarding the replacement of the applicant in a Limited Restoration Extension (which was pre-populated from the limited restoration filing). Is it allowed? wait, I'm still trying to understand what's going on; no need to answer this

deleted obsolete comment

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Sep 12, 2024

SB's analysis

The first bug is a schema validation error. Arwen, you'll have to compare the filing JSON with the appropriate schemas to see what's invalid.

The second bug is a back end error. Arwen will create a new bug ticket for it.

@ArwenQin
Copy link
Collaborator

New bug ticket created for the back end error (remove and add person):
https://app.zenhub.com/workspaces/entities---olga-65af15f59e89f5043c2911f7/issues/gh/bcgov/entity/23277

@ArwenQin
Copy link
Collaborator

image.png

And the middleName (UI) doesn't match middleInitial (schema), but there is a note:

image.png

Not sure whether we should update the schema?

@severinbeauvais

@severinbeauvais
Copy link
Collaborator Author

  • For the schema, I found the taxId and identifier are not in the Person schema, while we have it in the filing JSON

@vysakh-menon-aot , do you think we should add optional taxId and identified in the schema?

And the middleName (UI) doesn't match middleInitial (schema), but there is a note:

@vysakh-menon-aot , the UI saves "middleName" instead of "middleInitial". Does the BE convert it? (I think if we convert it in the UI then a lot of other UI code will need to be updated to convert it back and forth on draft resume.)

@ArwenQin
Copy link
Collaborator

@vysakh-menon-aot
CC: @severinbeauvais
Hi Vysakh, could you please try to reproduce this bug:
file a correction to an IA, try to change the directors (add one, edit, or remove one). After file the correction, will you see the incorporator showing in the director's section?
image.png
Please see the unnamed incorporator. It seems impossible, the incorporator shouldn't show in the director's page. This bug may be hard to reproduce.
And I was able to delete the incorporator from that section for BC1218838. Could you please check in the database?
Thanks.

@vysakh-menon-aot
Copy link
Collaborator

vysakh-menon-aot commented Sep 17, 2024

  • For the schema, I found the taxId and identifier are not in the Person schema, while we have it in the filing JSON

@vysakh-menon-aot , do you think we should add optional taxId and identifier in the schema?

there is no logic around taxid in party (can keep it in the json without adding to schema). identifier is already part of schema (https://github.com/bcgov/business-schemas/blob/main/src/registry_schemas/schemas/party.json#L34).

And the middleName (UI) doesn't match middleInitial (schema), but there is a note:

@vysakh-menon-aot , the UI saves "middleName" instead of "middleInitial". Does the BE convert it? (I think if we convert it in the UI then a lot of other UI code will need to be updated to convert it back and forth on draft resume.)

Currently we support both middleName and middleInitial in party object (event though only middleInitial in schema) while creating/updating parties (entity-filer), I don't remember if we have a ticket to change this. If we are changing it to middleInitial we need to verifiy if its showing in all scenarios

@vysakh-menon-aot
Copy link
Collaborator

@vysakh-menon-aot CC: @severinbeauvais Hi Vysakh, could you please try to reproduce this bug: file a correction to an IA, try to change the directors (add one, edit, or remove one). After file the correction, will you see the incorporator showing in the director's section? image.png Please see the unnamed incorporator. It seems impossible, the incorporator shouldn't show in the director's page. This bug may be hard to reproduce. And I was able to delete the incorporator from that section for BC1218838. Could you please check in the database? Thanks.

I cannot reproduce

@ArwenQin
Copy link
Collaborator

@vysakh-menon-aot

I cannot reproduce

Thanks a lot!
We are wondering it might be that someone did something weird in the Dev db. Do you think if changing something in the db could lead to this bug, or whether it has to be a logic bug.

@vysakh-menon-aot
Copy link
Collaborator

If its a draft filing (which is in json) backend will not modify the data

@ArwenQin
Copy link
Collaborator

#23343 created for the bug that business name didn't show in the Restoration application/extension output

@ArwenQin
Copy link
Collaborator

Test note:
Fixed 3 bugs in this ticket:

  1. Conversion to Full Restoration or Limited Restoration Extension page, when we edited the Applicant Person's information, click "Save and Resume", then go back - the "Undo" function doesn't work anymore
    Fixed this bug by removing the re-assign officer ID.
  2. Fixed the bug that removed and added person are not updated, previously in ticket
    https://app.zenhub.com/workspaces/entities---olga-65af15f59e89f5043c2911f7/issues/gh/bcgov/entity/23277
    (verified ticket 23277)
  3. Fixed the bug when the person is a business/corporation, the file and pay fails (fixed the party schema)

@severinbeauvais
Copy link
Collaborator Author

I cannot reproduce

@vysakh-menon-aot Can you think of any db or logic situation that could have caused this? Both Arwen and I saw the Incorporator (albeit with no name) in the director list in a correction.

@severinbeauvais
Copy link
Collaborator Author

As for the incorporator displaying in the list of directors, see ticket #23395.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Entities - Olga A label to filter on the tickets for the Entities based team that Olga is PO for. ENTITY Business Team Priority2
Projects
None yet
Development

No branches or pull requests

8 participants