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

Re-order temperature_log and temperature_breach integration sync #291

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

jmbrunskill
Copy link
Contributor

@jmbrunskill jmbrunskill commented Feb 8, 2024

Fixes #290

Sends the temperature breach records first, then temperature logs, which means the temperature logs changes won't fail with a foreign key constraint.

Note: Accidentally committed this to the develop branch 🫣

See: msupply-foundation/open-msupply#2901 for more context if required

@jmbrunskill jmbrunskill changed the title Re-order temperature_log and temperature_breach integration ordering Re-order temperature_log and temperature_breach integration sync Feb 8, 2024
@mark-prins
Copy link
Contributor

I had removed the FK on the temperature log table to prevent this issue without changing CCA. Odd that the FK is still there.
I was concerned that older versions of CCA would sync with OMS and have issues - how would you work around that? A version check for the API?

@jmbrunskill
Copy link
Contributor Author

how would you work around that?

I have a suggestion PR which creates a temporary record here?
msupply-foundation/open-msupply#2913

I had removed the FK on the temperature log table to prevent this issue without changing CCA. Odd that the FK is still there.

            CREATE TABLE temperature_log (
                id TEXT NOT NULL PRIMARY KEY,
                temperature {DOUBLE} NOT NULL,
                sensor_id TEXT NOT NULL REFERENCES sensor(id),
                store_id TEXT NOT NULL REFERENCES store(id),
                location_id TEXT REFERENCES location(id),
                datetime {DATETIME} NOT NULL,
                temperature_breach_id TEXT REFERENCES temperature_breach(id)
            );   

I discussed the same idea with Andrei yesterday (e.g. removing the Foreign key, but he was concerned about it)
I put some other thoughts here in the OMS PR
msupply-foundation/open-msupply#2901

Copy link
Contributor

@mark-prins mark-prins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks James

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync Order causes problems with open-mSupply
2 participants