Remove ScannerResult.state and related code

Author: willdurand

src/olympia/constants/scanners.py

@@ -41,18 +41,6 @@
     DISABLE_AND_BLOCK: ('Force-disable and block'),
 }
 
-UNKNOWN = None
-TRUE_POSITIVE = 1
-FALSE_POSITIVE = 2
-INCONCLUSIVE = 3
-
-RESULT_STATES = {
-    UNKNOWN: 'Unknown',
-    TRUE_POSITIVE: 'True positive',
-    FALSE_POSITIVE: 'False positive',
-    INCONCLUSIVE: 'Inconclusive',
-}
-
 NEW = 1
 RUNNING = 2
 ABORTED = 3
@@ -69,9 +57,6 @@
     SCHEDULED: 'Scheduled',
 }
 
-LABEL_BAD = 'bad'
-LABEL_GOOD = 'good'
-
 # Scanner service accounts group
 SCANNER_SERVICE_ACCOUNTS_GROUP = 'Service accounts for scanners'
 

src/olympia/scanners/admin.py

@@ -1,4 +1,4 @@
-from urllib.parse import urljoin, urlparse
+from urllib.parse import urljoin
 
 from django.conf import settings
 from django.contrib import admin, messages
@@ -23,21 +23,15 @@
     NumericRangeFilter,
 )
 from olympia.amo.templatetags.jinja_helpers import vite_asset
