Add feedback for invalid substitution patterns. #687
Conversation
landonreed
changed the title
| function extractStateFields (props: Props): Substitution { | ||
| const { description, normalizeSpace, pattern, replacement } = props.substitution | ||
| const { description, normalizeSpace, pattern, replacement, valid } = props.substitution | ||
| return { | ||
| description: description || '', | ||
| normalizeSpace, | ||
| normalizeSpace: normalizeSpace || false, | ||
| pattern, | ||
| replacement | ||
| replacement, | ||
| valid | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @returns true if the given substitution pattern is null or empty, false otherwise. | ||
| */ | ||
| export function isSubstitutionInvalid (substitution: Substitution) { | ||
| export function isSubstitutionBlank (substitution: Substitution) { | ||
| return !substitution.pattern || substitution.pattern === '' | ||
| } | ||
|
|
||
| /** | ||
| * @returns true if the given substitution is marked invalid by the backend, false otherwise. | ||
| */ | ||
| export function isSubstitutionInvalid (substitution: Substitution) { | ||
| return !substitution.valid | ||
| } |
There was a problem hiding this comment.
To better organize things, can these util functions be moved to a util file?
There was a problem hiding this comment.
Actually, maybe just the isSubstitutionBlank and isSubstitutionInvalid functions.
There was a problem hiding this comment.
Updated in cf0d3a0.
| /** | ||
| * Determines whether two substitutions are equal (except for the valid field). | ||
| */ | ||
| function areEqual (sub1: ?Substitution, sub2: ?Substitution): boolean { |
There was a problem hiding this comment.
| function areEqual (sub1: ?Substitution, sub2: ?Substitution): boolean { | |
| function substitutionsAreEqual (sub1: ?Substitution, sub2: ?Substitution): boolean { |
There was a problem hiding this comment.
Updated in cf0d3a0.
There was a problem hiding this comment.
This seems like an indirect way of performing validation while preserving the input data. This attempts to save the data and there just happens to be a helpful (but somewhat intrusive) error message that happens to be displayed due to any webservice failure. But of course, the updateFeedSource action is refetching the feed source and thus overwrites the data hence the need for this state management stuff to prevent this "side effect".
The more direct way of doing this would be to attempt a pre-update of the data by making a request to the server and if that failed, don't do the actual update, but do display a non-intrusive error message in the appropriate place in the form. That way the react values would still be preserved and none of this fancy state management workaround would be needed.
That being said, it's built and functions as is. However, since it indirectly depends on the functionality of a somewhat unrelated process, this has the potential to be dead code or if the error message display is refactored, then the UX is different. I don't really like it, but I suppose I can approve.
I will, however, make this conditional on creating a util file for the mentioned functions.
3 participants
Checklist
devbefore they can be merged tomaster)Description
Companion PR to ibi-group/datatools-server#392 to address ibi-group/datatools-server#391.
This PR adds UI feedback for invalid substitution patterns.
To test on configurations that enable Normalize Field Transformation:
After you enter the invalid substitution patternEDIT for previously saved invalid substitutions, notice the feedback on the UI (see screenshot). Modify the incorrect substitution pattern to remove the red feedback box, and click the check mark to save the changes and observe the subsequent UI feedback.