-
Notifications
You must be signed in to change notification settings - Fork 0
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
MLPAB-1552: Attorney moving from paper to online #179
Conversation
@@ -212,7 +212,16 @@ | |||
} | |||
], | |||
"type": "object", | |||
"required": ["dateOfBirth", "email", "status"], | |||
"required": ["dateOfBirth", "email", "status", "channel"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably want to remove email
from the default required set?
"required": ["dateOfBirth", "email", "status", "channel"], | |
"required": ["dateOfBirth", "status", "channel"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this - yep agree, I'll remove
@@ -212,7 +212,16 @@ | |||
} | |||
], | |||
"type": "object", | |||
"required": ["dateOfBirth", "email", "status"], | |||
"required": ["dateOfBirth", "email", "status", "channel"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a breaking change on the shiny new schema, should this be 2024-04-1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but also it isn't required, as the validation hasn't been updated yet. So maybe hold the required
change for your next PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think realistically we can break schema for now since no real LPAs are being registered, otherwise we're going to be making new versions pretty much daily.
"then": { | ||
"required": ["email"] | ||
}, | ||
"required": ["dateOfBirth", "email", "status"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but as Greg says, you may want to remove email (and the existing validation in create) so you can do the consumer side
Partially fixes MLPAB-1552. As with CPs, I've left email and channel optional until I make MRLPA changes (including pact consumer tests) and then I'll loop back round, remove optional and apply
MustMatchExisting
.