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

Cold chain: Handle ongoing breaches (which can't easily be resolved) #3207

Open AnushaUp opened 8 months ago

AnushaUp commented 8 months ago

What went wrong? 😲

Adam comments:

Anusha's comment raises some good questions about how we handle ongoing breaches that are unlikely to be resolved in the normal manner (i.e. by recording a 'normal' 2-8 °C temperature reading once again).

Reasons for not being able to record a normal non-breach temperature reading could include:

We need to think of a way for users to 'acknowledge' this type of breach situation as well as the normal 'breach resolved' situation.

Perhaps we need a special option in the acknowledge modal that the user can choose in these situations so they can remove these annoying ever-living breaches.

E.g.

I.e. a button that says Sensor malfunctioned or missing or something. And if the user presses it it tells them Selecting this will acknowledge the ongoing breach and disable future temperature readings from this sensor

But also makes me wonder how we would deal with a Fridge-tag that is breaching by the time it's plugged into Open mSupply - that would also be a never-ending breach but the user would want to be able to read from the Fridge-tag next time too...

Note: this was already flagged for consideration in #2718 but it got accidentally closed.


As per the title, the Oms application is unable to update the sensor duration when the sensor is removed from the CCA app in the middle of breach ongoing. In this image the selected sensor is already removed form the sensor and still showing ongoing after 2/3 hrs. Tried restarting the omSupply also. ![Screenshot 2024-03-06 at 2 57 43 pm](https://github.com/msupply-foundation/open-msupply/assets/115983430/903160b8-29af-4d88-ba1d-c5c9f5b56143) ## Expected behaviour 🤔 The status should be updated if there's no sensor maybe. ## How to Reproduce 🔨 Steps to reproduce the behaviour: 1. Setup CCA and OMS applications 2. Make condition for breach info 3. Remove the sensor when the breach is ongoing 4. See error ## Your environment 🌱 Found while Testing [THIS](https://github.com/msupply-foundation/open-msupply/issues/3006)

Agreed Solution

adamdewey commented 8 months ago

Yeah, this raises a good point - how does the user deal with a situation when they can't ever receive a 'normal' temperature from a sensor again?

E.g. if the sensor is broken, or removed from the system.

We need to think of a way for users to 'acknowledge' this type of situation as well as the normal 'breach resolved' situation.

Perhaps we need to a special option in the acknowledge modal that the user can choose in these situations so they can remove these annoying ever-living breaches.

E.g.

I.e. a button that says Sensor malfunctioned or missing or something. And if the user presses it it tells them Selecting this will acknowledge the ongoing breach and disable future temperature readings from this sensor

But also makes me wonder how we would deal with a Fridge-tag that is breaching by the time it's plugged into Open mSupply - that would also be a never-ending breach but the user would want to be able to read from the Fridge-tag next time too...

Hmm, this requires some thought.

I'm changing the title of the issue to be Handle ongoing breaches (which can't easily be resolved) and the type to feature

Chris-Petty commented 8 months ago

@adamdewey just gonna flag that it's weird to insert your discussion into some one else's issue description - it reads like Anusha has written in 3rd person doing a review of her own comments. It would have been better in a comment.


Just simplifying the situation a bit - why can't we just acknowledge breaches that are currently occurring and they go away? Say you close the door to the fridge and acknowledge the breach, but it takes 30mins of out of temp logs before the fridge cools. Surely the breach should be silenced, though not forever (maybe the open door was only part of the problem?)

For fridge tags yea might need special handling for non-live temperature logs. If the breach is acknowledge then it just goes away forevs.

Chris-Petty commented 8 months ago

This might be a must have...

adamdewey commented 8 months ago

@adamdewey just gonna flag that it's weird to insert your discussion into some one else's issue description - it reads like Anusha has written in 3rd person doing a review of her own comments. It would have been better in a comment.

Oh yes you're right, sorry!

I think it's because the original issue that was meant to be the feature issue was closed - so was trying to change this into a feature request, but didn't work so well 🤦

adamdewey commented 8 months ago

Just simplifying the situation a bit - why can't we just acknowledge breaches that are currently occurring and they go away? Say you close the door to the fridge and acknowledge the breach, but it takes 30mins of out of temp logs before the fridge cools. Surely the breach should be silenced, though not forever (maybe the open door was only part of the problem?) For fridge tags yea might need special handling for non-live temperature logs. If the breach is acknowledge then it just goes away forevs.

Yes, we could just allow acknowledging of ongoing breaches.

I don't think a simple snooze would work though because you'd get reminded forever for the broken sensor scenario.

But also, somehow you need to remind the user to sort out a genuine issue if it's going on for a long time.

Chris-Petty commented 8 months ago

Related: https://github.com/msupply-foundation/open-msupply/issues/2599

adrianwb commented 8 months ago

I can't speak for BT sensors, but for Fridgetags, I don't see any great problem in letting the user acknowledge an ongoing breach. The next time it's plugged in, it'll flag the same breach again, although we could deliberately change our code to ignore it?

I'm assuming that we're only allowing the user to acknowledge breaches in the (active) store they're currently logged into, and not when logged into the same (collector) store on the central server?

Chris-Petty commented 8 months ago

@adrianwb

I can't speak for BT sensors, but for Fridgetags, I don't see any great problem in letting the user acknowledge an ongoing breach. The next time it's plugged in, it'll flag the same breach again, although we could deliberately change our code to ignore it?

Yea I agree!

I'm assuming that we're only allowing the user to acknowledge breaches in the (active) store they're currently logged into, and not when logged into the same (collector) store on the central server?

I think that's a good assumption, I think breaches are store data so only active store should touch it. That said, a sync race condition over who acknowledged it doesn't matter much (remote pushes before pull and will override who acknowledged it if that's even a thing)

On a different note - if you're logged into a different store than the one having a breach, do you get notified regardless in OMS? Part of me says no because realistically having multiple stores on a site is usually something like pharmacies on different side of the hospital... but in Rarotonga the pharmacy is 100m away from the central warehouse and the staff constantly move between. If working in the pharmacy I'd want to be notified if a fridge door in the warehouse was left open! Perhaps it should be a site setting... Not to mention complexity regarding whether or not the current user can login to the store that is having a breach occur 😵

adamdewey commented 8 months ago

I can't speak for BT sensors, but for Fridgetags, I don't see any great problem in letting the user acknowledge an ongoing breach. The next time it's plugged in, it'll flag the same breach again,

Do you mean the '2nd half' of the original breach will be flagged again? (i.e. the temperature logs that were recorded after the Fridge-tag is disconnected from the computer and before its temperature returns to normal once back in the fridge)

For the '1st half' of the breach - we wouldn't import the same data again if it already exists in the DB would we?

I'm assuming that we're only allowing the user to acknowledge breaches in the (active) store they're currently logged into, and not when logged into the same (collector) store on the central server?

I think that's a safe assumption for now - can't think of a use case for why we would need to allow it on the collector store, and so don't think this needs to be considered unless a need is identified in the field?

adamdewey commented 8 months ago

On a different note - if you're logged into a different store than the one having a breach, do you get notified regardless in OMS? Part of me says no because realistically having multiple stores on a site is usually something like pharmacies on different side of the hospital... but in Rarotonga the pharmacy is 100m away from the central warehouse and the staff constantly move between. If working in the pharmacy I'd want to be notified if a fridge door in the warehouse was left open! Perhaps it should be a site setting... Not to mention complexity regarding whether or not the current user can login to the store that is having a breach occur 😵

That's a good point... hmm

I think this is actually part of a broader question: how do we provide an elegant UX for users that have responsibilities that span more than 1 store?

Similar questions have come to my mind for the CCEI work for national and regional administrators - i.e. users that need to be able to perform actions on or across multiple stores without having to log into the individual stores.

This is the flip side - how to provide alerts to users from multiple stores even if they're logged into an individual store.

I like your idea of configurable settings for the user Chris - it would be good if the user was able to configure what stores they want to be able to receive alerts from.

But yeah, I think this can be separated into a different issue?

adrianwb commented 8 months ago

I can't speak for BT sensors, but for Fridgetags, I don't see any great problem in letting the user acknowledge an ongoing breach. The next time it's plugged in, it'll flag the same breach again,

Do you mean the '2nd half' of the original breach will be flagged again? (i.e. the temperature logs that were recorded after the Fridge-tag is disconnected from the computer and before its temperature returns to normal once back in the fridge)

For the '1st half' of the breach - we wouldn't import the same data again if it already exists in the DB would we?

It's the same breach - we'd just be updating the end timestamp and duration, but we wouldn't be re-importing temperature logs that had previously been imported, or any previous (completed) breaches.

adamdewey commented 8 months ago

It's the same breach - we'd just be updating the end timestamp and duration, but we wouldn't be re-importing temperature logs that had previously been imported, or any previous (completed) breaches.

Ah ok, I wonder how the acknowledgement functionality would handle that - i.e. the breach timestamp and duration changing after it has been 'acknowledged' ?

jmbrunskill commented 5 months ago

@adamdewey can you confirm you're happy with the Agreed solution section. Hopefully strikes a good balance between easy to implement and meets the needs of allowing users to ignore sensors they now don't want to see.

adamdewey commented 5 months ago

@jmbrunskill - looks great!

Couple of things:

jmbrunskill commented 5 months ago

@adamdewey Makes sense, we won't be the filter in the monitoring tab then.