Skip to content
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

Bulk tagging JS fix #1115

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Bulk tagging JS fix #1115

merged 4 commits into from
Oct 3, 2024

Conversation

rabstejnek
Copy link
Collaborator

If the IDs in an excel file were parsed as strings, then the bulk tagging form would not be able to map them to integer based IDs. This PR addresses this issue by casting all incoming excel IDs explicitly as strings and the keys in the reference map also as strings; since these "stringified" values are only used for matching and because some IDs (ie doi) are already strings this seemed like the most unified approach.

Whitespace is also trimmed from incoming excel IDs, since identifiers stored in HAWC should not have leading or trailing whitespace; this also aids in a better match between excel IDs and the reference map.

Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, a simple fix that should work well.

@shapiromatron shapiromatron merged commit 0b0e373 into main Oct 3, 2024
6 checks passed
@shapiromatron shapiromatron deleted the bulk-tagging-js-fix branch October 3, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants