bcgov / entity

ServiceBC Registry Team working on Legal Entities
Apache License 2.0
23 stars 57 forks source link

UI - Sale and Gift - Error Handling for Roles when removing Exec/Admin/Bank Trustee #17034

Closed mstanton1 closed 6 months ago

mstanton1 commented 10 months ago

Staff and Qualified Supplier - Transfer due to sale or giftNote: This includes the special Sale/Gift transfer after an AFFE

Error handling when removing Exec/Admin/Bank Trustee

If there is a group with more than one admin/exec/bankruptcy trustee then:

Figma File : https://www.figma.com/file/YcXMXw581zx7YxR34gkKvs/MHR-Transfers?type=design&node-id=2966%3A134604&mode=design&t=joNzKHOxP1jZZgAK-1

mstanton1 commented 10 months ago

@LizGovier can you add the items from your document into this ticket for grooming?

LizGovier commented 10 months ago

Hey team! Please add your planning poker estimate with Zenhub @cameron-eyds @doug-lovett @chdivyareddy @orelbn @dimak1

dimak1 commented 7 months ago

@LizGovier I want to check with you about this MHR 107811. It has mixed owners, and when I delete one owner I get the mixed owners error. Is this right? Is the data also correct (or bad MHR and should not have mixed owners)?

Private Zenhub Image

LizGovier commented 7 months ago

Hey @dimak1 You shouldn't be getting that message here because you don't have a mix of owner types here.

Tracey Test (Owner) West Distribution Incorporated (Owner) John Smith (Owner)

The person you deleted 'Tracey Test' says in the additional text field to be a Trustee, but in fact it is an Owner Type. You can tell because the icon does not have a star beside it to indicate that it's an Executor, Admin, Bank Trustee.

All that to say, this error message should not be appearing as they are all 'Owner' types and not Exec, admin, ect.

/cc @tlebedovich Could you confirm my explanation is correct.

tlebedovich commented 7 months ago

@LizGovier @dimak1 - Liz you are correct. All 3 of these owners should have the regular "Owner" role. None of these owners are executors, admins or bankruptcy trustees from what I can see as they don't have the special icon with star.

To test, I checked these owners using the Transfer to Surviving Joint Tenants transfer type where you are allowed to go in and edit the owner's names and see their roles. Ignore the fact the role radio buttons aren't styled correctly, but I did notice that the West Distribution did not have a role radio button assigned to it . Maybe that's the issue?

Private Zenhub Image

Private Zenhub Image

Private Zenhub Image

dimak1 commented 7 months ago

@tlebedovich @LizGovier I found the reason for this issue: in our logic, we treat OWNER_IND and OWNER_BUS party types as unique roles and show mixed roles error. We check the partType property to determine the uniqueness of owners in the group, since we don't have an explicit 'role' property.

Please confirm that I should update the code to treat OWNER_IND and OWNER_BUS as the same 'role'?

tlebedovich commented 7 months ago

@dimak1 - I think we will need both the "Role" - aka the four Role radio buttons - Owner / Executor / Administrator / Bankrupty Trustee

And then we will also need to know if the owner (any role) is a Person or a Business because we now have a design ticket coming up to stop Qualified Suppliers from doing certain transfers where a business is part of an owner group.

So in summary we need to establish the Role (Owner / Executor / Administrator / Bankrupty Trustee radio button) AND the Party Type (Person or Business).

The mismatched Role errormessage is based on the "Role" radio buttons .

I assume this will need to be coordinated with the API as well

c/c @doug-lovett @mstanton1

dimak1 commented 7 months ago

There is a bug open for this issue (mixed roles error), let's move the conversation there: #18070.

dimak1 commented 7 months ago

Ready for UXA: https://bcregistry-assets-dev--pr-1573-s1d2bh8b.web.app/ @LizGovier

tlebedovich commented 7 months ago

@dimak1 - I moved our comments about roles/types over to #18070

LizGovier commented 7 months ago

@dimak1 I'm still getting the mixed role error when there is a business in a joint tenancy:

Private Zenhub Image

dimak1 commented 7 months ago

@LizGovier mixed roles error is not part of this ticket, it is fixed in another ticket, #18070 / #17650. (You should be just checking the scope of the ticket as described in the Description).

LizGovier commented 7 months ago

Hey @dimak1 Error message is showing as requested for Executors, Admins, and Bankruptcy Trustee. This ticket is ready to move forward!

dimak1 commented 7 months ago

Thanks! Merged to Dev.

chdivyareddy commented 7 months ago

Hey @dimak1 , when one of the admin/exec/bankruptcy trustee’s is deleted from a group and if user clicks on Review & Confirm button, then the user is taken to the Review screen without blocking it from navigating to the review screen. Please take a look, thanks!!

MHR 107829 in DEV.

Private Zenhub Image

Private Zenhub Image

dimak1 commented 7 months ago

@chdivyareddy thanks for catching this, maybe some odd scenario, will take a look now.

tlebedovich commented 7 months ago

@dimak1 - fyi I just added the design ticket as a blocker just so we have the two tickets linked. its not a real blocker.

dimak1 commented 7 months ago

@chdivyareddy I fixed it, here is the preview link, or I can merge it to Dev. Let me know, thanks.

https://bcregistry-assets-dev--pr-1585-kmnsjibm.web.app/

chdivyareddy commented 7 months ago

Hey @dimak1 , I just tried in DEV with the preview link you provided and still seeing the same issue as noted above. Please take a look, thanks!!

Private Zenhub Image

Private Zenhub Image

dimak1 commented 7 months ago

@chdivyareddy I have not merged to Dev yet (as I need a PR review). Could you please try the preview lik I provided? Thanks!

chdivyareddy commented 7 months ago

@dimak1 , Yes I did try with the preview link you provided, please take a look at the screenshots provided, thank you!!

dimak1 commented 7 months ago

@chdivyareddy that's strange, because in my local dev this is fixed and working...

I run the preview build again, and it is working now: https://bcregistry-assets-dev--pr-1585-kmnsjibm.web.app/

chdivyareddy commented 7 months ago

@dimak1 , this looks strange...I'm able to navigate to the review screen without being blocked:(

Please let me know once this is merged to DEV, thanks!!

dimak1 commented 7 months ago

@chdivyareddy I fixed the issues we discussed. Here is link if you want to check: https://bcregistry-assets-dev--pr-1585-kmnsjibm.web.app/ Otherwise, let me know if you want me to merge to dev.

chdivyareddy commented 7 months ago

@dimak1 , it is working as expected. Please merge to DEV, thanks!!

image

dimak1 commented 7 months ago

Thanks @chdivyareddy, merged to DEV.

tlebedovich commented 7 months ago

@dimak1 @chdivyareddy - i'm not sure if this is related to the work done in this ticket or totally unrelated, but I noticed this MHR for Staff Sale/Gift is not showing the "Delete All Owners/Groups" button. I created a new bug ticket for this if its needed -- #18218 18218

chdivyareddy commented 7 months ago

@tlebedovich good catch! @dimak1 looks like this issue is with the recent change, in the previous screenshot it did display the "Delete All Owners/Groups" button.

tlebedovich commented 6 months ago

@dimak1 - I think this ticket can be moved along now, no? delete all button is appearing as it should and think the work here is done and in DEV already. Will move to RQFA

tlebedovich commented 6 months ago

oh wait, its already there, haha

dimak1 commented 6 months ago

@tlebedovich :) thanks for follow up, yes it is waiting for QA to start

chdivyareddy commented 6 months ago

Verified in DEV!

Staff: image

QS: image