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

Vaccine module #1353

Closed
37 of 76 tasks
josh-griffin opened this issue Oct 14, 2019 · 40 comments
Closed
37 of 76 tasks

Vaccine module #1353

josh-griffin opened this issue Oct 14, 2019 · 40 comments
Labels
Epic A discussion and/or parent issue for module/feature design
Projects

Comments

@josh-griffin
Copy link
Contributor

josh-griffin commented Oct 14, 2019

Description

Vaccine module integration and slight refactor into current develop

Previous epic: https://github.com/sussol/org-issues/issues/23

Context

The Vaccine module was never merged into the code base, and now the current branch is very far behind master. Needs to be redone. It is in production in Solomon Islands and Vanuatu.

Update (13/05):

Vaccines is the main feature for v6. Main feature branch merged into develop to get everything up-to-date and to get a head start on integration testing. All future vaccines work should be based off/merged into develop.

INFORMATION

Vaccine work has been merged into the main develop branch.

Old feature branches:

TAG which is deployed in Vannuatu & Solomon Islands: https://github.com/openmsupply/mobile/commits/v2.2.0-RC4

TASKS

I've written the issues quite vague and non-specific as more specific details may not be needed dependent on the tissue and reviewer. Can provide far greater detail on a specific issue when needed!!

Tasks will be separated between back-end logic and front-end UI to try to avoid as much blocking as possible. Setup issues should likely all be done before others.

Each grouping will most likely need to be done roughly in this order

SETUP

BACKEND

FRONTEND

DESKTOP

TESTING

  • CustomerRequisitionPage
    • ISSUE: TODO
  • CustomerInvoicePage
    • ISSUE: TODO
  • SupplierInvoicePage
    • ISSUE: TODO
  • VaccineModulePage
    • ISSUE: TODO
  • VaccineAdminPage
    • ISSUE: TODO
  • VaccineStockPage
    • ISSUE: TODO
  • VaccineItemPage
    • ISSUE: TODO

DOCUMENTATION

beginnings of documentation is here:

Design documents:

To be written 'draft like' as features are completed:

  • How to set up from desktop
    • Locations/Location types
    • Options?
    • store custom data
  • How to enable on mobile
    • Sync/restart
  • How to enable syncing
    • Admin page, setting up sensors/fridges
    • sync modal
  • Managing vaccine stock
    • Manage vaccine item/stock pages
    • managing vvm status' / disposing
  • Main vaccine page
    • fridges
    • charts
  • Breaches
    • breach modal
    • breach icons
  • Customer invoice page
    • columns
  • supplier invoice page
    • columns
  • customer requisition page
    • expansion
  • Tupaia (???)
@josh-griffin josh-griffin added the Epic A discussion and/or parent issue for module/feature design label Oct 14, 2019
@josh-griffin josh-griffin changed the title Feature/module name and concise purpose if not conveyed in name Vaccine module Oct 14, 2019
@josh-griffin josh-griffin pinned this issue Oct 14, 2019
@josh-griffin

This comment has been minimized.

@josh-griffin josh-griffin added this to To do in mobile via automation Oct 22, 2019
This was referenced Oct 23, 2019
@josh-griffin josh-griffin unpinned this issue Dec 13, 2019
@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@josh-griffin josh-griffin pinned this issue Mar 15, 2020
@josh-griffin

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg

This comment has been minimized.

@josh-griffin

This comment has been minimized.

@andreievg
Copy link
Contributor

Wow color in the breachConfig ! That's a great idea, I was just thinking we use the Temperature_min and Temperature on location_type to figure it out but I am pretty sure it would work well if we add color to breachConfig. (btw breachConfig can be assigned to the 'breach').

Brings up a very interesting discussion about having tables/records be 'user-defined', but having hard-coded records

Yeah, although in this case it's maybe 'pre-defined' rather then hard-coded ?

Cool - my reservation with this is just 'when does it stop' .. i.e. do we have isPill, isLiquid, isMaterial, isPlastic .. etc (maybe these are bad examples but hopefully you understand!) .. so I don't know when to 'stop'.. but I think the only way to stop this is to have a itemCategoryZZZ field (another one) which is hardcoded itemCategoryZZZ records..?

