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.

Sign up for GitHub

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

Jump to bottom

Importing a fridge tag fails if an import has happened in another store #4755

Open
mark-prins opened this issue Sep 9, 2024 · 7 comments · May be fixed by #5071
Open

Importing a fridge tag fails if an import has happened in another store #4755

mark-prins opened this issue Sep 9, 2024 · 7 comments · May be fixed by #5071
Assignees
@jmbrunskill
Labels
bug Something is borken Severity: High Bugs breaking core functionality or with no/unacceptable workaround. High impact. Next patch release Team Piwakawaka James, Laché, Carl
Milestone
Refactor fortnight

Comments

@mark-prins
Copy link
Collaborator

What went wrong? 😲

Given:

  • you are running postgres
  • there are mutliple stores configured for your site
  • a fridge tag has been imported in another store

Then:
any fridge tag import will fail

The issue is that this call to check matching sensor config (berlinger.rs, line 204)

let result = get_matching_sensor_breach_config(
        connection,
        &sensor_row.store_id,
        temperature_breach_config,
        &breach_row_type,
    )?;

    if !result.is_empty() {
        return Ok(());
    };

is checking for a config based on store_id, duration_milliseconds, minimum_temperature, maximum_temperature and type.
However, the constraint on the table is for a unique description and this does not include the store_id, so the insert following the code above will fail.

Expected behaviour 🤔

Able to import the fridge tag data for a new store.

How to Reproduce 🔨

Steps to reproduce the behaviour:

  1. Go to Cold chain / monitoring
  2. Import a fridge tag
  3. Switch to another store and repeat
  4. See error

Your environment 🌱

  • Open mSupply Version:
  • Legacy mSupply Central Server Version:
  • Platform:
  • Database type:
@mark-prins mark-prins added bug Something is borken needs triage needs daily triage New bug to be triaged by the release team labels Sep 9, 2024
@lache-melvin lache-melvin added Severity: Normal Bugs which have an acceptable workaround. Moderate/tolerable user impact. Next minor release. and removed needs daily triage New bug to be triaged by the release team labels Sep 9, 2024
@lache-melvin
Copy link
Contributor

Setting as normal as I don't believe there are any deployed multi-store sites, but you should be able to do this!

@adrianwb
Copy link
Contributor

adrianwb commented Sep 9, 2024

Yes, you shouldn't be able to import the same fridgetag into a different store (you'd need to move it to the new store first), but you should definitely be able to import a different fridgetag

@andreievg andreievg added this to the Refactor fortnight milestone Sep 12, 2024
@andreievg andreievg added Severity: High Bugs breaking core functionality or with no/unacceptable workaround. High impact. Next patch release and removed needs triage Severity: Normal Bugs which have an acceptable workaround. Moderate/tolerable user impact. Next minor release. labels Sep 12, 2024
@andreievg andreievg added the Team Ruru 🦉 Andrei, Roxy, Chris, Ferg label Sep 30, 2024
@andreievg andreievg removed the Team Ruru 🦉 Andrei, Roxy, Chris, Ferg label Oct 9, 2024
@roxy-dao roxy-dao added the Team Piwakawaka James, Laché, Carl label Oct 9, 2024
@jmbrunskill jmbrunskill linked a pull request Oct 10, 2024 that will close this issue
Open
8 tasks
@jmbrunskill jmbrunskill linked a pull request Oct 10, 2024 that will close this issue
Remove unique constraint on breach description #5071
Open
8 tasks
@lache-melvin
Copy link
Contributor

See PR for needs refinement context 🙏

@jmbrunskill
Copy link
Contributor

jmbrunskill commented Oct 10, 2024 •
edited by lache-melvin
Loading

Refinement:

If you import a fridge tag and you're logged into a store, other than the one already associated with the sensor, throw error.
Duplicated configs for different stores is ok...

@lache-melvin lache-melvin mentioned this issue Nov 7, 2024
Open
@adamdewey
Copy link
Contributor

adamdewey commented Nov 7, 2024 •
edited
Loading

Yes, you shouldn't be able to import the same fridgetag into a different store (you'd need to move it to the new store first), but you should definitely be able to import a different fridgetag

How do you move a Fridge-tag to a different store?

@adrianwb
Copy link
Contributor

adrianwb commented Nov 8, 2024 •
edited
Loading

Yes, you shouldn't be able to import the same fridgetag into a different store (you'd need to move it to the new store first), but you should definitely be able to import a different fridgetag

How do you move a Fridge-tag to a different store?

Hmm, I was thinking of the "Set location" button when viewing a sensor on desktop mSupply, but just realised that locations are specific to a store, and you can only see sensors associated with the store you're logged into => you can only ever move it to another location in the same store!

You can un-assign a sensor location though, and that blanks out the sensor's location, but doesn't change its store => if we changed vaccineShowSensor to include sensors with a blank location, then it would show up if you login to a different store and then you could assign a location in the new store (and that would change the sensor's store as well)?

The other issue if you move store is that you'd still want to be able to see the original temperature logs and breaches in the original store. vaccineShowTemperatureBreach filters the breaches by store, so that's fine. vaccineShowLogs filters temperature logs by store, so that's also fine, but it also filters sensors by store, so that would need tweaked so that it filters by locations which belong to the current store instead.

I'd imagine that something similar is also possible in omSupply?

@adamdewey
Copy link
Contributor

adamdewey commented Nov 10, 2024 •
edited
Loading

Hmm, I was thinking of the "Set location" button when viewing a sensor on desktop mSupply, but just realised that locations are specific to a store, and you can only see sensors associated with the store you're logged into => you can only ever move it to another location in the same store!

Ah, I wonder if this is also what happened for me on the oms demo server.

I had sensors (not Fridge-tag) associated with a store, then switched them to a new store on the same site.

The Chart page shows the data ok, but the sensors don't appear in the Sensors page

image

image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jmbrunskill jmbrunskill

Labels
bug Something is borken Severity: High Bugs breaking core functionality or with no/unacceptable workaround. High impact. Next patch release Team Piwakawaka James, Laché, Carl
Projects
None yet
Milestone
Refactor fortnight
Development

Successfully merging a pull request may close this issue.

Remove unique constraint on breach description msupply-foundation/open-msupply
7 participants
@adrianwb @jmbrunskill @mark-prins @adamdewey @andreievg @lache-melvin @roxy-dao