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

Follow up for "Migrate from BlockVersion.soft to BlockVersion.block_type" #22821

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bakulf
Copy link
Contributor

@bakulf bakulf commented Nov 4, 2024

Fixes: mozilla/addons/issues/15111

Description

This PR is the follow up for #22811. It does the following:

  • Remove the 'soft' field
  • Make 'block_type' not nullable in the model and in the DB

Context

See #22811.

Testing

No new tests needed.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@bakulf bakulf changed the base branch from master to positiveTinyInteger November 4, 2024 20:44
@bakulf bakulf requested a review from KevinMind November 5, 2024 10:08
Base automatically changed from positiveTinyInteger to master November 5, 2024 10:59
@willdurand
Copy link
Member

Fixes: mozilla/addons/issues/15111

This appears to be fixed already. Either the other PR shouldn't have closed as fixed the issue, or another issue is needed :)

('blocklist', '0037_alter_blockversion_block_type_and_more'),
]

operations = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This migration should also have a reversible mapping of soft -> block_type as there could be rows with soft defined and block_type not defined. That should happen before the alterfield as re-introducing the null constraint could raise in those cases.

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.

[Task]: Migrate from BlockVersion.soft to BlockVersion.block_type
3 participants