I guess what we are trying to do is add a behaviour to this particular item. Awhile ago I as thinking of a structure where most of those properties are controlled by a 'tag' and there would be user defined 'tags' and hard coded tags that alter app behaviour. You could also have 'tag' category, where only one tag out of a group that can be attached to an entity (i.e. unit/or form) (but i think that's getting off topic, isVaccine should be sufficient for now)

Before I respond to this I think I need some more info on what you mean by "I guess the issue for me is how do we link items with suitable locations. "

So, a where house can have multiple locations and multiple location types, i.e. in Vanuatu there are currently two location_types ('safe' and 'fridge'). Consider a small warehouse with multiple fridges and a safe and a shelves. We have 'vaccines' that go into fridges only, we have things like 'morphine' that needs to be kept in a safe, and everything else can go in other locations. Usually locations are used to identify where 'things' are in on a rack, so that the picker knows where to look for things or where to place things.
So potentially there could be quite a few locations of different 'location_types', so how do we know what list to have in UI when choosing location for a batch ? (in mobile VM prototype we currently look for location with locationType.description contains fridge, that's pretty hacky, so thought if we could associated item with a locationType, then it becomes pretty easy and structure

@josh-griffin
Copy link
Contributor Author

ok sweet!

  • Item.isVaccine
  • TemperatureBreachConfiguration : (locationTypeID, minTemp, maxTemp, duration, colour)

what list to have in UI when choosing location for a batch

ahhh ok - I'm a bit slow but I understand now!

Sounds like another need for a bulk config setting interface :trollface: Where are most of the fields for item_store_join even edited? I like the default_location_id field.. that could be super nice.

I think in general it might actually be a bit better to set a lot of details on a per-store basis.. except.. sync and the amount of records it creates.. :( The bright side being (hopefully, I haven't checked) that item_store_join surely only syncs to the sites where the store is? Buut.. on second thought - I suppose item_store_join is store data? so it shouldn't be edited on the server? I was thinking you could just have an object in desktop in the [item]input form and that would update all the item_store_join records.. then you can edit them individually on a store/satellite, if you need. That would give the greatest flexibility, but also breaks 'sync rules' :(

I think it's a really good idea.. should probably try to use the item_store_join way since its there already? Maybe a default_location_type on item could help?

@andreievg
Copy link
Contributor

No sync rules broken btw, even though item_store_join is store specific, they are like preferences, controlled by main server.

So the confusion for me is, item is editable on server, but some things in the item input are actually for currently logged in store:

check out bottom of the screenshot, this is how you set item_store_join.restricted_location_type_id in desktop:

Screen Shot 2020-03-25 at 5 31 24 PM

Sounds like another need for a bulk config setting interface

You are on point, not slow at all.

was thinking you could just have an object in desktop in the [item]input form and that would update all

Great, so if you look at the screenshot, if Restricted to is changed to Restricted to in Store {current store name}: and then add another one Restricted to in All Stores:

Maybe a default_location_type on item could help?

Perhaps should be named default_restricted_location_type ? That would be that one Restricted to in All Stores: ?

If we want to overwrite the default one, then we can set Restricted to in Store.. in item.input, which will set it on item_store_join.

And in UI, when we select location, we first look at item_store_join, then item ?

There is actually very limited places where this is used in mSupply desktop, so easy to change.

@josh-griffin
Copy link
Contributor Author

All sounds really good to me!

And in UI, when we select location, we first look at item_store_join, then item ?

Just this one: Would it be easier that when you change Restricted to in all stores - that this updates all [item_store_join] for the item? Then, when you make a new item_store_join when you add visibility, the default is to have the item.default_restricted_location_type be set to the new item_store_join?

So I am having trouble communicating the difference so hopefully this all makes sense. But basically from what I understand your suggesting:

  • Changing AllStores just changes item.restricted..
  • New item_store_join have item_store_join.restricted.. set to the item.restricted...
  • Changing ForStore changes item_store_join

I think making it so

  • Changing AllStores just changes item.restricted.. AND changes all item_store_join.restricted to the new locationType.

This is because I think the use cases are

  1. Change the location type for every store
  2. Change the location type for a single store

For 1 - if you have some stores who have set a location type themselves, then to change it for everyone you have to change it individually for every store if you don't change the item_store_join when you change the all stores location type. For 2 - If you change it for everyone, it means everyone - so to change it again for individual stores you have to go back and change it again.

Hopefully that makes sense? Also there is the added benefit of only ever checking item_store_join, rather than item_store_join then item

@andreievg
Copy link
Contributor

I guess the question is:

  • "Do we want to persist individually configured item_store_join settings when changing the global item one ?"

Yes -> Then have to do it the way i've described
No -> Don't technically need a field on item or another drop down, just "Apply location restriction to all stores" button/checkbox ?

Or we can combine both methods, in that case, two drop downs and a checkbox.
What you reckon ?

@josh-griffin
Copy link
Contributor Author

"Do we want to persist individually configured item_store_join settings when changing the global item one ?"

ugh that's the sentence I was trying to write in my paragraphs 🤣

I think your idea for the checkbox is nice - or a pref? Or just a dialog when changing in the interface..? "Do you want to apply this change to all stores?" Then we can still just use the item_store_join

@andreievg
Copy link
Contributor

I guess the simplest would be a dialog, when value in list box changes.

@josh-griffin
Copy link
Contributor Author

@andreievg

Just been thinking about doses column again.. we said don’t have it adjust quantity and quantity not to adjust doses or enforce limits etc other than what we have now. I'm thinking if we don't, we will need to show an amount of doses column, won't we?

@andreievg
Copy link
Contributor

@joshxg, how hard would it be to add an on focus change event (on top of on data change)?

@andreievg
Copy link
Contributor

Actually what I was thinking about won't help, am i correct to say that on focus change fires after on button click (if say you change quantity and without exiting the cell press finalise) ?

@josh-griffin
Copy link
Contributor Author

@andreievg

yup it'll be the same behaviour as what it used to be - button click before on losing focus. I think having two columns which are 'dependent' or 'coupled' is really annoying.. same thing for reasons.. and even the stocktake item/batch quantity columns .. it's not the greatest user experience :(

Trying to think a little outside the box on this one.. the 'easy' solutions are for a 'doses' column to show the item.doses field, or to 'couple' the columns... What about if the quantity column is actually for doses for a vaccine item? So you enter number of doses and it sets doses, but also sets numberOfPacks equal to the minimum number of packs...? For example, 2 doses per vial.. enter 17.. doses is set to 17 and numberOfPacks set to 9.. but only display 17.. so just one column?

Or.. the little bottom modal popup thing that we use on the current stock page (and soon to be requisition page), could show the item.doses rather than showing a column? Or, it could show numberOfPacks for a vaccine item if the normal column was actually for doses for a vaccine item...?

@andreievg
Copy link
Contributor

hmm, I like where you going but the main idea behind doses during issue was to be able to figure out 'open vial wastage', or how many doses were actually used for a vial. I remember our main issue was with minimum doses = numberOfPacks (say 10 doses per quantity), so when you enter quantity = 10, and then enter like 100 doses, but want to change to say 19 doses, it goes all haywire (when zero is removed, it would go to 1, which would be auto set to 10, which is the minimum number of doses for quantity 10).

Maybe we don't worry about minimum doses, just restrict by max ?
So then the idea would be: when quantities are changed, change doses to dose per vial * quantity, when doses are changed, restrict max allowed doses per quantity, but don't worry about minimum ?

Most vaccines btw are 1 vial = 1 dose, there are a couple with more then 1, say BCG = 10 doses per vial.

I think having two columns which are 'dependent' or 'coupled' is really annoying

I agree, what you reckon, in ideal world when one tries to finalise and things are missing or incorrect, each 'wrong' thing is highlighted, with maybe exclamation mark you can press (when you press it it tells you what is wrong) ?

@josh-griffin
Copy link
Contributor Author

Maybe we don't worry about minimum doses, just restrict by max ?

Yeah that'll work too, nice.

I agree, what you reckon, in ideal world when one tries to finalise and things are missing or incorrect, each 'wrong' thing is highlighted, with maybe exclamation mark you can press (when you press it it tells you what is wrong) ?

even showing the indication that something is wrong after losing focus is good, I think!

@josh-griffin
Copy link
Contributor Author

@andreievg

Do you think auto-applying a vvm status is a good idea? We have added a level field - so maybe applying the lowest level vvm status after batches are synced for a SI, and when adding a StocktakeBatch/TransactionBatch, auto-apply it there?

@andreievg
Copy link
Contributor

@joshxg, sounds good to me.

@wlthomson
Copy link
Contributor

Update:

@joshxg off for the remainder of this week and part-time for a bit, so will go ahead and pick up the remaining mobile issues.

@Chris-Petty going to take care of the main desktop UI: https://github.com/sussol/msupply/issues/6089, https://github.com/sussol/msupply/issues/6116.

To ensure sufficient time and to ensure compatibility, all vaccines work is targetted for desktop v12, as the PR deadline for v11 is too close. This gives plenty of time for development + testing + documentation.

@josh-griffin
Copy link
Contributor Author

I'm so sick of this issue - I'm gonna close and use #3161 for new additions..!

mobile automation moved this from To do to Needs testing Nov 26, 2020
@josh-griffin josh-griffin unpinned this issue Nov 27, 2020
@josh-griffin josh-griffin moved this from Needs testing to Done in mobile Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic A discussion and/or parent issue for module/feature design
Projects
mobile
  
Done
Development

No branches or pull requests

3 participants