Skip to content

Commit 5e42b37

Browse files
committed
migrate PrimaryHero model to use addon instead of promoted_addon in a migration safe way (#23333)
* migrate PrimaryHero model to use addon instead of promoted_addon in a migration safe way - Changed the `addon` field in the `PrimaryHero` model to be nullable to be able to continue querying before the migration is complete - Introduced a new task `sync_primary_hero_addon` to synchronize the `addon` field with the `promoted_addon` where applicable. * Fix from review
1 parent c047017 commit 5e42b37

File tree

5 files changed

+71
-9
lines changed

5 files changed

+71
-9
lines changed

src/olympia/hero/migrations/0020_primaryhero_addon.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,4 @@ class Migration(migrations.Migration):
3636
name='addon',
3737
field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='addons.addon', null=True),
3838
),
39-
# Migrate promoted_addon to addon
40-
migrations.RunPython(primary_hero_addon_up, primary_hero_addon_down),
41-
# Make the addon field non-nullable after populating the existing data
42-
migrations.AlterField(
43-
model_name='primaryhero',
44-
name='addon',
45-
field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='addons.addon', null=False),
46-
),
4739
]

src/olympia/hero/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class PrimaryHero(ModelBase):
179179
promoted_addon = models.OneToOneField(
180180
PromotedAddon, on_delete=models.CASCADE, null=True
181181
)
182-
addon = models.OneToOneField(Addon, on_delete=models.CASCADE, null=False)
182+
addon = models.OneToOneField(Addon, on_delete=models.CASCADE, null=True)
183183

184184
is_external = models.BooleanField(default=False)
185185

src/olympia/hero/tasks.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import olympia.core.logger
2+
from olympia.amo.celery import task
3+
from olympia.amo.decorators import use_primary_db
4+
from olympia.hero.models import PrimaryHero
5+
6+
7+
log = olympia.core.logger.getLogger('z.hero')
8+
9+
10+
@task
11+
@use_primary_db
12+
def sync_primary_hero_addon():
13+
invalid_heroes = []
14+
for hero in PrimaryHero.objects.filter(addon__isnull=True):
15+
promoted_addon = hero.promoted_addon
16+
17+
if promoted_addon is None:
18+
invalid_heroes.append(hero)
19+
else:
20+
log.info(f'Syncing hero {hero} with promoted addon {hero.promoted_addon}')
21+
hero.addon = promoted_addon.addon
22+
hero.save()
23+
24+
if len(invalid_heroes) > 0:
25+
log.error(f'Invalid PrimaryHero records: {invalid_heroes}')

src/olympia/hero/tests/test_tasks.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
from olympia.amo.tests import TestCase, addon_factory
2+
from olympia.hero.models import PrimaryHero
3+
from olympia.hero.tasks import sync_primary_hero_addon
4+
from olympia.promoted.models import PromotedAddon
5+
6+
7+
class TestSyncPrimaryHeroAddon(TestCase):
8+
def test_no_heros(self):
9+
assert PrimaryHero.objects.count() == 0
10+
sync_primary_hero_addon.apply()
11+
assert PrimaryHero.objects.count() == 0
12+
13+
def test_with_legacy_promoted_addon(self):
14+
addon = addon_factory()
15+
# Create legacy promoted_addon
16+
promoted_addon = PromotedAddon.objects.create(addon=addon)
17+
# Also make the addon promoted with the new promoted addon model
18+
self.make_addon_promoted(addon, 1)
19+
hero = PrimaryHero.objects.create(promoted_addon=promoted_addon)
20+
sync_primary_hero_addon.apply()
21+
hero.reload()
22+
assert hero.addon == addon
23+
24+
def test_with_new_promoted_addon(self):
25+
addon = addon_factory()
26+
self.make_addon_promoted(addon, 1)
27+
hero = PrimaryHero.objects.create(addon=addon)
28+
sync_primary_hero_addon.apply()
29+
hero.reload()
30+
assert hero.addon == addon
31+
32+
def test_with_no_addon_or_legacy_promoted_addon_should_raise(self):
33+
"""
34+
During the transition to using `addon` instead of `promoted_addon`,
35+
It is possible that there are some heroes that have neither an addon
36+
or a promoted addon. This should not happen, but we should handle the edge case
37+
until we have removed the legacy promoted_addon field.
38+
"""
39+
PrimaryHero.objects.create()
40+
41+
with self.assertLogs(logger='z.hero', level='ERROR') as cm:
42+
sync_primary_hero_addon.apply()
43+
44+
assert 'Invalid PrimaryHero records' in cm.output[0]

src/olympia/lib/settings_base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,7 @@ def get_db_config(environ_var, atomic_requests=True):
743743
'olympia.files.tasks.backfill_file_manifest': {'queue': 'adhoc'},
744744
'olympia.files.tasks.extract_host_permissions': {'queue': 'adhoc'},
745745
'olympia.lib.crypto.tasks.bump_and_resign_addons': {'queue': 'adhoc'},
746+
'olympia.hero.tasks.sync_primary_hero_addon': {'queue': 'adhoc'},
746747
# Misc AMO tasks.
747748
'olympia.blocklist.tasks.monitor_remote_settings': {'queue': 'amo'},
748749
'olympia.abuse.tasks.appeal_to_cinder': {'queue': 'amo'},

0 commit comments

Comments
 (0)