-from olympia.amo.utils import is_safe_url
 from olympia.api.models import APIKey
 from olympia.constants.scanners import (
     ABORTING,
     COMPLETED,
-    FALSE_POSITIVE,
-    INCONCLUSIVE,
     NARC,
     NEW,
-    RESULT_STATES,
     RUNNING,
     SCANNERS,
     SCHEDULED,
-    TRUE_POSITIVE,
-    UNKNOWN,
     WEBHOOK,
     WEBHOOK_EVENTS,
     YARA,
@@ -153,34 +147,6 @@ def queryset(self, request, queryset):
         return queryset.filter(scanner=self.value())
 
 
-class StateFilter(SimpleListFilter):
-    title = 'result state'
-    parameter_name = 'state'
-
-    def lookups(self, request, model_admin):
-        return (('all', 'All'), *RESULT_STATES.items())
-
-    def choices(self, cl):
-        for lookup, title in self.lookup_choices:
-            selected = (
-                lookup == UNKNOWN
-                if self.value() is None
-                else self.value() == str(lookup)
-            )
-            yield {
-                'selected': selected,
-                'query_string': cl.get_query_string({self.parameter_name: lookup}, []),
-                'display': title,
-            }
-
-    def queryset(self, request, queryset):
-        if self.value() == 'all':
-            return queryset
-        if self.value() is None:
-            return queryset.filter(state=UNKNOWN)
-        return queryset.filter(state=self.value())
-
-
 class ScannerRuleListFilter(admin.RelatedOnlyFieldListFilter):
     include_empty_choice = False
 
@@ -351,7 +317,6 @@ def get_unfiltered_changelist_params(self):
         might filter out some results by default."""
         return {
             WithVersionFilter.parameter_name: 'all',
-            StateFilter.parameter_name: 'all',
         }
 
     def has_block_addons_permission(self, request):
@@ -405,27 +370,6 @@ def has_add_permission(self, request):
     def has_change_permission(self, request, obj=None):
         return False
 
-    # Custom actions
-    def has_actions_permission(self, request):
-        return acl.action_allowed_for(
-            request.user, amo.permissions.ADMIN_SCANNERS_RESULTS_EDIT
-        )
-
-    def get_list_display(self, request):
-        fields = super().get_list_display(request)
-        return self._excludes_fields(request=request, fields=fields)
-
-    def get_fields(self, request, obj=None):
-        fields = super().get_fields(request, obj)
-        return self._excludes_fields(request=request, fields=fields)
-
-    def _excludes_fields(self, request, fields):
-        to_exclude = []
-        if not self.has_actions_permission(request):
-            to_exclude = ['result_actions']
-        fields = list(filter(lambda x: x not in to_exclude, fields))
-        return fields
-
     def formatted_addon(self, obj):
         if obj.version:
             return format_html(
@@ -639,9 +583,7 @@ class ScannerResultAdmin(AbstractScannerResultAdminMixin, AMOModelAdmin):
         'guid',
         'formatted_scanner',
         'created',
-        'state',
         formatted_matched_rules_with_files_and_data,
-        'result_actions',
         'formatted_results',
     )
     list_display = (
@@ -652,12 +594,10 @@ class ScannerResultAdmin(AbstractScannerResultAdminMixin, AMOModelAdmin):
         'formatted_scanner',
         'formatted_matched_rules',
         'formatted_created',
-        'result_actions',
     )
     list_filter = (
         ScannerFilter,
         MatchesFilter,
-        StateFilter,
         ('matched_rules', ScannerRuleListFilter),
         WithVersionFilter,
         ExcludeMatchedRulesFilter,
@@ -672,135 +612,6 @@ def formatted_scanner(self, obj):
 
     formatted_scanner.short_description = 'Scanner'
 
-    def safe_referer_redirect(self, request, default_url):
-        referer = request.META.get('HTTP_REFERER')
-        allowed_hosts = (
-            settings.DOMAIN,
-            urlparse(settings.EXTERNAL_SITE_URL).netloc,
-        )
-        if referer and is_safe_url(referer, request, allowed_hosts):
-            return redirect(referer)
-        return redirect(default_url)
-
-    def handle_true_positive(self, request, pk, *args, **kwargs):
-        can_use_actions = self.has_actions_permission(request)
-        if not can_use_actions or request.method != 'POST':
-            raise Http404
-
-        result = self.get_object(request, pk)
-        result.update(state=TRUE_POSITIVE)
-
-        messages.add_message(
-            request,
-            messages.INFO,
-            f'Scanner result {pk} has been marked as true positive.',
-        )
-
-        return self.safe_referer_redirect(
-            request, default_url='admin:scanners_scannerresult_changelist'
-        )
-
-    def handle_inconclusive(self, request, pk, *args, **kwargs):
-        can_use_actions = self.has_actions_permission(request)
-        if not can_use_actions or request.method != 'POST':
-            raise Http404
-
-        result = self.get_object(request, pk)
-        result.update(state=INCONCLUSIVE)
-
-        messages.add_message(
-            request,
-            messages.INFO,
-            f'Scanner result {pk} has been marked as inconclusive.',
-        )
-
-        return self.safe_referer_redirect(
-            request, default_url='admin:scanners_scannerresult_changelist'
-        )
-
-    def handle_false_positive(self, request, pk, *args, **kwargs):
-        can_use_actions = self.has_actions_permission(request)
-        if not can_use_actions or request.method != 'POST':
-            raise Http404
-
-        result = self.get_object(request, pk)
-        result.update(state=FALSE_POSITIVE)
-
-        messages.add_message(
-            request,
-            messages.INFO,
-            f'Scanner result {pk} has been marked as false positive.',
-        )
-
-        return self.safe_referer_redirect(
-            request, default_url='admin:scanners_scannerresult_changelist'
-        )
-
-    def handle_revert(self, request, pk, *args, **kwargs):
-        is_admin = acl.action_allowed_for(
-            request.user, amo.permissions.ADMIN_SCANNERS_RESULTS_EDIT
-        )
-        if not is_admin or request.method != 'POST':
-            raise Http404
-
-        result = self.get_object(request, pk)
-        result.update(state=UNKNOWN)
-
-        messages.add_message(
-            request,
-            messages.INFO,
-            f'Scanner result {pk} report has been reverted.',
-        )
-
-        return self.safe_referer_redirect(
-            request, default_url='admin:scanners_scannerresult_changelist'
-        )
-
-    def get_urls(self):
-        urls = super().get_urls()
-        info = self.model._meta.app_label, self.model._meta.model_name
-        custom_urls = [
-            re_path(
-                r'^(?P<pk>.+)/report-false-positive/$',
-                self.admin_site.admin_view(self.handle_false_positive),
-                name='%s_%s_handlefalsepositive' % info,
-            ),
-            re_path(
-                r'^(?P<pk>.+)/report-true-positive/$',
-                self.admin_site.admin_view(self.handle_true_positive),
-                name='%s_%s_handletruepositive' % info,
-            ),
-            re_path(
-                r'^(?P<pk>.+)/report-inconclusive/$',
-                self.admin_site.admin_view(self.handle_inconclusive),
-                name='%s_%s_handleinconclusive' % info,
-            ),
-            re_path(
-                r'^(?P<pk>.+)/revert-report/$',
-                self.admin_site.admin_view(self.handle_revert),
-                name='%s_%s_handlerevert' % info,
-            ),
-        ]
-        return custom_urls + urls
-
-    def result_actions(self, obj):
-        info = self.model._meta.app_label, self.model._meta.model_name
-        return render_to_string(
-            'admin/scannerresult_actions.html',
-            {
-                'handlefalsepositive_urlname': (
-                    'admin:%s_%s_handlefalsepositive' % info
-                ),
-                'handletruepositive_urlname': ('admin:%s_%s_handletruepositive' % info),
-                'handleinconclusive_urlname': ('admin:%s_%s_handleinconclusive' % info),
-                'handlerevert_urlname': 'admin:%s_%s_handlerevert' % info,
-                'obj': obj,
-            },
-        )
-
-    result_actions.short_description = 'Actions'
-    result_actions.allow_tags = True
-
     # Remove the "delete" button
     def has_delete_permission(self, request, obj=None):
         return False
@@ -933,11 +744,6 @@ def download(self, obj):
             )
         return '-'
 
-    def has_actions_permission(self, request):
-        return acl.action_allowed_for(
-            request.user, amo.permissions.ADMIN_SCANNERS_QUERY_EDIT
-        )
-
 
 @admin.register(ScannerRule)
 class ScannerRuleAdmin(AbstractScannerRuleAdminMixin, AMOModelAdmin):

src/olympia/scanners/migrations/0073_remove_scannerresult_state.py

@@ -0,0 +1,19 @@
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('scanners', '0072_delete_enable_customs_waffle_switch'),
+    ]
+
+    operations = [
+        migrations.RemoveIndex(
+            model_name='scannerresult',
+            name='scanners_re_state_2893a0_idx',
+        ),
+        migrations.RemoveField(
+            model_name='scannerresult',
+            name='state',
+        ),
+    ]

src/olympia/scanners/models.py

@@ -36,12 +36,10 @@
     NEW,
     NO_ACTION,
     QUERY_RULE_STATES,
-    RESULT_STATES,
     RUNNING,
     SCANNER_SERVICE_ACCOUNTS_GROUP,
     SCANNERS,
     SCHEDULED,
-    UNKNOWN,
     WEBHOOK,
     WEBHOOK_EVENTS,
     YARA,
@@ -357,9 +355,6 @@ class ScannerResult(AbstractScannerResult):
     )
     model_version = models.CharField(max_length=30, null=True)
     has_matches = models.BooleanField(null=True)
-    state = models.PositiveSmallIntegerField(
-        choices=RESULT_STATES.items(), null=True, blank=True, default=UNKNOWN
-    )
 
     class Meta(AbstractScannerResult.Meta):
         db_table = 'scanners_results'
@@ -370,7 +365,6 @@ class Meta(AbstractScannerResult.Meta):
             )
         ]
         indexes = [
-            models.Index(fields=('state',)),
             models.Index(fields=('has_matches',)),
         ]
 
@@ -395,12 +389,6 @@ def save(self, *args, **kwargs):
         for scanner_rule in matched_rules:
             self.matched_rules.add(scanner_rule)
 
-    def can_report_feedback(self):
-        return self.state == UNKNOWN
-
-    def can_revert_feedback(self):
-        return self.state != UNKNOWN
-
     @classmethod
     def run_action(cls, version):
         """Try to find and execute an action for a given version, based on the