Fix bulk add writing wrong values and dropping rows and columns - #1172
Open
labkey-martyp wants to merge 8 commits into
Open
Fix bulk add writing wrong values and dropping rows and columns#1172labkey-martyp wants to merge 8 commits into
labkey-martyp wants to merge 8 commits into
Conversation
Date columns other than the form's primary date field were read as UTC, so imported dates landed a day early or vanished when they hit the epoch, and lookup values matched on a prefix instead of exactly, letting one code silently store another.
The bulk add importer silently stored raw text for unmatched lookups and dropped unparseable dates, and resolved project names by prefix. All three now surface a per-row error.
Rows keyed solely on a server-assigned identity column have an empty key before save, so every new row after the first resolved to the first and collapsed onto it. Such tables could not add more than one row per save.
Error messages rendered pasted cell values as HTML, a lookup store with no records loaded silently wrote raw text through instead of reporting it, and exact matching rejected whitespace-padded cells and pasted key values that previously imported.
Header cells were matched exactly and case-sensitively, so a padded or recased header silently dropped its whole column; they are now normalized, and a header matching no field, or two naming the same one, is reported. Columns handled before the field loop are skipped by name rather than truthiness so a failed project or date is not resolved a second time by a path that disagrees, and unknown-project errors echo the pasted value instead of its zero-padded form.
Header matching covered only a field's name and first import alias, so a header spelled as the field's label, or as any later alias, resolved to nothing -- and since an unrecognized header now stops the import, the recognized-name set was also vouching for aliases the matcher could not actually place, dropping those columns silently. Resolution now runs through LABKEY.ext4.Util.resolveFieldNameFromLabel for both, keyed on the resolved field so duplicates are caught per field rather than per header spelling. An exact name match is tried first because that helper breaks on its first name/caption/label hit and would otherwise let a field merely labelled the same win on config order.
Trimming the pasted block with Ext4.String.trim() counted a tab as whitespace, so the last row lost its empty trailing cells and read as truncated even though every value in it was correct; spaces and line endings are still trimmed, since a stray one at either end parses as a junk row or is mistaken for the header row. A row is now measured against the last column a required field maps to rather than the number of required fields, which was unrelated to where those fields sit, and a missing required column or an unusable header row is reported once instead of as a missing value on every row. Header resolution also prefers importable fields for exact names, matching the order already used for display names, so a field the importer skips cannot claim a name another query declares.
…fb_bulk_add_date_parse
This file contains hidden or 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
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.
Rationale
Fixes a set of bugs that let the EHR bulk add importer write wrong or incomplete data without reporting anything, in the UI, the browser console, or the server log.
Because nothing fails, the damage is only discoverable by diffing the loaded data back against the source, so an import can appear clean while carrying shifted dates, missing dates, lookup values swapped for unrelated ones, whole columns absent because their header was spelled differently, and — on tables whose key is assigned by the server — only the last of the pasted rows. The reporting added here echoes pasted text back to the user, so it is escaped; an import commonly starts from a spreadsheet someone else produced.
The same work also covers the opposite failure, where the importer refused input that was perfectly good or blamed the wrong thing for it, since a rejection nobody can act on costs the user as much as a silent one.
Related Pull Requests
None.
Changes