-
Notifications
You must be signed in to change notification settings - Fork 13
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
Condom management features #380
Conversation
…t match instead of case insentive contains match which can result in multiple records being returned
…e GIS mapping feature
… stock level information is incomplete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! This looks great overall.
I had a few mostly-minor suggestions, as well as a few questions that I'm hoping you can answer. Most importantly, can you tell me the easiest way to test this so I can run things on my development environment?
Feel free to respond/update the PR based on the feedback so far, or let me know if you'd like my assistance in doing that.
@@ -1482,7 +1482,7 @@ def get_product(self, product_code): | |||
if the product can't be found. | |||
""" | |||
try: | |||
return Product.objects.get(sms_code__icontains=product_code) | |||
return Product.objects.get(sms_code__iexact=product_code) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like a good change. out of curiosity, do you remember which product / scenario specifically tripped the error that this fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scenario that produced this issue is one where we were reporting stock on hand for the products below. The use of "contains" using either product code "cm" or "cf" would result in more than one record being returned which would then generate an error identifying either of the product codes as not existing
- Condom-Male with product code cm
- Condom-Female with product code cf
- Socially Marketed Condom-Male with product code mcm
- Socially Marked Condom-Female with product code mcf
def _validate_latitude(self,latitude): | ||
if self._is_float(latitude): | ||
lat_value = float(latitude) | ||
if (lat_value > -90) and (lat_value < 90): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to consider updating these checks to also limit it to a bounding box around Malawi? might be helpful though is not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounding box would be good to ensure that its specific to Malawi
I think that the bounding box itself can be generalized to accommodate other areas if need be. I will look into how best that can be addressed going forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the follow ups @kmkamoto-ad - this is looking good.
Before merging and deploying , I'd like to run the code locally - is there an easy way to do that? Just submit a few coordinates for facilities and then load the report?
You can use the following to test the map SMS keyword once you have registered an HSA. The report requires product stock for an HSA so that can be verified once a mapped HSA reports stock on hand I have also included the database that was used in case you want to restore it and look at the reports with more data |
@kmkamoto-ad I was able to verify the SMS workflows and the example map. Everything is looking good. Are there any other steps needed on this, or is it ok to go ahead and deploy it to production? I can take care of that either later today or tomorrow. |
Great! |
The features cover the use of GIS coordinates to enable tracking of product stock levels at HSA level
This includes a keyword handler to allow an HSA to set their work location using GIS coordinates (values of latitude and longitude), as well as a geographic visualization of the stock levels given a specific health product (under Stock Status reports)