Skip to content

Remove emergency alerts code#3976

Merged
klssmith merged 3 commits into
mainfrom
remove-emergency-alerts-code
Jul 31, 2025
Merged

Remove emergency alerts code#3976
klssmith merged 3 commits into
mainfrom
remove-emergency-alerts-code

Conversation

@leohemsted

@leohemsted leohemsted commented Dec 14, 2023

Copy link
Copy Markdown
Contributor

This doesn't deal with any database migration, but it does get rid of all code.

TODO before we merge:

  • check nothing in admin app will break. for example, what happens with existing older broadcast templates? do they still exist, are they viewable? will they break if you go view them?
  • check nothing with redis will break
  • figure out what to do with broadcast data in the db. especially tables that are used for other things (eg templates, services, etc)

@quis

quis commented Dec 19, 2023

Copy link
Copy Markdown
Member

At the moment if you try to view a broadcast template in the admin app you will get an exception (the app doesn’t know how to preview one of these templates).

But the only way you can get to that page is by manually entering the id of the template in the URL. You can’t see a list of those templates on the templates page for example. I don’t think this PR will change that behaviour, so I don’t think handling it needs to be a blocker to merging.

We could put in a workaround to return a 404. Or we could do a migration later on to delete all broadcast templates.

@leohemsted

leohemsted commented Dec 19, 2023

Copy link
Copy Markdown
Contributor Author

i think to start with we could put 404s on any broadcast template or broadcast service endpoint, and then delete the data later

i'd be keen on deleting the data in the DB earlier rather than later so we dont get surprised by weird data if we want to update schemas on those tables or migrate data based on columns listed in the models or things like that

@idavidmcdonald

Copy link
Copy Markdown
Contributor

FYI, we can't delete any of the broadcast data out of our database for 1 year please. Either that or we need to take a snapshot of that data and store it elsewhere for a year. This is because emergency alerts need it as an audit trail for any security incidents (precedent is to store it for a year, just like Notify does).

The 1 year is from the time they migrated to their own system (was that like early November maybe?)

@leohemsted

Copy link
Copy Markdown
Contributor Author

thanks, that's useful to know david. I'd probably be keen on snapshotting and storing it in a pgdump in s3 ideally

@leohemsted leohemsted marked this pull request as draft December 28, 2023 11:40
@leohemsted leohemsted force-pushed the remove-emergency-alerts-code branch from bbf8b56 to 4bbec83 Compare January 10, 2024 21:15
@leohemsted leohemsted force-pushed the remove-emergency-alerts-code branch from 4bbec83 to c6d66eb Compare February 7, 2024 10:07
@leohemsted leohemsted force-pushed the remove-emergency-alerts-code branch from c6d66eb to 85bbc98 Compare September 27, 2024 15:32
@leohemsted leohemsted force-pushed the remove-emergency-alerts-code branch 2 times, most recently from 9ce67d6 to dd6210a Compare October 1, 2024 14:54
@leohemsted leohemsted marked this pull request as ready for review October 1, 2024 15:03
Leo Hemsted added 2 commits July 28, 2025 16:51
no longer used. there might be additional cleanup we can do of other
utils functions that dont have "alert" or "broadcast" in the name that
are no longer used
the schema was originally created so that it could not show content.
However, then it needed content conditionally for broadcast messages so
content was added back in.

Now that broadcasts no longer exist we can revert that - however, for a
template that has a specific list of expected keys, it's nicer to just
list those rather than maintain a huge list of exceptions that needs to
be modified every time a field changes on the template model
@klssmith klssmith force-pushed the remove-emergency-alerts-code branch from dd6210a to e45afcd Compare July 28, 2025 15:52
@klssmith klssmith force-pushed the remove-emergency-alerts-code branch from e45afcd to 394d124 Compare July 30, 2025 06:46
@klssmith

Copy link
Copy Markdown
Contributor

This has been checked and rebased to get rid of merge conflicts. The data has been deleted, so the comments relating to that no longer apply.

The failure of the migration-isolation-check ("Please separate migration changes from other code changes into separate PRs. This helps ensure safer deployments and easier rollbacks if needed") is fine - we're only removing a couple of unused imports from the migration files, not making any database changes.

@klssmith klssmith merged commit ca4bb97 into main Jul 31, 2025
5 of 6 checks passed
@klssmith klssmith deleted the remove-emergency-alerts-code branch July 31, 2025 07:00
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.

4 participants