Repository navigation
notice(non_ascii_or_non_printable_char): use standard fieldName - #2165
c-tonneslan wants to merge 2 commits into
Conversation
The serialized output of this notice carries the offending column under the key 'columnName', but every other notice uses 'fieldName' (see InvalidCurrencyNotice, InvalidPhoneNumberNotice, etc.). Pipelines consuming the validator output have to special-case this one validator. Just match the convention. Callers already pass cellContext.fieldName() into the constructor, so no behavior changes - only the JSON key the field reflects to. Closes MobilityData#1205. Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
|
|
|
|
skalexch
left a comment
skalexch
left a comment
There was a problem hiding this comment.
Thank you for the change! It makes sense since fieldName is the standard attribute for all other notices using the header name of the column.
@emmambd this requires another review from a dev.
Also, a change needs to be done on the web page?
We need to swap columnName for FieldName and change the definition to "The name of the faulty field."
|
@skalexch The documentation is produced by the notice code, so the notice code is what should include the changed definition. FieldName will automatically appear once this PR is merged and released. |
|
@davidgamez Woohoo! Is this a breaking change since it means downstream users need to remove the special-case for this rule? |
|
|
||
| /** Name of the column where the error occurred. */ | ||
| private final String columnName; | ||
| /** Faulty record's field name. */ |
There was a problem hiding this comment.
[nit]: Just to be consistent with other fields' documentation:
| /** Faulty record's field name. */ | |
| /** The name of the faulty field. */ |
Yes, this is a breaking change, as any consumer parsing this rule will need to consume the "new" property name. |
|
@c-tonneslan, please remove |
5 participants
The serialized output of
NonAsciiOrNonPrintableCharNoticecarries the offending column under the keycolumnName, but every other notice (InvalidCurrencyNotice,InvalidPhoneNumberNotice, etc.) usesfieldName. That forces downstream pipelines to special-case this one rule.Just renamed the field to match. Callers already pass
cellContext.fieldName()into the constructor positionally, so no caller signatures change and no behavior changes — only the JSON key the field reflects to.Closes #1205.