msupply-foundation / open-msupply

Open mSupply represents our most recent advancement in the Logistics Management Information System (LMIS), expanding on more than two decades of development inherited from the well-established legacy of the original mSupply.
https://msupply.foundation/open-msupply/
Other
23 stars 14 forks source link

Duration for ongoing breaches #2430

Open andreievg opened 1 year ago

andreievg commented 1 year ago

What to do for duration for ongoing breaches, carry over from: https://github.com/openmsupply/open-msupply/pull/2398

https://github.com/openmsupply/open-msupply/pull/2398#discussion_r1372926552

Currently we set it to 0. There was a telegram discussion suggesting that when we showing ongoing breach, we should show duration (and example why, would be to indicate to the user the severity of ongoing breach, did it just start, a few minutes, or is it really bad, 10s of hours).

We can still show to the user breach duration, even if it's set to 0 in database, but that would involve:

There is some concern that it's not accurate to set duration in database for the breach if it's still onging, i think that would be true if it was end_duration, but I think it's correct to say that duration for this breach is X, even if it's still ongoing.

@adamdewey @richardmoizeau @mark-prins @louisaw123

mark-prins commented 1 year ago

My view is that we cannot show a duration if there is no end - anything we display would have little meaning.

If the consensus is to display a duration, then we could:

andreievg commented 1 year ago

If the consensus is to display a duration, then we could:

I think it's a lot easer to just set it during API integration (much simpler and allows for sorting/filtering)

adamdewey commented 1 year ago

Hmm, interesting one.

So, as I understand it, the breach is supposed to signify 'It's already potentially too late to save the vaccines - but you need to go and check the VVMs on the vials anyway, you might get lucky and be able to salvage some'.

This is because the system has been configured to only record a breach once the duration required to potentially compromise the vaccines has elapsed. And this is also why we have the concept of an excursion for any period that doesn't qualify as a breach.

So from that point of view there should be relatively little difference between a breach that has just happened and a breach that has been going on for days: the user should go and check the vaccines as soon as they're aware of the breach, they shouldn't say to themselves 'Oh, well it's only been breaching for 10 minutes, I'll start walking faster when it's been breaching for 3 hours'

On the other hand it's probably human nature to be like: 'Oh no, there's been a breach! How long has this been going on for??' and it would be a courtesy for us to do the calculation for them.

adamdewey commented 1 year ago

But as a follow-up, if it's too hard to calculate the duration, I would just put 'Ongoing' in red in the duration column and that should be enough to frighten the user into trying to rectify the situation post haste!

andreievg commented 1 year ago

@mark-prins, just want to summarise our chat.

Since temperature breaches are only sent when updated (when originally inserted or when updated), so we can't really update duration if breach is ongoing.

We can use sensor last connection date though to calculate duration on the go in front end UI.

Also can have custom filter and sort in back end for those two, if needed in the future