Introduce PropertyFormat to avoid validation errors that can lead to internal exceptions #248
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -42,7 +42,6 @@ public sealed partial class SmtpIntegration : IIntegration | |||
|
|
|||
| public static readonly IntegrationProperty FromEmailProperty = new IntegrationProperty("fromEmail", PropertyType.Text) | |||
| { | |||
| Pattern = Patterns.Email, | |||
There was a problem hiding this comment.
Can you put this property at the end, just like in the other property?. My brain is annoying sometimes ;)
| if (property.format && property.format !== "None") { | ||
| const format = FORMAT_REGEXPS.get(property.format); | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove double line.
|
|
||
|
|
||
| if (format) { | ||
| propertyType = propertyType.matches(format, texts.validation.formatFn); |
There was a problem hiding this comment.
Yup already provides validation method for email and url. Just use that. There is also emailI18n and perhaps url18n for localized methods in the code base.
| @@ -441,6 +441,7 @@ export const EN = { | |||
| minItemsFn: (p: { label?: string; min: number }) => `${p.label} must have at least ${p.min || 0} items.`, | |||
| minLengthFn: (p: { label?: string; min: number }) => `${p.label} must have at least ${p.min} characters.`, | |||
| moreThanFn: (p: { label?: string; more: number }) => `${p.label} must be greater than ${p.more || 0}.`, | |||
| formatFn: (p: { label?: string }) => `${p.label} is not in a valid format for this type of field.`, | |||
There was a problem hiding this comment.
We have introduced the format property to provide better validation messages. Otherwise we could just use "pattern", which has a very generic method.
| @@ -39,6 +39,8 @@ public sealed record IntegrationProperty(string Name, PropertyType Type) | |||
|
|
|||
| public string? Pattern { get; init; } | |||
|
|
|||
| public PropertyFormat Format { get; init; } = PropertyFormat.None; | |||
There was a problem hiding this comment.
Validation is missing in the TryGetString method.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #247 , more info there.
Can you please review my code for now so that I can make some appropriate changes?
Cheers