rm PROMOTED_GROUP_CHOICES#25106
Conversation
WIP TODO: filters.py changes are lacking - need way to replicate badged
eviljeff
force-pushed
the
15976-rm-promoted-group-choices-references
branch
2 times, most recently
from
657bb5d to
16a4768
Compare
eviljeff
force-pushed
the
15976-rm-promoted-group-choices-references
branch
from
16a4768 to
8c43966
Compare
eviljeff
changed the title
eviljeff
force-pushed
the
15976-rm-promoted-group-choices-references
branch
from
21c7d2a to
ed8041a
Compare
eviljeff
requested review from
a team and
chrstinalin
and removed request for
a team
| STATIC_THEMES_REVIEW = AclPermission('Addons', 'ThemeReview') | ||
| # Can review recommend(ed|able) add-ons | ||
| ADDONS_RECOMMENDED_REVIEW = AclPermission('Addons', 'RecommendedReview') | ||
| # Can triage (and therefore see in the queues) add-ons with a temporary delay |
There was a problem hiding this comment.
Just out of curiosity, is there no need for an equivalent to this (can review add-ons with promotions), or is that going to just fall to admins only?
There was a problem hiding this comment.
Some of the groups are admin only, but the others are any reviewer (not that there's a differentiation at the moment - all reviewers are admin reviewers). This drops Recommended as a being a special group with a dedicated permission
| } | ||
| BADGED_GROUPS = ['line', 'recommended'] # special badged groups | ||
| RECOMMENDED_API_NAME = 'recommended' # Name of the "original" promoted group | ||
| NOTABLE_API_NAME = 'notable' # Name of the group we automatically add add-ons to |
There was a problem hiding this comment.
theres some mix of using these vs. directly using a string in the tests
There was a problem hiding this comment.
it's supposed to only be the constant in tests where the api_name is important (which should be very few), and a random string otherwise but it's inconsistent and most of the time the test was directly changed from referencing a constant named RECOMMENDED to a string "recommended" ... I'll have a look through...
| listed_pre_review=True, | ||
| badged=True, | ||
| ) | ||
| self.grant_permission(self.reviewer, 'Addons:RecommendedReview') |
There was a problem hiding this comment.
leftover Addons:RecommendedReview
There was a problem hiding this comment.
Also at src/olympia/reviewers/decorators.py:50
| assert doc('.is_promoted') | ||
| for entry in doc('.is_promoted').items(): | ||
| assert entry.text() == ( | ||
| "This is a Spotlight add-on. You don't have permission to review it." |
There was a problem hiding this comment.
I don't see this 'You don't have permission to review it' behaviour being tested elsewhere, I don't think it can be removed wholesale
|
|
||
|
|
||
| class TestPromotedGroup(TestCase): | ||
| def test_deactived_group_ids_raise(self): |
There was a problem hiding this comment.
Since active is still a field on the model, I don't think this test should be removed wholesale
There was a problem hiding this comment.
was active supposed to be just for the sponsored and verified groups? I assumed it was more generic than that. If it's just for those groups we can drop the model field too.
2 participants
Fixes mozilla/addons#15976
Description
Removes PROMOTED_GROUP_CHOICES from live and test code. Also drops
Addons:RecommendedReviewpermission.Context
Almost the last part of removing all constants relating to Promoted groups - with the aim that PromotedGroups can be added and edited freely in production without code changes or migrations.
Almost, because we still need to know what
badgedgroup means, without the database; and addons-frontend still has some hard-coded constants controlling the behavior of the groups it knows about.I made the changes in code, and had claude update the tests.
Testing
Checklist
#ISSUENUMat the top of your PR to an existing open issue in the mozilla/addons repository.