From e5de4ecc7c0f4af250fca3a268aac2c3a3b28b97 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Thu, 26 Oct 2023 14:43:31 +0200 Subject: [PATCH] Merge per-application categories, add command to merge and clean up obsolete ones (#21212) Expose categories flat in API v5, add compatibility shim for previous versions. --- docs/topics/api/categories.rst | 138 +++-------- docs/topics/api/overview.rst | 1 + src/olympia/addons/fields.py | 94 ++++---- .../commands/update_and_clean_categories.py | 80 ++++++ src/olympia/addons/models.py | 11 - src/olympia/addons/serializers.py | 10 +- src/olympia/addons/tests/test_commands.py | 106 +++++++- src/olympia/addons/tests/test_models.py | 77 +----- src/olympia/addons/tests/test_serializers.py | 27 ++- src/olympia/addons/tests/test_views.py | 103 ++++---- src/olympia/amo/sitemap.py | 4 +- src/olympia/amo/tests/__init__.py | 6 +- src/olympia/amo/tests/test_sitemap.py | 14 +- src/olympia/constants/categories.py | 227 ++++-------------- src/olympia/devhub/forms.py | 115 +++------ .../devhub/addons/edit/describe.html | 20 +- .../devhub/addons/submit/describe.html | 18 +- .../addons/submit/describe_minimal.html | 2 +- .../templates/devhub/includes/macros.html | 12 +- src/olympia/devhub/tests/test_forms.py | 7 +- src/olympia/devhub/tests/test_views_edit.py | 37 ++- src/olympia/devhub/tests/test_views_submit.py | 23 +- src/olympia/devhub/views.py | 14 +- src/olympia/landfill/generators.py | 9 +- .../management/commands/fetch_prod_addons.py | 4 +- src/olympia/landfill/tests/test_generators.py | 10 +- src/olympia/landfill/tests/test_users.py | 2 +- src/olympia/lib/settings_base.py | 2 + src/olympia/search/filters.py | 8 +- src/olympia/search/tests/test_filters.py | 16 +- static/css/zamboni/developers.css | 12 - 31 files changed, 490 insertions(+), 719 deletions(-) create mode 100644 src/olympia/addons/management/commands/update_and_clean_categories.py diff --git a/docs/topics/api/categories.rst b/docs/topics/api/categories.rst index c30fa30ff7..45f3a78d1f 100644 --- a/docs/topics/api/categories.rst +++ b/docs/topics/api/categories.rst @@ -14,9 +14,9 @@ Category List .. _category-list: -Categories are defined by a name, a slug, a type and an application. Slugs are -only guaranteed to be unique for a given ``app`` and ``type`` combination, and -can therefore be re-used for different categories. +Categories are defined by a name, a slug and a type. Slugs are +only guaranteed to be unique for a given ``type``, and can therefore be re-used +for different categories. This endpoint is not paginated. @@ -25,7 +25,6 @@ This endpoint is not paginated. :>json int id: The category id. :>json string name: The category name. Returns the already translated string. :>json string slug: The category slug. See :ref:`csv table ` for more possible values. - :>json string application: Application, see :ref:`add-on application ` for more details. :>json boolean misc: Whether or not the category is miscellaneous. :>json string type: Category type, see :ref:`add-on type ` for more details. :>json int weight: Category weight used in sort ordering. @@ -39,102 +38,37 @@ Current categories ------------------ .. csv-table:: - :header: "Name", "Slug", "Type", "Application" + :header: "Name", "Slug", "Type" - "Alerts & Updates", alerts-updates, extension, firefox - "Appearance", appearance, extension, firefox - "Bookmarks", bookmarks, extension, firefox - "Download Management", download-management, extension, firefox - "Feeds, News & Blogging", feeds-news-blogging, extension, firefox - "Games & Entertainment", games-entertainment, extension, firefox - "Language Support", language-support, extension, firefox - "Photos, Music & Videos", photos-music-videos, extension, firefox - "Privacy & Security", privacy-security, extension, firefox - "Search Tools", search-tools, extension, firefox - "Shopping", shopping, extension, firefox - "Social & Communication", social-communication, extension, firefox - "Tabs", tabs, extension, firefox - "Web Development", web-development, extension, firefox - "Other", other, extension, firefox - "Animals", animals, theme, firefox - "Compact", compact, theme, firefox - "Large", large, theme, firefox - "Miscellaneous", miscellaneous, theme, firefox - "Modern", modern, theme, firefox - "Nature", nature, theme, firefox - "OS Integration", os-integration, theme, firefox - "Retro", retro, theme, firefox - "Sports", sports, theme, firefox - "General", general, dictionary, firefox - "Bookmarks", bookmarks, search, firefox - "Business", business, search, firefox - "Dictionaries & Encyclopedias", dictionaries-encyclopedias, search, firefox - "General", general, search, firefox - "Kids", kids, search, firefox - "Multiple Search", multiple-search, search, firefox - "Music", music, search, firefox - "News & Blogs", news-blogs, search, firefox - "Photos & Images", photos-images, search, firefox - "Shopping & E-Commerce", shopping-e-commerce, search, firefox - "Social & People", social-people, search, firefox - "Sports", sports, search, firefox - "Travel", travel, search, firefox - "Video", video, search, firefox - "General", general, language, firefox - "Abstract", abstract, persona, firefox - "Causes", causes, persona, firefox - "Fashion", fashion, persona, firefox - "Film and TV", film-and-tv, persona, firefox - "Firefox", firefox, persona, firefox - "Foxkeh", foxkeh, persona, firefox - "Holiday", holiday, persona, firefox - "Music", music, persona, firefox - "Nature", nature, persona, firefox - "Other", other, persona, firefox - "Scenery", scenery, persona, firefox - "Seasonal", seasonal, persona, firefox - "Solid", solid, persona, firefox - "Sports", sports, persona, firefox - "Websites", websites, persona, firefox - "Appearance and Customization", appearance, extension, thunderbird - "Calendar and Date/Time", calendar, extension, thunderbird - "Chat and IM", chat, extension, thunderbird - "Contacts", contacts, extension, thunderbird - "Folders and Filters", folders-and-filters, extension, thunderbird - "Import/Export", importexport, extension, thunderbird - "Language Support", language-support, extension, thunderbird - "Message Composition", composition, extension, thunderbird - "Message and News Reading", message-and-news-reading, extension, thunderbird - "Miscellaneous", miscellaneous, extension, thunderbird - "Privacy and Security", privacy-and-security, extension, thunderbird - "Tags", tags, extension, thunderbird - "Compact", compact, theme, thunderbird - "Miscellaneous", miscellaneous, theme, thunderbird - "Modern", modern, theme, thunderbird - "Nature", nature, theme, thunderbird - "General", general, dictionary, thunderbird - "General", general, language, thunderbird - "Bookmarks", bookmarks, extension, seamonkey - "Downloading and File Management", downloading-and-file-management, extension, seamonkey - "Interface Customizations", interface-customizations, extension, seamonkey - "Language Support and Translation", language-support-and-translation, extension, seamonkey - "Miscellaneous", miscellaneous, extension, seamonkey - "Photos and Media", photos-and-media, extension, seamonkey - "Privacy and Security", privacy-and-security, extension, seamonkey - "RSS, News and Blogging", rss-news-and-blogging, extension, seamonkey - "Search Tools", search-tools, extension, seamonkey - "Site-specific", site-specific, extension, seamonkey - "Web and Developer Tools", web-and-developer-tools, extension, seamonkey - "Miscellaneous", miscellaneous, theme, seamonkey - "General", general, dictionary, seamonkey - "General", general, language, seamonkey - "Device Features & Location", device-features-location, extension, android - "Experimental", experimental, extension, android - "Feeds, News, & Blogging", feeds-news-blogging, extension, android - "Performance", performance, extension, android - "Photos & Media", photos-media, extension, android - "Security & Privacy", security-privacy, extension, android - "Shopping", shopping, extension, android - "Social Networking", social-networking, extension, android - "Sports & Games", sports-games, extension, android - "User Interface", user-interface, extension, android + "Alerts & Updates", alerts-updates, extension + "Appearance", appearance, extension + "Bookmarks", bookmarks, extension + "Download Management", download-management, extension + "Feeds, News & Blogging", feeds-news-blogging, extension + "Games & Entertainment", games-entertainment, extension + "Language Support", language-support, extension + "Photos, Music & Videos", photos-music-videos, extension + "Privacy & Security", privacy-security, extension + "Search Tools", search-tools, extension + "Shopping", shopping, extension + "Social & Communication", social-communication, extension + "Tabs", tabs, extension + "Web Development", web-development, extension + "Other", other, extension + "General", general, dictionary + "General", general, language + "Abstract", abstract, statictheme + "Causes", causes, statictheme + "Fashion", fashion, statictheme + "Film and TV", film-and-tv, statictheme + "Firefox", firefox, statictheme + "Foxkeh", foxkeh, statictheme + "Holiday", holiday, statictheme + "Music", music, statictheme + "Nature", nature, statictheme + "Other", other, statictheme + "Scenery", scenery, statictheme + "Seasonal", seasonal, statictheme + "Solid", solid, statictheme + "Sports", sports, statictheme + "Websites", websites, statictheme diff --git a/docs/topics/api/overview.rst b/docs/topics/api/overview.rst index 8be334ab4e..729d00d18d 100644 --- a/docs/topics/api/overview.rst +++ b/docs/topics/api/overview.rst @@ -466,6 +466,7 @@ These are `v5` specific changes - `v4` changes apply also. * 2023-07-06: added ``is_all_versions`` to blocklist block endpoint. https://github.com/mozilla/addons-server/issues/20857 * 2023-10-12: added ``reporter_name`` and ``reporter_email`` as two optional alternatives to an authenticated reporter in the abuse api. https://github.com/mozilla/addons-server/issues/21268 * 2023-10-26: added ``location`` to abuse api. https://github.com/mozilla/addons-server/issues/21330 +* 2023-11-02: removed ``application`` from categories endpoint, flattened ``categories`` in addon detail/search endpoint. https://github.com/mozilla/addons-server/issues/5989 .. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/ .. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/ diff --git a/src/olympia/addons/fields.py b/src/olympia/addons/fields.py index 6fac25332d..3c36f7d657 100644 --- a/src/olympia/addons/fields.py +++ b/src/olympia/addons/fields.py @@ -15,15 +15,15 @@ from rest_framework import exceptions, serializers from olympia import amo from olympia.amo.templatetags.jinja_helpers import absolutify -from olympia.amo.utils import ImageCheck, sorted_groupby +from olympia.amo.utils import ImageCheck from olympia.api.fields import ( ESTranslationSerializerField, GetTextTranslationSerializerField, OutgoingURLField, TranslationSerializerField, ) +from olympia.api.utils import is_gate_active from olympia.applications.models import AppVersion -from olympia.constants.applications import APPS from olympia.constants.categories import CATEGORIES from olympia.constants.licenses import LICENSES_BY_SLUG from olympia.files.utils import SafeTar, SafeZip @@ -35,51 +35,57 @@ from olympia.versions.models import ( class CategoriesSerializerField(serializers.Field): + @property + def flat(self): + request = self.context.get('request', None) + return not is_gate_active(request, 'categories-application') + def to_internal_value(self, data): - try: - categories = [] - for app_name, category_names in data.items(): - if len(category_names) > amo.MAX_CATEGORIES: - raise exceptions.ValidationError( - gettext( - 'Maximum number of categories per application ' - '({MAX_CATEGORIES}) exceeded' - ).format(MAX_CATEGORIES=amo.MAX_CATEGORIES) - ) - if len(category_names) > 1 and 'other' in category_names: - raise exceptions.ValidationError( - gettext( - 'The "other" category cannot be combined with another ' - 'category' - ) - ) - app_cats = CATEGORIES[APPS[app_name].id] - # We don't know the addon_type at this point, so try them all and we'll - # drop anything that's wrong later in AddonSerializer.validate - all_cat_slugs = set() - for type_cats in app_cats.values(): - categories.extend( - type_cats[name] for name in category_names if name in type_cats - ) - all_cat_slugs.update(type_cats.keys()) - # Now double-check all the category names were found - if not all_cat_slugs.issuperset(category_names): - raise exceptions.ValidationError(gettext('Invalid category name.')) - if not categories and self.required: - self.fail('required') - return categories - except KeyError: - raise exceptions.ValidationError(gettext('Invalid app name.')) + # Basic backwards-compatibility: accept categories as a dict, but only + # look at the Firefox ones since we removed the Android ones. + if isinstance(data, dict): + category_names = data.get('firefox', []) + else: + category_names = data + if not isinstance(category_names, list): + raise exceptions.ValidationError(gettext('Invalid value')) + + if len(category_names) > amo.MAX_CATEGORIES: + raise exceptions.ValidationError( + gettext( + 'Maximum number of categories per application ' + '({MAX_CATEGORIES}) exceeded' + ).format(MAX_CATEGORIES=amo.MAX_CATEGORIES) + ) + if len(category_names) > 1 and 'other' in category_names: + raise exceptions.ValidationError( + gettext( + 'The "other" category cannot be combined with another ' 'category' + ) + ) + + categories = [] + # We don't know the addon_type at this point, so try them all and we'll + # drop anything that's wrong later in AddonSerializer.validate + all_cat_slugs = set() + for type_cats in CATEGORIES.values(): + categories.extend( + type_cats[name] for name in category_names if name in type_cats + ) + all_cat_slugs.update(type_cats.keys()) + # Now double-check all the category names were found + if not all_cat_slugs.issuperset(category_names): + raise exceptions.ValidationError(gettext('Invalid category name.')) + + if not categories and self.required: + self.fail('required') + return categories def to_representation(self, value): - grouped = sorted_groupby( - sorted(value), - key=lambda x: getattr(amo.APP_IDS.get(x.application), 'short', ''), - ) - return { - app_name: [cat.slug for cat in categories] - for app_name, categories in grouped - } + if self.flat: + return [cat.slug for cat in value] + else: + return {amo.FIREFOX.short: [cat.slug for cat in value]} class ContributionSerializerField(OutgoingURLField): diff --git a/src/olympia/addons/management/commands/update_and_clean_categories.py b/src/olympia/addons/management/commands/update_and_clean_categories.py new file mode 100644 index 0000000000..8ade51aa39 --- /dev/null +++ b/src/olympia/addons/management/commands/update_and_clean_categories.py @@ -0,0 +1,80 @@ +import time + +from django.core.management.base import BaseCommand + +import olympia.core.logger +from olympia.addons.models import AddonCategory +from olympia.constants.categories import CATEGORIES_BY_ID + + +# Mapping between old Android categories and new global ones. +MAPPING_BY_ID = { + 145: 73, # 'device-features-location' —> 'other' + 151: 73, # 'experimental' —> 'other' + 147: 1, # 'feeds-news-blogging' —> 'feeds-news-blogging' + 144: 73, # 'performance' —> 'other' + 143: 38, # 'photos-media' —> 'photos-music-videos' + 149: 12, # 'security-privacy' —> 'privacy-security' + 150: 141, # 'shopping' —> 'shopping' + 148: 71, # 'social-networking' —> 'social-communication' + 146: 142, # 'sports-games' —> 'games-entertainment' + 152: 14, # 'user-interface' —> 'appearance' + 153: 73, # 'other' —> 'other +} + +log = olympia.core.logger.getLogger('z.addons.update_and_clean_categories') + + +class Command(BaseCommand): + BATCH_SIZE = 1000 + + def add_new_categories_for_old_android_categories(self): + for old_category_id, new_category_id in MAPPING_BY_ID.items(): + addon_ids = AddonCategory.objects.filter( + category_id=old_category_id + ).values_list('addon_id', flat=True) + addon_categories = [ + AddonCategory(category_id=new_category_id, addon_id=addon_id) + for addon_id in addon_ids + ] + # We can't run a single UPDATE query, because we might run into + # constraint violations, as we could potentially force an add-on to + # have the same category twice. To work around that we create new + # categories instead in bulk, ignoring conflicts. + # The old extra categories will be deleted later. + objs = AddonCategory.objects.bulk_create( + addon_categories, + batch_size=self.BATCH_SIZE, + ignore_conflicts=True, + ) + log.info('Created (or ignored) %d AddonCategory rows', len(objs)) + log.info('Done updating old android categories') + + def delete_old_categories(self): + qs = AddonCategory.objects.exclude( + category_id__in=list(CATEGORIES_BY_ID.keys()) + ) + threshold = qs.order_by('-pk').values_list('pk', flat=True).first() + ceiling = qs.order_by('pk').values_list('pk', flat=True).first() + count = 0 + while threshold and ceiling and threshold >= ceiling: + print(f'In loop {threshold}') + try: + # Delete by batch. Django doesn't support deleting with a limit + # and offset, but that's inefficient anyway, so we do it by pk, + # deleting _at most_ BATCH_SIZE per iteration. + threshold -= self.BATCH_SIZE + loop_count = qs.filter( + pk__gte=threshold, pk__lte=threshold + self.BATCH_SIZE + ).delete()[0] + except IndexError: + break + log.info('Deleted %d AddonCategory rows', loop_count) + if loop_count: + time.sleep(1) + count += loop_count + log.info('Done deleting %d obsolete categories', count) + + def handle(self): + self.add_new_categories_for_old_android_categories() + self.delete_old_categories() diff --git a/src/olympia/addons/models.py b/src/olympia/addons/models.py index 887d3c8d24..4f7c3eaa18 100644 --- a/src/olympia/addons/models.py +++ b/src/olympia/addons/models.py @@ -1640,17 +1640,6 @@ class Addon(OnChangeMixin, ModelBase): def has_per_version_previews(self): return self.type == amo.ADDON_STATICTHEME - @property - def app_categories(self): - app_cats = {} - categories = sorted_groupby( - sorted(self.all_categories), - key=lambda x: getattr(amo.APP_IDS.get(x.application), 'short', ''), - ) - for app, cats in categories: - app_cats[app] = list(cats) - return app_cats - def remove_locale(self, locale): """NULLify strings in this locale for the add-on and versions.""" for o in itertools.chain([self], self.versions.all()): diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 11be7ac682..365e8bd863 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -1613,8 +1613,16 @@ class StaticCategorySerializer(serializers.Serializer): weight = serializers.IntegerField() description = serializers.CharField() + def to_representation(self, obj): + data = super().to_representation(obj) + request = self.context['request'] + if request and not is_gate_active(request, 'categories-application'): + data.pop('application', None) + return data + def get_application(self, obj): - return APPS_ALL[obj.application].short + # Fake application for API < v5 + return amo.FIREFOX.short def get_type(self, obj): return ADDON_TYPE_CHOICES_API[obj.type] diff --git a/src/olympia/addons/tests/test_commands.py b/src/olympia/addons/tests/test_commands.py index 58a3dea426..b600f93f80 100644 --- a/src/olympia/addons/tests/test_commands.py +++ b/src/olympia/addons/tests/test_commands.py @@ -15,8 +15,9 @@ from olympia.abuse.models import AbuseReport from olympia.addons.management.commands import ( fix_langpacks_with_max_version_star, process_addons, + update_and_clean_categories, ) -from olympia.addons.models import Addon, DeniedGuid +from olympia.addons.models import Addon, AddonCategory, DeniedGuid from olympia.amo.tests import ( TestCase, addon_factory, @@ -24,6 +25,7 @@ from olympia.amo.tests import ( version_factory, ) from olympia.applications.models import AppVersion +from olympia.constants.categories import CATEGORIES from olympia.files.models import FileValidation from olympia.ratings.models import Rating, RatingAggregate from olympia.reviewers.models import AutoApprovalSummary @@ -576,3 +578,105 @@ def test_delete_list_theme_previews(): assert VersionPreview.objects.filter(id=other_firefox_preview.id).exists() assert VersionPreview.objects.filter(id=other_amo_preview.id).exists() assert not VersionPreview.objects.filter(id=other_old_list_preview.id).exists() + + +class TestUpdateAndCleanCategoriesCommand(TestCase): + def test_add_new_categories_for_old_android_categories(self): + regular_addon = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] + ) + addon_with_obsolete_category_and_other = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['other'] + ) + addon_with_obsolete_category_and_regular_one = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['shopping'] + ) + AddonCategory.objects.create( + # Obsolete category that would map to `other` which the add-on + # already has. + addon=addon_with_obsolete_category_and_other, + category_id=153, + ) + AddonCategory.objects.create( + # Obsolete extra category that maps to `feeds-news-blogging`. + addon=addon_with_obsolete_category_and_regular_one, + category_id=147, + ) + assert AddonCategory.objects.count() == 5 + + command = update_and_clean_categories.Command() + command.add_new_categories_for_old_android_categories() + + assert AddonCategory.objects.count() == 6 + # Existing categories unchanged (even the obsolete ones) + assert AddonCategory.objects.filter( + addon=regular_addon, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'].id, + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_other, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['other'].id, + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_regular_one, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['shopping'].id, + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_other, category_id=153 + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_regular_one, category_id=147 + ).exists() + # New category added to replace 147. No category was added for 153 + # since it maps to `other`, which would be a duplicate. + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_regular_one, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['feeds-news-blogging'].id, + ).exists() + + def test_delete_old_categories(self): + regular_addon = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] + ) + addon_with_obsolete_category_and_other = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['other'] + ) + addon_with_obsolete_category_and_regular_one = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['shopping'] + ) + addon_with_bad_category = addon_factory() + AddonCategory.objects.create( + # Obsolete category that would map to `other` which the add-on + # already has. + addon=addon_with_obsolete_category_and_other, + category_id=153, + ) + AddonCategory.objects.create( + # Obsolete extra category that maps to `feeds-news-blogging`. + addon=addon_with_obsolete_category_and_regular_one, + category_id=147, + ) + AddonCategory.objects.filter(addon=addon_with_bad_category).update( + # Obsolete extra category that doesn't map to anything. + category_id=666, + ) + assert AddonCategory.objects.count() == 6 + + command = update_and_clean_categories.Command() + command.BATCH_SIZE = 1 + command.delete_old_categories() + + assert AddonCategory.objects.count() == 3 + # Non-obsolete categories were kept. + assert AddonCategory.objects.filter( + addon=regular_addon, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'].id, + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_other, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['other'].id, + ).exists() + assert AddonCategory.objects.filter( + addon=addon_with_obsolete_category_and_regular_one, + category_id=CATEGORIES[amo.ADDON_EXTENSION]['shopping'].id, + ).exists() diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index 716a605793..ba8f01b68e 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -1166,77 +1166,6 @@ class TestAddonModels(TestCase): assert self.newlines_helper(before) == after - def test_app_categories(self): - def get_addon(): - return Addon.objects.get(pk=3615) - - # This add-on is already associated with three Firefox categories - # using fixtures: Bookmarks, Feeds, Social. - FIREFOX_EXT_CATS = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION] - expected_firefox_cats = [ - FIREFOX_EXT_CATS['bookmarks'], - FIREFOX_EXT_CATS['feeds-news-blogging'], - FIREFOX_EXT_CATS['social-communication'], - ] - - addon = get_addon() - assert sorted(addon.all_categories) == expected_firefox_cats - assert addon.app_categories == {'firefox': expected_firefox_cats} - - # Let's add a ANDROID category. - android_category = CATEGORIES[amo.ANDROID.id][amo.ADDON_EXTENSION][ - 'sports-games' - ] - AddonCategory.objects.create(addon=addon, category_id=android_category.id) - - # Reload the addon to get a fresh, uncached categories list. - addon = get_addon() - - # Test that the ANDROID category was added correctly. - assert sorted(addon.all_categories) == sorted( - expected_firefox_cats + [android_category] - ) - assert sorted(addon.app_categories.keys()) == ['android', 'firefox'] - assert addon.app_categories['firefox'] == expected_firefox_cats - assert addon.app_categories['android'] == [android_category] - - def test_app_categories_ignore_unknown_cats(self): - def get_addon(): - return Addon.objects.get(pk=3615) - - # This add-on is already associated with three Firefox categories - # using fixtures: Bookmarks, Feeds, Social. - FIREFOX_EXT_CATS = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION] - expected_firefox_cats = [ - FIREFOX_EXT_CATS['bookmarks'], - FIREFOX_EXT_CATS['feeds-news-blogging'], - FIREFOX_EXT_CATS['social-communication'], - ] - - addon = get_addon() - assert sorted(addon.all_categories) == sorted(expected_firefox_cats) - assert addon.app_categories == {'firefox': expected_firefox_cats} - - # Associate this add-on with a couple more categories, including - # one that does not exist in the constants. - AddonCategory.objects.create(addon=addon, category_id=12345) - android_static_cat = CATEGORIES[amo.ANDROID.id][amo.ADDON_EXTENSION][ - 'sports-games' - ] - AddonCategory.objects.create(addon=addon, category=android_static_cat) - - # Reload the addon to get a fresh, uncached categories list. - addon = get_addon() - - # The sunbird category should not be present since it does not match - # an existing static category, android one should have been added. - assert sorted(addon.all_categories) == sorted( - expected_firefox_cats + [android_static_cat] - ) - assert sorted(addon.app_categories.keys()) == ['android', 'firefox'] - assert addon.app_categories['firefox'] == expected_firefox_cats - assert addon.app_categories['android'] == [android_static_cat] - def test_review_replies(self): """ Make sure that developer replies are not returned as if they were @@ -1408,11 +1337,7 @@ class TestAddonModels(TestCase): def test_category_transform(self): addon = Addon.objects.get(id=3615) - cats = CATEGORIES[amo.FIREFOX.id][addon.type].values() - names = [c.name for c in cats] - - appname = getattr(amo.APP_IDS.get(amo.FIREFOX.id), 'short', '') - assert addon.app_categories.get(appname)[0].name in names + assert addon.all_categories[0] in CATEGORIES[addon.type].values() def test_listed_has_complete_metadata_no_categories(self): addon = Addon.objects.get(id=3615) diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index caf74a3f41..5b98e78a6c 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -145,7 +145,7 @@ class AddonSerializerOutputTestMixin: assert data['version'] == version.version def test_basic(self): - cat1 = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['bookmarks'] + cat1 = CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] license = License.objects.create( name={ 'en-US': 'My License', @@ -214,20 +214,15 @@ class AddonSerializerOutputTestMixin: min=av_min, max=av_max, ) - cat2 = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['alerts-updates'] + cat2 = CATEGORIES[amo.ADDON_EXTENSION]['alerts-updates'] AddonCategory.objects.create(addon=self.addon, category=cat2) - cat3 = CATEGORIES[amo.ANDROID.id][amo.ADDON_EXTENSION]['sports-games'] - AddonCategory.objects.create(addon=self.addon, category=cat3) result = self.serialize() assert result['id'] == self.addon.pk assert result['average_daily_users'] == self.addon.average_daily_users - assert result['categories'] == { - 'firefox': ['alerts-updates', 'bookmarks'], - 'android': ['sports-games'], - } + assert result['categories'] == ['bookmarks', 'alerts-updates'] # In this serializer latest_unlisted_version is omitted. assert 'latest_unlisted_version' not in result @@ -793,6 +788,18 @@ class AddonSerializerOutputTestMixin: 'url': 'http://www.gnu.org/licenses/gpl-3.0.html', } + def test_categories_as_object(self): + self.addon = addon_factory( + category=CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] + ) + result = self.serialize() + assert result['categories'] == ['bookmarks'] + + gates = {self.request.version: ('categories-application',)} + with override_settings(DRF_API_GATES=gates): + result = self.serialize() + assert result['categories'] == {'firefox': ['bookmarks']} + class TestAddonSerializerOutput(AddonSerializerOutputTestMixin, TestCase): serializer_class = AddonSerializer @@ -1125,7 +1132,7 @@ class TestESAddonSerializerOutput(AddonSerializerOutputTestMixin, ESTestCase): addons[0].all_categories = [category, old_category] category_name = 'music' - category = CATEGORIES[amo.FIREFOX.id][amo.ADDON_STATICTHEME][category_name] + category = CATEGORIES[amo.ADDON_STATICTHEME][category_name] old_category = copy.copy(category) object.__setattr__(old_category, 'id', 666666) object.__setattr__(old_category, 'application', amo.ANDROID.id) @@ -1133,7 +1140,7 @@ class TestESAddonSerializerOutput(AddonSerializerOutputTestMixin, ESTestCase): self.addon = addon_factory(type=amo.ADDON_STATICTHEME, category=category) result = self.serialize() - assert result['categories'] == {amo.FIREFOX.short: [category_name]} + assert result['categories'] == [category_name] class TestVersionSerializerOutput(TestCase): diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 2b6f1f3d34..d0ef43f3e9 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -959,7 +959,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas self.upload.update(channel=amo.CHANNEL_LISTED) response = self.request( data={ - 'categories': {'firefox': ['bookmarks']}, + 'categories': ['bookmarks'], 'version': { 'upload': self.upload.uuid, 'license': self.license.slug, @@ -1044,7 +1044,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas data={ 'summary': {'en-US': None}, 'name': {'en-US': None}, - 'categories': {'firefox': ['bookmarks']}, + 'categories': ['bookmarks'], 'version': { 'upload': self.upload.uuid, 'license': self.license.slug, @@ -1091,7 +1091,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas def test_missing_version(self): self.minimal_data = {} - response = self.request(data={'categories': {'firefox': ['bookmarks']}}) + response = self.request(data={'categories': ['bookmarks']}) assert response.status_code == 400, response.content assert response.data == {'version': ['This field is required.']} assert not Addon.objects.all() @@ -1099,23 +1099,21 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas def test_invalid_categories(self): response = self.request( # performance is an android category - data={'categories': {'firefox': ['performance']}}, + data={'categories': ['performance']}, ) assert response.status_code == 400, response.content assert response.data == {'categories': ['Invalid category name.']} response = self.request( # general is an firefox category but for dicts and lang packs - data={'categories': {'firefox': ['general']}} + data={'categories': ['general']} ) assert response.status_code == 400, response.content assert response.data == {'categories': ['Invalid category name.']} assert not Addon.objects.all() def test_other_category_cannot_be_combined(self): - response = self.request( - data={'categories': {'firefox': ['bookmarks', 'other']}} - ) + response = self.request(data={'categories': ['bookmarks', 'other']}) assert response.status_code == 400, response.content assert response.data == { 'categories': [ @@ -1124,33 +1122,15 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas } assert not Addon.objects.all() - # but it's only enforced per app though. - response = self.request( - data={'categories': {'firefox': ['bookmarks'], 'android': ['other']}} - ) - assert response.status_code == 201 - def test_too_many_categories(self): response = self.request( - data={ - 'categories': {'android': ['performance', 'shopping', 'experimental']} - }, + data={'categories': ['appearance', 'download-management', 'shopping']}, ) assert response.status_code == 400, response.content assert response.data == { 'categories': ['Maximum number of categories per application (2) exceeded'] } - - # check the limit is only applied per app - more than 2 in total is okay. - response = self.request( - data={ - 'categories': { - 'android': ['performance', 'experimental'], - 'firefox': ['bookmarks'], - }, - }, - ) - assert response.status_code == 201, response.content + assert not Addon.objects.all() def test_set_slug(self): # Check for slugs with invalid characters in it @@ -1192,7 +1172,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas def test_set_extra_data(self): self.upload.update(channel=amo.CHANNEL_LISTED) data = { - 'categories': {'firefox': ['bookmarks']}, + 'categories': ['bookmarks'], 'description': {'en-US': 'new description'}, 'developer_comments': {'en-US': 'comments'}, 'homepage': {'en-US': 'https://my.home.page/'}, @@ -1214,10 +1194,9 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas assert response.status_code == 201, response.content addon = Addon.objects.get() data = response.data - assert data['categories'] == {'firefox': ['bookmarks']} - assert addon.all_categories == [ - CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['bookmarks'] - ] + assert data['categories'] == ['bookmarks'] # v5 representation + assert addon.all_categories == [CATEGORIES[amo.ADDON_EXTENSION]['bookmarks']] + response = {'lol': 'blah'} assert data['description'] == {'en-US': 'new description'} assert addon.description == 'new description' assert data['developer_comments'] == {'en-US': 'comments'} @@ -1546,7 +1525,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas response = self.request( data={ 'version': {'upload': upload.uuid, 'license': self.license.slug}, - 'categories': {'firefox': ['other']}, + 'categories': ['other'], 'support_email': { # this field has the required locales 'it': 'rusiczki.ioana@gmail.com', 'ro': 'rusiczki.ioana@gmail.com', @@ -1573,7 +1552,7 @@ class TestAddonViewSetCreate(UploadMixin, AddonViewSetCreateUpdateMixin, TestCas response = self.request( data={ 'version': {'upload': upload.uuid, 'license': self.license.slug}, - 'categories': {'firefox': ['other']}, + 'categories': ['other'], 'support_email': { 'it': 'rusiczki.ioana@gmail.com', 'ro': 'rusiczki.ioana@gmail.com', @@ -2038,47 +2017,47 @@ class TestAddonViewSetUpdate(AddonViewSetCreateUpdateMixin, TestCase): assert self.addon.current_version.reload().release_notes != 'new notes' def test_update_categories(self): - bookmarks_cat = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['bookmarks'] - tabs_cat = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['tabs'] - other_cat = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['other'] + bookmarks_cat = CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] + tabs_cat = CATEGORIES[amo.ADDON_EXTENSION]['tabs'] + other_cat = CATEGORIES[amo.ADDON_EXTENSION]['other'] AddonCategory.objects.filter(addon=self.addon).update(category_id=tabs_cat.id) - assert self.addon.app_categories == {'firefox': [tabs_cat]} + assert self.addon.all_categories == [tabs_cat] - response = self.request(data={'categories': {'firefox': ['bookmarks']}}) + response = self.request(data={'categories': ['bookmarks']}) assert response.status_code == 200, response.content - assert response.data['categories'] == {'firefox': ['bookmarks']} + assert response.data['categories'] == ['bookmarks'] self.addon = Addon.objects.get() - assert self.addon.reload().app_categories == {'firefox': [bookmarks_cat]} + assert self.addon.reload().all_categories == [bookmarks_cat] self.addon.versions.first().update(version='0.123.1') # repeat, but with the `other` category - response = self.request(data={'categories': {'firefox': ['other']}}) + response = self.request(data={'categories': ['other']}) assert response.status_code == 200, response.content - assert response.data['categories'] == {'firefox': ['other']} + assert response.data['categories'] == ['other'] self.addon = Addon.objects.get() - assert self.addon.reload().app_categories == {'firefox': [other_cat]} + assert self.addon.reload().all_categories == [other_cat] def test_invalid_categories(self): - tabs_cat = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['tabs'] + tabs_cat = CATEGORIES[amo.ADDON_EXTENSION]['tabs'] AddonCategory.objects.filter(addon=self.addon).update(category_id=tabs_cat.id) - assert self.addon.app_categories == {'firefox': [tabs_cat]} + assert self.addon.all_categories == [tabs_cat] del self.addon.all_categories response = self.request( # performance is an android category - data={'categories': {'firefox': ['performance']}} + data={'categories': ['performance']} ) assert response.status_code == 400, response.content assert response.data == {'categories': ['Invalid category name.']} - assert self.addon.reload().app_categories == {'firefox': [tabs_cat]} + assert self.addon.reload().all_categories == [tabs_cat] response = self.request( # general is a firefox category, but for langpacks and dicts only - data={'categories': {'firefox': ['general']}}, + data={'categories': ['general']}, ) assert response.status_code == 400, response.content assert response.data == {'categories': ['Invalid category name.']} - assert self.addon.reload().app_categories == {'firefox': [tabs_cat]} + assert self.addon.reload().all_categories == [tabs_cat] def test_set_slug_invalid(self): response = self.request( @@ -5517,13 +5496,13 @@ class TestAddonSearchView(ESTestCase): assert data['results'][0]['id'] == addon.pk def test_filter_by_category(self): - category = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['alerts-updates'] + category = CATEGORIES[amo.ADDON_EXTENSION]['alerts-updates'] addon = addon_factory(slug='my-addon', name='My Addôn', category=category) self.refresh() # Create an add-on in a different category. - other_category = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['tabs'] + other_category = CATEGORIES[amo.ADDON_EXTENSION]['tabs'] addon_factory(slug='different-addon', category=other_category) self.refresh() @@ -5538,7 +5517,7 @@ class TestAddonSearchView(ESTestCase): def test_filter_by_category_multiple_types(self): def get_category(type_, name): - return CATEGORIES[amo.FIREFOX.id][type_][name] + return CATEGORIES[type_][name] addon_ext = addon_factory( slug='my-addon-ext', @@ -6268,7 +6247,7 @@ class TestStaticCategoryView(TestCase): assert response.status_code == 200 data = json.loads(force_str(response.content)) - assert len(data) == 43 + assert len(data) == 32 # some basic checks to verify integrity entry = data[0] @@ -6278,7 +6257,6 @@ class TestStaticCategoryView(TestCase): 'weight': 0, 'misc': False, 'id': 1, - 'application': 'firefox', 'description': ( 'Download Firefox extensions that remove clutter so you ' 'can stay up-to-date on social media, catch up on blogs, ' @@ -6297,7 +6275,7 @@ class TestStaticCategoryView(TestCase): assert response.status_code == 200 data = json.loads(force_str(response.content)) - assert len(data) == 43 + assert len(data) == 32 # some basic checks to verify integrity entry = data[0] @@ -6307,7 +6285,6 @@ class TestStaticCategoryView(TestCase): 'weight': 0, 'misc': False, 'id': 1, - 'application': 'firefox', 'description': 'does stuff', 'type': 'extension', 'slug': 'feeds-news-blogging', @@ -6328,6 +6305,16 @@ class TestStaticCategoryView(TestCase): assert response.status_code == 200 assert response['cache-control'] == 'max-age=21600' + @override_settings(DRF_API_GATES={'v5': ('categories-application',)}) + def test_with_application(self): + with self.assertNumQueries(0): + response = self.client.get(self.url) + assert response.status_code == 200 + data = json.loads(response.content) + assert len(data) == 32 + for entry in data: + assert entry['application'] == 'firefox' + class TestLanguageToolsView(TestCase): client_class = APITestClientSessionID diff --git a/src/olympia/amo/sitemap.py b/src/olympia/amo/sitemap.py index 86e4829fb0..1682a78a2b 100644 --- a/src/olympia/amo/sitemap.py +++ b/src/olympia/amo/sitemap.py @@ -229,9 +229,9 @@ class CategoriesSitemap(Sitemap): page_size = settings.REST_FRAMEWORK['PAGE_SIZE'] page_count_max = settings.ES_MAX_RESULT_WINDOW // page_size - def additems(type): + def additems(type_): items = [] - for category in CATEGORIES[current_app.id][type].values(): + for category in CATEGORIES[type_].values(): items.append((category, 1)) pages_needed = min( math.ceil(addon_counts.get(category.id, 1) / page_size), diff --git a/src/olympia/amo/tests/__init__.py b/src/olympia/amo/tests/__init__.py index 05aec8b87d..f91b2d4c63 100644 --- a/src/olympia/amo/tests/__init__.py +++ b/src/olympia/amo/tests/__init__.py @@ -758,9 +758,9 @@ def addon_factory(status=amo.STATUS_APPROVED, version_kw=None, file_kw=None, **k for user in users: addon.addonuser_set.create(user=user) - application = version_kw.get('application', amo.FIREFOX.id) - if not category and addon.type in CATEGORIES[application]: - category = random.choice(list(CATEGORIES[application][addon.type].values())) + version_kw.get('application', amo.FIREFOX.id) + if not category and addon.type in CATEGORIES: + category = random.choice(list(CATEGORIES[addon.type].values())) if category: AddonCategory.objects.create(addon=addon, category=category) diff --git a/src/olympia/amo/tests/test_sitemap.py b/src/olympia/amo/tests/test_sitemap.py index a84d30d8a4..e5f7790284 100644 --- a/src/olympia/amo/tests/test_sitemap.py +++ b/src/olympia/amo/tests/test_sitemap.py @@ -179,18 +179,12 @@ def test_categories_sitemap(): # without any addons we should still generate a url for each category empty_cats = list(CategoriesSitemap().items()) assert empty_cats == [ - *( - (category, 1) - for category in CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION].values() - ), - *( - (category, 1) - for category in CATEGORIES[amo.FIREFOX.id][amo.ADDON_STATICTHEME].values() - ), + *((category, 1) for category in CATEGORIES[amo.ADDON_EXTENSION].values()), + *((category, 1) for category in CATEGORIES[amo.ADDON_STATICTHEME].values()), ] # add some addons and check we generate extra pages when frontend would paginate - bookmarks_category = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['bookmarks'] - shopping_category = CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION]['shopping'] + bookmarks_category = CATEGORIES[amo.ADDON_EXTENSION]['bookmarks'] + shopping_category = CATEGORIES[amo.ADDON_EXTENSION]['shopping'] AddonCategory.objects.create( addon=addon_factory(category=bookmarks_category), category=shopping_category ) diff --git a/src/olympia/constants/categories.py b/src/olympia/constants/categories.py index ec3690df1d..9e739aa7bc 100644 --- a/src/olympia/constants/categories.py +++ b/src/olympia/constants/categories.py @@ -1,15 +1,10 @@ -import copy from functools import total_ordering from django.urls import reverse from django.utils.encoding import force_bytes from django.utils.translation import gettext_lazy as _ -from olympia.constants.applications import ANDROID, FIREFOX from olympia.constants.base import ( - _ADDON_PERSONA, - _ADDON_SEARCH, - _ADDON_THEME, ADDON_DICT, ADDON_EXTENSION, ADDON_LPAPP, @@ -27,9 +22,10 @@ class StaticCategory: to hard to debug sporadic test-failures. """ - def __init__(self, name=None, description=None, weight=0): + def __init__(self, *, id, name=None, description=None, weight=0): # Avoid triggering our own __setattr__ implementation # to keep immutability intact but set initial values. + object.__setattr__(self, 'id', id) object.__setattr__(self, 'name', name) object.__setattr__(self, 'weight', weight) object.__setattr__(self, 'description', description) @@ -38,10 +34,9 @@ class StaticCategory: return str(self.name) def __repr__(self): - return '<{}: {} ({})>'.format( + return '<{}: {}>'.format( self.__class__.__name__, force_bytes(self), - self.application, ) def __eq__(self, other): @@ -64,9 +59,12 @@ class StaticCategory: return self.id -CATEGORIES_NO_APP = { +# The category ids are used in AddonCategory. To add a category you can pick +# any unused id. +CATEGORIES = { ADDON_EXTENSION: { 'alerts-updates': StaticCategory( + id=72, name=_('Alerts & Updates'), description=_( 'Download Firefox extensions that help you stay ' @@ -76,6 +74,7 @@ CATEGORIES_NO_APP = { ), ), 'appearance': StaticCategory( + id=14, name=_('Appearance'), description=_( 'Download extensions that modify the appearance of ' @@ -85,6 +84,7 @@ CATEGORIES_NO_APP = { ), ), 'bookmarks': StaticCategory( + id=22, name=_('Bookmarks'), description=_( 'Download extensions that enhance bookmarks by ' @@ -93,6 +93,7 @@ CATEGORIES_NO_APP = { ), ), 'download-management': StaticCategory( + id=5, name=_('Download Management'), description=_( 'Download Firefox extensions that can help download web, ' @@ -101,6 +102,7 @@ CATEGORIES_NO_APP = { ), ), 'feeds-news-blogging': StaticCategory( + id=1, name=_('Feeds, News & Blogging'), description=_( 'Download Firefox extensions that remove clutter so you ' @@ -109,6 +111,7 @@ CATEGORIES_NO_APP = { ), ), 'games-entertainment': StaticCategory( + id=142, name=_('Games & Entertainment'), description=_( 'Download Firefox extensions to boost your entertainment ' @@ -117,6 +120,7 @@ CATEGORIES_NO_APP = { ), ), 'language-support': StaticCategory( + id=37, name=_('Language Support'), description=_( 'Download Firefox extensions that offer language support ' @@ -125,6 +129,7 @@ CATEGORIES_NO_APP = { ), ), 'photos-music-videos': StaticCategory( + id=38, name=_('Photos, Music & Videos'), description=_( 'Download Firefox extensions that enhance photo, music ' @@ -133,6 +138,7 @@ CATEGORIES_NO_APP = { ), ), 'privacy-security': StaticCategory( + id=12, name=_('Privacy & Security'), description=_( 'Download Firefox extensions to browse privately and ' @@ -142,6 +148,7 @@ CATEGORIES_NO_APP = { ), ), 'search-tools': StaticCategory( + id=13, name=_('Search Tools'), description=_( 'Download Firefox extensions for search and look-up. ' @@ -150,6 +157,7 @@ CATEGORIES_NO_APP = { ), ), 'shopping': StaticCategory( + id=141, name=_('Shopping'), description=_( 'Download Firefox extensions that can enhance your ' @@ -158,6 +166,7 @@ CATEGORIES_NO_APP = { ), ), 'social-communication': StaticCategory( + id=71, name=_('Social & Communication'), description=_( 'Download Firefox extensions to enhance social media and ' @@ -166,6 +175,7 @@ CATEGORIES_NO_APP = { ), ), 'tabs': StaticCategory( + id=93, name=_('Tabs'), description=_( 'Download Firefox extension to customize tabs and the ' @@ -174,6 +184,7 @@ CATEGORIES_NO_APP = { ), ), 'web-development': StaticCategory( + id=4, name=_('Web Development'), description=_( 'Download Firefox extensions that feature web ' @@ -183,6 +194,7 @@ CATEGORIES_NO_APP = { ), ), 'other': StaticCategory( + id=73, name=_('Other'), weight=333, description=_( @@ -190,85 +202,10 @@ CATEGORIES_NO_APP = { 'and creative, yet useful for those odd tasks.' ), ), - # Android only categories: - 'device-features-location': StaticCategory( - name=_('Device Features & Location'), - description=_( - 'Download extensions to enhance Firefox for Android. ' - 'Perform quick searches, free up system resources, take ' - 'notes, and more.' - ), - ), - 'experimental': StaticCategory( - name=_('Experimental'), - description=_( - 'Download Firefox extensions that are regularly updated ' - 'and ready for public testing. Your feedback informs ' - 'developers on changes to make in upcoming versions.' - ), - ), - 'performance': StaticCategory( - name=_('Performance'), - description=_( - 'Download extensions that give Firefox a performance ' - 'boost. Find extensions that help you be more productive ' - 'and efficient by blocking annoying ads and more.' - ), - ), - 'photos-media': StaticCategory( - name=_('Photos & Media'), - description=_( - 'Download Firefox extensions to enhance photos and ' - 'media. This category includes extensions to reverse ' - 'search images, capture full page screenshots, and more.' - ), - ), - 'security-privacy': StaticCategory( - name=_('Security & Privacy'), - description=_( - 'Download Firefox extensions to surf safely and ' - 'privately. Discover extensions that can stop sneaky ad ' - 'trackers in their tracks, easily clear browsing ' - 'history, and more.' - ), - ), - 'social-networking': StaticCategory( - name=_('Social Networking'), - description=_( - 'Download Firefox extensions to enhance your experience ' - 'on popular social networking websites such as YouTube, ' - 'GitHub, Reddit, and more.' - ), - ), - 'sports-games': StaticCategory( - name=_('Sports & Games'), - description=_( - 'Download Firefox extensions to give your entertainment ' - 'experience a boost with live stream enhancers, sports ' - 'updates, and more.' - ), - ), - 'user-interface': StaticCategory( - name=_('User Interface'), - description=_( - 'Download user interface Firefox extensions to alter web ' - 'pages for easier reading, searching, browsing, and more.' - ), - ), - }, - _ADDON_THEME: { - 'animals': StaticCategory(name=_('Animals')), - 'compact': StaticCategory(name=_('Compact')), - 'large': StaticCategory(name=_('Large')), - 'miscellaneous': StaticCategory(name=_('Miscellaneous')), - 'modern': StaticCategory(name=_('Modern')), - 'nature': StaticCategory(name=_('Nature')), - 'os-integration': StaticCategory(name=_('OS Integration')), - 'retro': StaticCategory(name=_('Retro')), - 'sports': StaticCategory(name=_('Sports')), }, ADDON_STATICTHEME: { 'abstract': StaticCategory( + id=300, name=_('Abstract'), description=_( 'Download Firefox artistic and conceptual themes. This ' @@ -277,6 +214,7 @@ CATEGORIES_NO_APP = { ), ), 'causes': StaticCategory( + id=320, name=_('Causes'), description=_( 'Download Firefox themes for niche interests and topics. ' @@ -285,6 +223,7 @@ CATEGORIES_NO_APP = { ), ), 'fashion': StaticCategory( + id=324, name=_('Fashion'), description=_( 'Download Firefox themes that celebrate style of all ' @@ -292,6 +231,7 @@ CATEGORIES_NO_APP = { ), ), 'film-and-tv': StaticCategory( + id=326, name=_('Film and TV'), description=_( 'Download Firefox themes with movies and television. ' @@ -300,6 +240,7 @@ CATEGORIES_NO_APP = { ), ), 'firefox': StaticCategory( + id=308, name=_('Firefox'), description=_( 'Download Firefox themes with the Firefox browser theme. ' @@ -308,6 +249,7 @@ CATEGORIES_NO_APP = { ), ), 'foxkeh': StaticCategory( + id=310, name=_('Foxkeh'), description=_( 'Download Firefox themes with the Japanese Firefox. This ' @@ -316,6 +258,7 @@ CATEGORIES_NO_APP = { ), ), 'holiday': StaticCategory( + id=328, name=_('Holiday'), description=_( 'Download Firefox themes with holidays. This category ' @@ -324,6 +267,7 @@ CATEGORIES_NO_APP = { ), ), 'music': StaticCategory( + id=322, name=_('Music'), description=_( 'Download Firefox themes for musical interests and ' @@ -333,6 +277,7 @@ CATEGORIES_NO_APP = { ), ), 'nature': StaticCategory( + id=302, name=_('Nature'), description=_( 'Download Firefox themes with animals and natural ' @@ -341,6 +286,7 @@ CATEGORIES_NO_APP = { ), ), 'other': StaticCategory( + id=314, name=_('Other'), weight=333, description=_( @@ -348,6 +294,7 @@ CATEGORIES_NO_APP = { ), ), 'scenery': StaticCategory( + id=306, name=_('Scenery'), description=_( 'Download Firefox themes that feature the environment ' @@ -356,6 +303,7 @@ CATEGORIES_NO_APP = { ), ), 'seasonal': StaticCategory( + id=312, name=_('Seasonal'), description=_( 'Download Firefox themes for all four seasons—fall, ' @@ -364,6 +312,7 @@ CATEGORIES_NO_APP = { ), ), 'solid': StaticCategory( + id=318, name=_('Solid'), description=_( 'Download Firefox themes with solid and gradient colors ' @@ -372,6 +321,7 @@ CATEGORIES_NO_APP = { ), ), 'sports': StaticCategory( + id=304, name=_('Sports'), description=_( 'Download Firefox themes that feature a variety of ' @@ -380,6 +330,7 @@ CATEGORIES_NO_APP = { ), ), 'websites': StaticCategory( + id=316, name=_('Websites'), description=_( 'Download Firefox themes that capture the essence of the ' @@ -387,34 +338,12 @@ CATEGORIES_NO_APP = { ), ), }, - ADDON_DICT: {'general': StaticCategory(name=_('General'))}, - _ADDON_SEARCH: { - 'bookmarks': StaticCategory(name=_('Bookmarks')), - 'business': StaticCategory(name=_('Business')), - 'dictionaries-encyclopedias': StaticCategory( - name=_('Dictionaries & Encyclopedias') - ), - 'general': StaticCategory(name=_('General')), - 'kids': StaticCategory(name=_('Kids')), - 'multiple-search': StaticCategory(name=_('Multiple Search')), - 'music': StaticCategory(name=_('Music')), - 'news-blogs': StaticCategory(name=_('News & Blogs')), - 'photos-images': StaticCategory(name=_('Photos & Images')), - 'shopping-e-commerce': StaticCategory(name=_('Shopping & E-Commerce')), - 'social-people': StaticCategory(name=_('Social & People')), - 'sports': StaticCategory(name=_('Sports')), - 'travel': StaticCategory(name=_('Travel')), - 'video': StaticCategory(name=_('Video')), - }, - ADDON_LPAPP: {'general': StaticCategory(name=_('General'))}, + ADDON_DICT: {'general': StaticCategory(id=95, name=_('General'))}, + ADDON_LPAPP: {'general': StaticCategory(id=98, name=_('General'))}, } -CATEGORIES_NO_APP[_ADDON_PERSONA] = { - slug: copy.copy(cat) for slug, cat in CATEGORIES_NO_APP[ADDON_STATICTHEME].items() -} - -for type_ in CATEGORIES_NO_APP: - for slug, cat in CATEGORIES_NO_APP[type_].items(): +for type_ in CATEGORIES: + for slug, cat in CATEGORIES[type_].items(): # Flatten some values and set them, avoiding immutability # of `StaticCategory` by calling `object.__setattr__` directly. object.__setattr__(cat, 'slug', slug) @@ -422,78 +351,8 @@ for type_ in CATEGORIES_NO_APP: object.__setattr__(cat, 'misc', slug in ('miscellaneous', 'other')) -# These category ids are used in AddonCategory. To add a category to an app you can use -# any unused id. -CATEGORIES = { - FIREFOX.id: { - ADDON_EXTENSION: { - 'alerts-updates': 72, - 'appearance': 14, - 'bookmarks': 22, - 'download-management': 5, - 'feeds-news-blogging': 1, - 'games-entertainment': 142, - 'language-support': 37, - 'photos-music-videos': 38, - 'privacy-security': 12, - 'search-tools': 13, - 'shopping': 141, - 'social-communication': 71, - 'tabs': 93, - 'web-development': 4, - 'other': 73, - }, - ADDON_STATICTHEME: { - 'abstract': 300, - 'causes': 320, - 'fashion': 324, - 'film-and-tv': 326, - 'firefox': 308, - 'foxkeh': 310, - 'holiday': 328, - 'music': 322, - 'nature': 302, - 'other': 314, - 'scenery': 306, - 'seasonal': 312, - 'solid': 318, - 'sports': 304, - 'websites': 316, - }, - ADDON_LPAPP: { - 'general': 98, - }, - ADDON_DICT: { - 'general': 95, - }, - }, - ANDROID.id: { - ADDON_EXTENSION: { - 'device-features-location': 145, - 'experimental': 151, - 'feeds-news-blogging': 147, - 'performance': 144, - 'photos-media': 143, - 'security-privacy': 149, - 'shopping': 150, - 'social-networking': 148, - 'sports-games': 146, - 'user-interface': 152, - 'other': 153, - }, - }, -} - - CATEGORIES_BY_ID = {} -for app in CATEGORIES: - for type_ in CATEGORIES[app]: - for slug, id_ in CATEGORIES[app][type_].items(): - cat = copy.copy(CATEGORIES_NO_APP[type_][slug]) - # Flatten some values and set them, avoiding immutability - # of `StaticCategory` by calling `object.__setattr__` directly. - object.__setattr__(cat, 'id', id_) - object.__setattr__(cat, 'application', app) - CATEGORIES_BY_ID[id_] = cat - CATEGORIES[app][type_][slug] = cat +for type_ in CATEGORIES: + for cat in CATEGORIES[type_].values(): + CATEGORIES_BY_ID[cat.id] = cat diff --git a/src/olympia/devhub/forms.py b/src/olympia/devhub/forms.py index 34bb9f2435..579f14fa60 100644 --- a/src/olympia/devhub/forms.py +++ b/src/olympia/devhub/forms.py @@ -7,7 +7,6 @@ from django import forms from django.conf import settings from django.core.validators import MinLengthValidator from django.db.models import Q -from django.forms.formsets import BaseFormSet, formset_factory from django.forms.models import BaseModelFormSet, modelformset_factory from django.forms.widgets import RadioSelect from django.urls import reverse @@ -45,7 +44,7 @@ from olympia.amo.messages import DoubleSafe from olympia.amo.utils import remove_icons, slug_validator from olympia.amo.validators import OneOrMoreLetterOrNumberCharacterValidator from olympia.applications.models import AppVersion -from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID, CATEGORIES_NO_APP +from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID from olympia.devhub.widgets import CategoriesSelectMultiple, IconTypeSelect from olympia.files.models import FileUpload from olympia.files.utils import SafeTar, SafeZip, parse_addon @@ -139,45 +138,56 @@ class AddonFormBase(TranslationFormMixin, AMOModelForm): class CategoryForm(forms.Form): - application = forms.TypedChoiceField( - choices=amo.APPS_CHOICES, coerce=int, widget=forms.HiddenInput, required=True - ) categories = forms.MultipleChoiceField(choices=(), widget=CategoriesSelectMultiple) - def save(self, addon): - application = self.cleaned_data.get('application') + def __init__(self, *args, **kw): + self.addon = kw.pop('addon') + self.request = kw.pop('request', None) + super().__init__(*args, **kw) + cats = sorted( + CATEGORIES.get(self.addon.type, {}).values(), + key=lambda x: x.name, + ) + if self.addon.type == amo.ADDON_STATICTHEME: + self.max_categories = 1 + self.fields['categories'] = forms.ChoiceField(widget=forms.RadioSelect) + else: + self.max_categories = amo.MAX_CATEGORIES + self.fields['categories'].choices = [(c.id, c.name) for c in cats] + self.fields['categories'].initial = [c.id for c in self.addon.all_categories] + + def save(self): categories_new = [int(c) for c in self.cleaned_data['categories']] - categories_old = [ - c.id for c in addon.app_categories.get(amo.APP_IDS[application].short, []) - ] + categories_old = [c.id for c in self.addon.all_categories] # Add new categories. for c_id in set(categories_new) - set(categories_old): - AddonCategory(addon=addon, category_id=c_id).save() + AddonCategory(addon=self.addon, category_id=c_id).save() # Remove old categories. for c_id in set(categories_old) - set(categories_new): - AddonCategory.objects.filter(addon=addon, category_id=c_id).delete() + AddonCategory.objects.filter(addon=self.addon, category_id=c_id).delete() # Remove old, outdated categories cache on the model. - del addon.all_categories + del self.addon.all_categories # Make sure the add-on is properly re-indexed - addons_tasks.index_addons.delay([addon.id]) + addons_tasks.index_addons.delay([self.addon.id]) def clean_categories(self): categories = self.cleaned_data['categories'] + if isinstance(categories, str): + categories = [categories] total = len(categories) - max_cat = amo.MAX_CATEGORIES - if total > max_cat: + if total > self.max_categories: # L10n: {0} is the number of categories. raise forms.ValidationError( ngettext( 'You can have only {0} category.', 'You can have only {0} categories.', - max_cat, - ).format(max_cat) + self.max_categories, + ).format(self.max_categories) ) has_misc = list(filter(lambda x: CATEGORIES_BY_ID.get(int(x)).misc, categories)) @@ -192,47 +202,6 @@ class CategoryForm(forms.Form): return categories -class BaseCategoryFormSet(BaseFormSet): - def __init__(self, *args, **kw): - self.addon = kw.pop('addon') - self.request = kw.pop('request', None) - super().__init__(*args, **kw) - self.initial = [] - apps = sorted(self.addon.compatible_apps.keys(), key=lambda x: x.id) - - # Drop any apps that don't have appropriate categories. - for app in list(apps): - if app and not CATEGORIES.get(app.id, {}).get(self.addon.type): - apps.remove(app) - - if not CATEGORIES_NO_APP.get(self.addon.type): - apps = [] - - for app in apps: - cats = self.addon.app_categories.get(app.short, []) - self.initial.append({'categories': [c.id for c in cats]}) - - for app, form in zip(apps, self.forms, strict=True): - key = app.id if app else None - form.request = self.request - form.initial['application'] = key - form.app = app - cats = sorted( - CATEGORIES.get(key, {}).get(self.addon.type, {}).values(), - key=lambda x: x.name, - ) - form.fields['categories'].choices = [(c.id, c.name) for c in cats] - - def save(self): - for f in self.forms: - f.save(self.addon) - - -CategoryFormSet = formset_factory( - form=CategoryForm, formset=BaseCategoryFormSet, extra=0 -) - - ICON_TYPES = [('', 'default'), ('image/jpeg', 'jpeg'), ('image/png', 'png')] @@ -1448,31 +1417,3 @@ class AgreementForm(forms.Form): if not checker.is_submission_allowed(check_dev_agreement=False): raise forms.ValidationError(checker.get_error_message()) return self.cleaned_data - - -class SingleCategoryForm(forms.Form): - category = forms.ChoiceField(widget=forms.RadioSelect) - - def __init__(self, *args, **kw): - self.addon = kw.pop('addon') - self.request = kw.pop('request', None) - if len(self.addon.all_categories) > 0: - kw['initial'] = {'category': self.addon.all_categories[0].slug} - super().__init__(*args, **kw) - - sorted_cats = sorted( - CATEGORIES_NO_APP[self.addon.type].items(), key=lambda slug_cat: slug_cat[0] - ) - self.fields['category'].choices = [(slug, c.name) for slug, c in sorted_cats] - - def save(self): - category_slug = self.cleaned_data['category'] - # Clear any old categor[y|ies] - AddonCategory.objects.filter(addon=self.addon).delete() - # Add new categor[y|ies] - for app in CATEGORIES.keys(): - category = CATEGORIES[app].get(self.addon.type, {}).get(category_slug, None) - if category: - AddonCategory(addon=self.addon, category_id=category.id).save() - # Remove old, outdated categories cache on the model. - del self.addon.all_categories diff --git a/src/olympia/devhub/templates/devhub/addons/edit/describe.html b/src/olympia/devhub/templates/devhub/addons/edit/describe.html index cdc6ab369f..284c4e3f38 100644 --- a/src/olympia/devhub/templates/devhub/addons/edit/describe.html +++ b/src/olympia/devhub/templates/devhub/addons/edit/describe.html @@ -190,26 +190,16 @@ "exposure.")) }} - {% if editable and addon.type != amo.ADDON_STATICTHEME %} - {{ cat_form.non_form_errors() }} - {{ cat_form.management_form }} - {% for form in cat_form.initial_forms %} - {{ select_cats(amo.MAX_CATEGORIES, form) }} - {% endfor %} - {% elif editable and addon.type == amo.ADDON_STATICTHEME %} - {{ cat_form.category.errors }} - {{ cat_form.category }} + {% if cat_form %}data-max-categories="{{ cat_form.max_categories }}"{% endif %}> + {% if editable %} + {{ select_cats(cat_form) }} {% else %} - {% set categories = addon.app_categories %} + {% set categories = addon.all_categories %} {% call empty_unless(categories) %}
    - {% for app_short_name, cats in categories.items() %}
  • - {{ amo.APPS[app_short_name].pretty }}: - {{ cats|join(' · ')|safe }} + {{ categories|join(' · ')|safe }}
    • - {% endfor %}
{% endcall %} {% endif %} diff --git a/src/olympia/devhub/templates/devhub/addons/submit/describe.html b/src/olympia/devhub/templates/devhub/addons/submit/describe.html index 60c3ab9866..969280bb0e 100644 --- a/src/olympia/devhub/templates/devhub/addons/submit/describe.html +++ b/src/olympia/devhub/templates/devhub/addons/submit/describe.html @@ -141,21 +141,11 @@ "in order to work.") }}">? -
- {{ cat_form.non_form_errors() }} - {{ cat_form.management_form }} - {% for form in cat_form.initial_forms %} - {{ select_cats(amo.MAX_CATEGORIES, form) }} - {% endfor %} -
- {% else %} -
- - {{ cat_form.category.errors }} - {{ cat_form.category }} -
{% endif %} +
+ {{ select_cats(cat_form) }} +