Fixes #33936 - Delete Pools with null subscription_id before adding not-null constraint

[jturel]

What are the changes introduced in this pull request?

Corrects a DB migration to delete Pools having a null subscription ID (this is safe - they are effectively orphaned) before running the rest of the migration that says "subscription ID can't be nil"

What is the thinking behind these changes?

Failed DB migrations == Bad
Working DB migrations == Good

What are the testing steps for this pull request?

Hmm, I recommend simply evaluating that the code change will meet the requirements I've described here otherwise I'm open to ideas as testing the entire upgrade process seems a bit extreme for this :)

Notice that the migration already handled this case for null organization_id but missed subscription ID. Oops!

[theforeman-bot]

Issues: #33936

[jeremylenz]

Makes sense to me!

Is it safer to add a new migration instead of editing this one? That way users who hit this issue could simply run migrations to fix it.

[jturel]

Is it safer to add a new migration instead of editing this one? That way users who hit this issue could simply run migrations to fix it.

At the end of the day something needs to be cherry-picked back to get the migrations passing for those hitting this bug ie running the migrations again is necessary either way. A new migration would be executed last without changing the filename in order to insert it before the one I've changed here. That seems a bit odd especially since it incurs a new migration for those who have already ran the one I edited here successfully. If the case were that the original migration passed but put the db into a bad state then that would be the right time for a new migration to correct the broken data.

[jeremylenz]

makes sense, thanks @jturel !