MLPAB-2318: Support ATTORNEY_OPT_OUT #241
Conversation
internal/shared/update.go
Outdated
| parts := strings.Split(u.Author, ":users:") | ||
| if len(parts) > 1 && parts[1] != "" { | ||
| return parts[1] | ||
| } else { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
| parts := strings.Split(u.Author, ":users:") | |
| if len(parts) > 1 && parts[1] != "" { | |
| return parts[1] | |
| } else { | |
| return "" | |
| } | |
| _, after, _ := strings.Cut(u.Author, ":users:") | |
| return after |
lambda/update/attorney_opt_out.go
Outdated
| return AttorneyOptOut{}, []shared.FieldError{{Source: "/changes", Detail: "expected empty"}} | ||
| } | ||
|
|
||
| if update.AuthorUID() == "" { |
There was a problem hiding this comment.
if this were changed to pass the change specifying which attorney is opting-out it might allow Sirius to use it too? (and also I'm starting to wonder if having empty updates is a bad pattern)
There was a problem hiding this comment.
Yeah I would prefer that you have to specify the UID, and iif the JWT comes from MRLPA then the UID must match. That would give us more flexibility for Sirius and maintain a security check.
lambda/update/attorney_opt_out.go
Outdated
| // TODO Check if status correct | ||
| attorney.Status = shared.AttorneyStatusRemoved |
There was a problem hiding this comment.
Makes sense to stick with removed for now. If we find a need for something more particular then we can add that later (either a new status, or a new "reason for removal"-type field)
lambda/update/attorney_opt_out.go
Outdated
| switch len(lpa.Attorneys) { | ||
| case 1, 2: |
There was a problem hiding this comment.
Should this be checking that there are 1 or 2 active attorneys? In case they've already been removed.
There was a problem hiding this comment.
oh yeah, and will also need to take into account trust corporations and replacements
There was a problem hiding this comment.
Good call on the attorney status, I'll update.
I was planning on handling replacements and trust corps in a separate ticket as the logic gets a bit spicy then but will double check if there's a ticket in the backlog and add one if not.
enforce URN format, match author and subject UI when makeregister and validate on active attorneys
lambda/update/attorney_opt_out.go
Outdated
|
|
||
| uidErrors := validate.All( | ||
| validate.UUID("/update/author/uid", author.UID), | ||
| validate.UUID("/update/subject", update.Subject), |
There was a problem hiding this comment.
this should be /subject to match the JSON given, and I think maybe just ignore validation on author.UID?
lambda/update/attorney_opt_out.go
Outdated
|
|
||
| attorneysCount := len(lpa.ActiveAttorneys()) + len(lpa.ActiveTrustCorporations()) | ||
| switch attorneysCount { | ||
| case 0, 1: |
There was a problem hiding this comment.
I'm now questioning the logic here myself 😂 would having one remaining attorney invalidate the LPA in all cases or only if its the jointly or jointly for some...?
There was a problem hiding this comment.
do you have a link to where this logic came from? (I would've thought a single attorney is fine for "jointly and severally" if the other attorneys can be removed)
There was a problem hiding this comment.
That's the problem, I thought I could rely on the delete attorney MRLPA logic but as its pre-signing we're not focused on validity there.
I've asked Abbi to add this topic to collab time tomorrow to discuss the various scenarios. I'll update this and we can tweak if needed after tomorrow's discussion.
docs/openapi/openapi.yaml
Outdated
| subject: | ||
| type: string | ||
| format: uuid |
There was a problem hiding this comment.
The original intent of this was that you would do:
{"changes": [
{
"key": "/attorneys/5373cdf6-c74c-4e1c-a126-e7aa60afb72d/status",
"old": "active",
"new": "removed"
}
]}Which was particularly thinking about editing multiple fields at once (e.g. scanning corrections or changing someone's contact details).
It feels like this pattern isn't working for us in a few places now. I wonder if we should allow each update type to have its own shape?
Purpose
Enables MLPAB-2318.