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.

Sign up for GitHub

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

Jump to bottom

Update Licenses available for add-on developers to choose from on AMO #22818

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

Update Licenses available for add-on developers to choose from on AMO #22818

wants to merge 1 commit into from
+199 −63

Conversation

bakulf
Copy link
Contributor

@bakulf bakulf commented Nov 4, 2024

Fixes: mozilla/addons/15092

Description

WIP - do not review this code yet.

Context

Testing

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 marked this pull request as draft November 4, 2024 14:59
@bakulf bakulf force-pushed the licenses branch 2 times, most recently from f51b50e to 0ab26c2 Compare November 7, 2024 08:37
KevinMind
KevinMind reviewed Nov 7, 2024
View reviewed changes
src/olympia/applications/management/commands/import_new_licenses.py
# a new set of licenses.


class Command(BaseCommand):
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to simply stop importing licenses from prod? I'm not really sure why we have to do that in the first place and it very likely could be the case this is just some historical artifact. If we have the licenses we want now defined in a migration.. why not just use the migration to populate the local DB and remove the logic that would remove them.

Copy link
Contributor Author

@bakulf bakulf Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We must import the new licenses to prod in order to show them. We have to run this command to prod.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your migration will get run in prod once this PR is shipped... so I'm not sure why we need a command for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The migration does not create new licenses. It changes the built-in values of anything already existing in the DB, which is consistent with what we have for the previous licenses.

@bakulf bakulf force-pushed the licenses branch 3 times, most recently from 560d912 to eadc82f Compare November 7, 2024 11:49
@bakulf bakulf changed the title WIP: Update Licenses available for add-on developers to choose from on AMO Update Licenses available for add-on developers to choose from on AMO Nov 7, 2024
@bakulf bakulf requested a review from KevinMind November 7, 2024 12:16
@bakulf bakulf marked this pull request as ready for review November 10, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KevinMind KevinMind Awaiting requested review from KevinMind

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bakulf @KevinMind