bcgov / entity

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

UI - Deleteing an executor (WILL) #16290

Closed mstanton1 closed 11 months ago

mstanton1 commented 1 year ago

FIGMA FILE

Deleting Executor (See Figma pages 2.b, 2.c, and 2.d) When an executor is deleted:

mstanton1 commented 1 year ago

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

dimak1 commented 1 year ago

@tlebedovich @LizGovier just want to clarify something. The design does not address a few mixed-owners scenarios, which kind of blocking me from continuing the work:

Basically all the odd scenarios with mixed users.

Thanks!

LizGovier commented 12 months ago

Hey @dimak1 ,

As per your first question, I would pre-populate the suffix to be the last executor who was deleted.

For the second and third question, when removing an executor, there shouldn't be a situation where there are a mix of owners and executors. Let me know if you'd like to jump on a call and discuss.

dimak1 commented 12 months ago

@LizGovier yea, let's discuss this. If we have to track the last deleted executor/admin it will increase the scope of the ticket.

dimak1 commented 12 months ago

Hi @LizGovier, as discussed this morning, the NEW RULE - Additional name for executors and can only be changed by editing could only be implemented if we disable dynamic change of the Additional Name when Owners are added/deleted/edited in the Home Owners page view. So once the new Owner is added, the Additional Name will not be updated, for all transfer types. We can either allow the user to change the Add. Name manually or change it auto-dynamically, otherwise, we'll need a separate ticket for this to discuss further.

Screenshot 2023-05-25 at 11.46.09 AM.png

LizGovier commented 11 months ago

@dimak1 Thanks Dima. I agree with manually updating the name change, but will wait to hear back from Scott before giving the go-ahead

dimak1 commented 11 months ago

@LizGovier I pushed the PR, but without implementing a NEW RULE for Executors. I will wait for your feedback from Scott, and we can do it after. If it takes longer to get a response, then we should take it out from the Description.

@chdivyareddy please note that NEW RULE - Additional name for executors and can only be changed by editing is not part of this ticket, we'll know more once we hear back from Scott.

dimak1 commented 11 months ago

@LizGovier both tickets (Add/Remove Executors + Admins) are in the DEV env. I put UXA label only on this ticket, so please check out both flows, and we'll track any feedback in this ticket only. OK?

saragunnarsson commented 11 months ago

Most of this looks great @dimak1 - I ignored the NEW RULE additional name field AC for now.

According to figma, they should be able to delete only one executor..so this error message should not show up

image.png

I added another exec (a business) and the additional name / suffix showed these errors..If i remember right the second exec should have the same suffix as the first? image.png

dimak1 commented 11 months ago

Hi @saragunnarsson, thanks for reviewing it. Here are my comments:

  1. Agree, this message should not be shown. However, I didn't see a specific comment in Figma about it. I will update the code for Execs and Admins. Please add a comment to Figma to reflect this.
  2. I rely on Figma comments to implement the requirements and only rely a little on memorizing them. I can update it, but it needs to be backed up by a comment in Figma. If it's there, I apologize for missing it. Thanks
saragunnarsson commented 11 months ago

@dimak1 - yea these were both in a bit of a grey area..

@LizGovier can you update Figma to reflect the scenarios listed above (in my comment)? I think adding a couple notes to the mockups should be enough.

dimak1 commented 11 months ago

Hi @saragunnarsson I've opened a PR to hide error message (screenshot below). I'll move the ticket to UXA once it's in the DEV env. Screenshot 2023-05-29 at 10.17.17 PM.png

Also, please note, if Transfer type is not WILL, the error will show (because we are removing Executors but the transfer flow allows to only add Administrators, so if it's not the same, error will be shown). Screenshot below.

Screenshot 2023-05-29 at 9.24.59 PM.png

dimak1 commented 11 months ago

Hi @saragunnarsson I've opened a PR to hide error message (screenshot below). I'll move the ticket to UXA once it's in the DEV env. Screenshot 2023-05-29 at 10.17.17 PM.png

Also, please note, if Transfer type is not WILL, the error will show (because we are removing Executors but the transfer flow allows to only add Administrators, so if it's not the same, error will be shown). Screenshot below. cc: @LizGovier

Screenshot 2023-05-29 at 9.24.59 PM.png

LizGovier commented 11 months ago

@dimak1 @saragunnarsson

Re: "All owners must be deceased and an executor added"

Re: "Executor of the Will of N/A"

dimak1 commented 11 months ago

Hi @LizGovier yes, if at least one Exec or Admin is still in the group, there won't be any errors. I updated it, and PR is under review. Regarding the suffix, let's remove the bullet point from the description and create a new ticket, covering both Exec and Admin transfer flows. Thanks!

dimak1 commented 11 months ago

Hi @LizGovier yes, if at least one Exec or Admin is still in the group, there won't be any errors. I updated it, and PR is under review. Regarding the suffix, let's remove the bullet point from the description and create a new ticket, covering both Exec and Admin transfer flows. Thanks!

saragunnarsson commented 11 months ago

Ahhh, one more thing @dimak1

image.png

dimak1 commented 11 months ago

@saragunnarsson yea I noticed that as well when doing verification. I think it marks the group because there is still one existing/current owner that is not deleted. Validation needs to be updated, will fix, thanks!

saragunnarsson commented 11 months ago

Awesome! then I'll put this back in progress (and if you want it anywhere else, feel free to move it).

dimak1 commented 11 months ago

@LizGovier or @tlebedovich could you please remove suffix related items from this ticket, as we have a new ticket #16581 to address it. Thanks!

dimak1 commented 11 months ago

@LizGovier or @tlebedovich could you please remove suffix related items from this ticket, as we have a new ticket #16581 to address it. Thanks!

LizGovier commented 11 months ago

@dimak1 suffix bullet point has been removed!

dimak1 commented 11 months ago

Thanks @LizGovier. I tagged you in the comment above, about Transfer type that's not matching with Owner types. In that case the error is there. Please let me know.

dimak1 commented 11 months ago

@saragunnarsson fixed the error for the Group and ready for UXA.

saragunnarsson commented 11 months ago

Error still happening on the review and confirm button (but it's not happening on LETA transfers)

image.png

dimak1 commented 11 months ago

Hi @saragunnarsson, it is ready for another round of UXA. Thanks!

saragunnarsson commented 11 months ago

Hey @LizGovier I think we are gonna need your help in order to move this ticket along..

In the below screenshot..Should there be an error message? and if so, what should it say? The one showing isn’t really working.. and I can’t find a scenario in figma that’s showing the deletion of 1 executor in a group of multiple executors. I think we might need a new error?

image.png

LizGovier commented 11 months ago

@saragunnarsson @dimak1

Because you're in the 'Grant of Probate' transfer, the message you have above is correct. However once you add an executor you should get an error message which says "Group cannot have a mix of owner, executor, administrator and bankruptcy trustee roles." Screen Shot 2023-06-06 at 2.37.54 PM.png

saragunnarsson commented 11 months ago

@LizGovier Ohhh. I see. I keep getting these owner types mixed up. Sorry! my bad.

saragunnarsson commented 11 months ago

This ticket is good to go, moving along to RFQA.

dimak1 commented 11 months ago

Hi @saragunnarsson I tried the MHR 150655 and adding a 3rd owner worked ok. Even thou it says Trustee in the Additional Name, the owner type is actually an Executor. So we do have a mixed owner situation, but visually in UI we can't distinguish (screenshot below).

As per @LizGovier comment above, there should be an error for mixed roles, but we cannot Edit existing owners, only the Added, and we can't change the role type (it has to be Executor in this Transfer Type). Maybe we need a separate ticket to fix the error message, as Trustees are not in scope for this ticket.

Thanks!

Screenshot 2023-06-06 at 1.12.31 PM.png

chdivyareddy commented 11 months ago

Verified in DEV with MHR 150629.

image.png

image.png