OBPIH-7443 fix import demo products error when tag is null and when category nam…#5436
Merged
kchelstowski merged 1 commit intorelease/0.9.5from Aug 7, 2025
Merged
Conversation
…e doesn't match the case
github-actions
bot
added
type: bug
ewaterman
commented
Aug 7, 2025
| ,CR388,Default,Lamivudine 150mg + nevirapine 200mg + zidovudine 300mg tablet,,ARVS,,,Each,Primary Care,0.29,TRUE,,,,,,,,,,,,,,, | ||
| ,TZ835,Default,Lamivudine 150mg + zidovudine 300mg tablet,,ARVS,,,Each,Primary Care,0.29,TRUE,,,,,,,,,,,,,,, | ||
| ,CM871,Default,Lamivudine 150mg tablet,,ARVS,,,Each,Primary Care,0.29,TRUE,,,,,,,,,,,,,,, | ||
| ,MC017,Default,Lamivudine 30mg + nevirapine 50mg + zidovudine 60mg tablet,,ARVS,,,Each,Primary Care,0.29,TRUE,,,,,,,,,,,,,,, |
Member
Author
There was a problem hiding this comment.
Take a look at default_OB_categories.csv (and changelog-insert-data-core.groovy but I don't think that's used). The categories are created with ARVS but this file tried to set ARVs.
The code change in ProductService makes this csv change unnecessary (because it doesn't case-insensitive matching), but I figured it'd be best to keep the data in sync.
ewaterman
commented
Aug 7, 2025
| * when importing data since user input errors are common. In other scenarios this equivalency might not be useful | ||
| * (we might want to treat "name" and "NAME" as different), so make sure to be intentional when using this class. | ||
| */ | ||
| private class CategoriesByDesensitizedName { |
Member
Author
There was a problem hiding this comment.
I opted to make this a class because it allowed for clearer usage of the map. Users of the class don't need to remember to desensitize the data, it happens automatically. All they need to do is provide the category for puts, and category name for gets
kchelstowski
approved these changes
Aug 7, 2025
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
2 participants
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.
…e doesn't match the case
✨ Description of Change
Link to GitHub issue or Jira ticket: https://pihemr.atlassian.net/browse/OBPIH-7443
Description: Two fixes:
tagNamesForAllProducts + products.tags.flatten() as List<String>throws an error when tagNamesForAllProducts is nullproducts.csvdemo data it was trying to assign "ARVs" to the products. This caused the import service to not be able to resolve the category and thus threw an error. I changed the csv file to use "ARVS" so that it matches the real category, but I also refactored the import logic to be case-insensitive so that even without the csv change it'll see them as the same category. The current behaviour on prod is that it's case-insensitive so the change in this PR preserves existing behaviour. (I refactored product import recently which changed the behaviour to be case-sensitive.)The behaviour is slightly different depending on if the categories already exist:
Say we want to import product 1 with category "HI" and product 2 with category "hi"
Scenario 1) Categories "HI" and "hi" both already exist. When we import, p1 gets category "HI" and p2 gets category "hi"
Scenario 2) Category "HI" already exists, "hi" does not. When we import, both p1 and p2 get category "HI"
Scenario 3) Category "hi" already exists, "HI" does not. When we import, both p1 and p2 get category "hi"
Scenario 4) Neither category "hi" nor "HI" already exist. Whichever category appears first in the csv will get created and used for both products. If "hi" is first, both p1 and p2 get the categcategory "hi".
Kinda weird behaviour but it does make sense since if we already have two categories with the same name but a different case, we presumably want to match those directly, and if we only have one or neither, we assume user input error and do a case-insensitive match. And again, this is the existing behaviour.
📷 Screenshots & Recordings (optional)