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

186 cold chain alerts should default to enabled #187

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

jmbrunskill
Copy link
Collaborator

Closes #186

πŸ‘©πŸ»β€πŸ’» What does this PR do?

Default to all alerts turned on for Cold Chain

image

πŸ§ͺ How has/should this change been tested?

  1. Create a new cold chain alert.
  2. Check the defaults are correct
  3. Check that the existing configurations haven't be inadvertently updated.

πŸ’Œ Any notes for the reviewer?

I've also removed the backwards compatibility code for recipientIds, as I think the only placing which was still using this old method has been updated now.

@jmbrunskill jmbrunskill linked an issue Oct 10, 2023 that may be closed by this pull request
@github-actions
Copy link

Bundle size difference

Comparing this PR to main

Old size New size Diff
2.1 MB 2.1 MB 3 KB (0.14%)

@mark-prins
Copy link
Contributor

Is this one targeting the right branch to merge into? There's a lot of files changed for what sounds like a simple task πŸ€”

@jmbrunskill jmbrunskill changed the base branch from main to 171-cold-chain-should-send-high-and-low-temperature-alerts October 13, 2023 01:33
@jmbrunskill
Copy link
Collaborator Author

Is this one targeting the right branch to merge into? There's a lot of files changed for what sounds like a simple task πŸ€”

Nope! Good catch should be right now.

Base automatically changed from 171-cold-chain-should-send-high-and-low-temperature-alerts to main October 13, 2023 02:46
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.

πŸ‘ LGTM!

@mark-prins mark-prins self-assigned this Oct 16, 2023
@jmbrunskill jmbrunskill merged commit 977aa51 into main Oct 16, 2023
@jmbrunskill jmbrunskill deleted the 186-cold-chain-alerts-should-default-to-enabled branch October 16, 2023 20:29
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.

Cold chain alerts should default to Enabled
2 participants