Skip to content

Prevent creation of PromotedAddonVersion for PromotedApproval with null application_id; added corresponding test case. #23217

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

Merged
merged 2 commits into from
Mar 25, 2025

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Mar 24, 2025

Fixes: mozilla/addons#15488

Testing

  1. create a PromotedApproval from any version with a null application_id
  2. run sync_promoted_addons command
  3. expect command to pass and not create any new PromotedAddonVersion instances.

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.

@KevinMind KevinMind requested a review from chrstinalin March 24, 2025 16:33
@KevinMind KevinMind force-pushed the kevinmind/addons/15488 branch from 8c8ce8f to 553ee1b Compare March 24, 2025 20:40
@KevinMind KevinMind requested a review from chrstinalin March 24, 2025 20:57
@KevinMind KevinMind force-pushed the kevinmind/addons/15488 branch from 553ee1b to b9cabea Compare March 24, 2025 21:17
@KevinMind KevinMind changed the base branch from kevinmind/addons/15486 to master March 24, 2025 21:26
@KevinMind KevinMind force-pushed the kevinmind/addons/15488 branch from b9cabea to 2335f4e Compare March 24, 2025 21:28
@KevinMind KevinMind force-pushed the kevinmind/addons/15488 branch from 2335f4e to d601180 Compare March 25, 2025 17:05
@KevinMind KevinMind merged commit 7757e31 into master Mar 25, 2025
42 checks passed
@KevinMind KevinMind deleted the kevinmind/addons/15488 branch March 25, 2025 17:24
@KevinMind
Copy link
Contributor Author

Verified on dev

olympia@addons-server-v1-deploy-uwsgi-amo-web-584b8c87b7-mvm6z:~$ ./manage.py verify_promoted_addons
All PromotedAddon and PromotedAddonPromotion records are in sync
olympia@addons-server-v1-deploy-uwsgi-amo-web-584b8c87b7-mvm6z:~$ 

Using this command

from django.core.management.base import BaseCommand, CommandError
from django.db.models import Q

from olympia.constants.promoted import PROMOTED_GROUP_CHOICES
from olympia.promoted.models import (
    PromotedAddon,
    PromotedAddonPromotion,
    PromotedAddonVersion,
    PromotedApproval,
)


class Command(BaseCommand):
    help = (
        'Verify PromotedAddon -> PromotedAddonPromotion sync'
        'and PromotedApproval -> PromotedAddonVersion sync'
    )

    def log_issue(self, from_model, to_model, addon, group, application):
        self.stdout.write(self.style.ERROR(
            f"Missing {to_model} for {from_model}: \n"
            f"addon={addon} \n"
            f"group={group} \n"
            f"application={application}"
        ))

    def verify_promoted_addon_promotions(self):
        issues_found = 0

        for promoted_addon in PromotedAddon.objects.exclude(
            group_id=PROMOTED_GROUP_CHOICES.NOT_PROMOTED
        ):
            for app in promoted_addon.all_applications:
                promotion = PromotedAddonPromotion.objects.filter(
                    addon=promoted_addon.addon,
                    application_id=app.id,
                    promoted_group__group_id=promoted_addon.group_id
                ).exists()

                if not promotion:
                    issues_found += 1
                    self.log_issue(
                        'PromotedAddon',
                        'PromotedAddonPromotion',
                        promoted_addon.addon,
                        promoted_addon.get_group_id_display(),
                        app.short
                    )
        return issues_found

    def verify_promoted_addon_addons(self):
        issues_found = 0

        for promotion in PromotedAddonPromotion.objects.all():
            # Skip checking if group is NOT_PROMOTED
            if promotion.promoted_group.group_id == PROMOTED_GROUP_CHOICES.NOT_PROMOTED:
                continue

            # Check if there's a corresponding PromotedAddon
            # A PromotedAddon matches if:
            # 1. It has the same addon
            # 2. It has the same group_id
            # 3. Its application_id is either null (meaning all apps) OR matches this app
            promoted_addon = PromotedAddon.objects.filter(
                addon=promotion.addon,
                group_id=promotion.promoted_group.group_id
            ).filter(
                Q(application_id__isnull=True) | Q(application_id=promotion.application_id)
            ).exists()

            if not promoted_addon:
                issues_found += 1
                self.log_issue(
                    'PromotedAddonPromotion',
                    'PromotedAddon',
                    promotion.addon,
                    promotion.promoted_group.name,
                    promotion.application.short
                )

        return issues_found

    def verify_promoted_addon_versions(self):
        issues_found = 0

        for approval in PromotedApproval.objects.exclude(
            Q(application_id__isnull=True) | Q(group_id__isnull=True)
        ):
            addon_version = PromotedAddonVersion.objects.filter(
                version=approval.version,
                promoted_group__group_id=approval.group_id,
                application_id=approval.application_id
            ).exists()

            if not addon_version:
                issues_found += 1
                self.log_issue(
                    'PromotedApproval',
                    'PromotedAddonVersion',
                    approval.version.addon,
                    approval.get_group_id_display(),
                    approval.application.short if approval.application else 'Unknown'
                )

        return issues_found

    def verify_promoted_approvals(self):
        issues_found = 0

        for promoted_version in PromotedAddonVersion.objects.all():
            approval = PromotedApproval.objects.filter(
                version=promoted_version.version,
                group_id=promoted_version.promoted_group.group_id,
                application_id=promoted_version.application_id
            ).exists()

            if not approval:
                issues_found += 1
                self.log_issue(
                    'PromotedAddonVersion',
                    'PromotedApproval',
                    promoted_version.version.addon,
                    promoted_version.promoted_group.name,
                    promoted_version.application.short
                )

        return issues_found


    def handle(self, *args, **kwargs):
        missing_promotions = self.verify_promoted_addon_promotions()
        missing_addons = self.verify_promoted_addon_addons()

        missing_versions = self.verify_promoted_addon_versions()
        missing_approvals = self.verify_promoted_approvals()

        issues_found = missing_promotions + missing_addons + missing_versions + missing_approvals

        if issues_found:
            raise CommandError(
                f"Found {issues_found} sync issues"
            )
        else:
            self.stdout.write(self.style.SUCCESS(
                "All PromotedAddon and PromotedAddonPromotion records are in sync"
            ))
            return 0

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.

[Bug]: sync_promoted_addons integrity error when promoted approval has no application_id
2 participants