Skip to content

Commit

Permalink
bug-1809726: fix deprecation warnings
Browse files Browse the repository at this point in the history
This fixes a bunch of DeprecationWarnings introduced with Django 4.1 and
4.2 for things that are going away in Django 5.0.

* change in how forms are rendered -- I picked `as_table()` for now
  since that's what it used to do; we want to switch to the new
  accessible-friendly form rendering--I'll write up a bug to do that
* deprecating django.utils.timezone.utc -- these are all switched to
  datetime.timezone.utc
* pass in headers as headers dict to test client
* switch setting admin method properties to use a decorator

This also fixes a new bug where RequestFactory `wsgi.input` value needs
a `read()` method.
  • Loading branch information
willkg committed Feb 28, 2024
1 parent 4f5a5da commit e9cecf8
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 48 deletions.
20 changes: 11 additions & 9 deletions webapp/crashstats/api/tests/test_views.py
Expand Up @@ -110,7 +110,7 @@ def test_base_classes_raise_not_found(self):
def test_option_CORS(self):
"""OPTIONS request for model_wrapper returns CORS headers"""
url = reverse("api:model_wrapper", args=("NoOp",))
response = self.client.options(url, HTTP_ORIGIN="http://example.com")
response = self.client.options(url, headers={"origin": "http://example.com"})
assert response.status_code == 200
assert response["Access-Control-Allow-Origin"] == "*"

Expand Down Expand Up @@ -158,14 +158,14 @@ def test_hit_or_not_hit_ratelimit(self):
# https://bugzilla.mozilla.org/show_bug.cgi?id=1148470
for _ in range(current_limit * 2):
response = self.client.get(
url, {"product": "good"}, HTTP_X_REAL_IP="12.12.12.12"
url, {"product": "good"}, headers={"x-real-ip": "12.12.12.12"}
)
assert response.status_code == 429

# But it'll work if you use a different X-Real-IP
# because the rate limit is based on your IP address
response = self.client.get(
url, {"product": "good"}, HTTP_X_REAL_IP="11.11.11.11"
url, {"product": "good"}, headers={"x-real-ip": "11.11.11.11"}
)
assert response.status_code == 200

Expand All @@ -175,7 +175,7 @@ def test_hit_or_not_hit_ratelimit(self):
)

response = self.client.get(
url, {"product": "good"}, HTTP_AUTH_TOKEN=token.key
url, {"product": "good"}, headers={"auth-token": token.key}
)
assert response.status_code == 200

Expand Down Expand Up @@ -255,7 +255,9 @@ def mocked_get(**params):
token = Token.objects.create(user=user, notes="test token")
token.permissions.add(view_pii_perm)

response = self.client.get(url, {"crash_id": "123"}, HTTP_AUTH_TOKEN=token.key)
response = self.client.get(
url, {"crash_id": "123"}, headers={"auth-token": token.key}
)
assert response.status_code == 200
dump = json.loads(response.content)
for key in public_data.keys():
Expand Down Expand Up @@ -326,7 +328,7 @@ def mocked_get(**params):
token.permissions.add(view_pii_perm)

response = self.client.get(
url, {"crash_id": "abc123"}, HTTP_AUTH_TOKEN=token.key
url, {"crash_id": "abc123"}, headers={"auth-token": token.key}
)
assert response.status_code == 200
dump = json.loads(response.content)
Expand Down Expand Up @@ -548,7 +550,7 @@ def mocked_publish(queue, crash_ids):
assert response.status_code == 403

params = {"crash_ids": crash_id}
response = self.client.get(url, params, HTTP_AUTH_TOKEN="somecrap")
response = self.client.get(url, params, headers={"auth-token": "somecrap"})
assert response.status_code == 403

user = User.objects.create(username="test")
Expand All @@ -560,10 +562,10 @@ def mocked_publish(queue, crash_ids):
token = Token.objects.create(user=user, notes="Only reprocessing")
token.permissions.add(perm)

response = self.client.get(url, params, HTTP_AUTH_TOKEN=token.key)
response = self.client.get(url, params, headers={"auth-token": token.key})
assert response.status_code == 405

response = self.client.post(url, params, HTTP_AUTH_TOKEN=token.key)
response = self.client.post(url, params, headers={"auth-token": token.key})
assert response.status_code == 200
assert json.loads(response.content) is True

Expand Down
6 changes: 2 additions & 4 deletions webapp/crashstats/authentication/admin.py
Expand Up @@ -38,12 +38,12 @@ class UserAdminBetter(UserAdmin):
"get_policyexception",
]

@admin.display(description="Hacker (PII)")
def is_hacker(self, obj):
"""Return whether user is in the Hackers group and has access to PII."""
return obj.groups.filter(name="Hackers").exists()

is_hacker.short_description = "Hacker (PII)"

@admin.display(description="PolicyException")
def get_policyexception(self, obj):
"""Return whether user has a PolicyException with links to change/delete or add one."""
if hasattr(obj, "policyexception"):
Expand All @@ -57,8 +57,6 @@ def get_policyexception(self, obj):
)
return format_html('<a href="{}">Create</a>', url)

get_policyexception.short_description = "PolicyException"


@admin.register(PolicyException)
class PolicyExceptionAdmin(admin.ModelAdmin):
Expand Down
18 changes: 8 additions & 10 deletions webapp/crashstats/crashstats/admin.py
Expand Up @@ -30,14 +30,14 @@ class LogEntryAdmin(admin.ModelAdmin):

list_display = [
"action_time",
"admin",
"admin_email",
"object_link",
"action",
"get_change_message",
]
list_display_links = ["action_time", "get_change_message"]

def admin(self, obj):
def admin_email(self, obj):
return obj.user.email

def action(self, obj):
Expand All @@ -51,6 +51,10 @@ def obj_repr(self, obj):
return edited_obj.email
return edited_obj

@admin.display(
description="object",
ordering="object_repr",
)
def object_link(self, obj):
object_link = self.obj_repr(obj) # Default to just name
content_type = obj.content_type
Expand All @@ -69,14 +73,10 @@ def object_link(self, obj):
pass
return object_link

object_link.admin_order_field = "object_repr"
object_link.short_description = "object"

@admin.display(description="change message")
def get_change_message(self, obj):
return obj.get_change_message()

get_change_message.short_description = "change message"

def has_add_permission(self, request):
return False

Expand Down Expand Up @@ -127,6 +127,7 @@ class SignatureAdmin(admin.ModelAdmin):
search_fields = ["signature"]


@admin.action(description="Process crashes")
def process_crashes(modeladmin, request, queryset):
"""Process selected missing processed crashes from admin page."""
priority_api = PriorityJob()
Expand All @@ -137,9 +138,6 @@ def process_crashes(modeladmin, request, queryset):
)


process_crashes.short_description = "Process crashes"


@admin.register(MissingProcessedCrash)
class MissingProcessedCrashAdmin(admin.ModelAdmin):
list_display = [
Expand Down
5 changes: 3 additions & 2 deletions webapp/crashstats/crashstats/tests/test_views.py
Expand Up @@ -3,6 +3,7 @@
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

import copy
from io import StringIO
import json
import re
from unittest import mock
Expand Down Expand Up @@ -201,7 +202,7 @@ def test_html(self):
handler500 = getattr(views, end)

# to make a mock call to the django view functions you need a request
fake_request = RequestFactory().request(**{"wsgi.input": None})
fake_request = RequestFactory().request(**{"wsgi.input": StringIO("")})

# the reason for first causing an exception to be raised is because
# the handler500 function is only called by django when an exception
Expand All @@ -223,7 +224,7 @@ def test_json(self):
views = __import__(par, globals(), locals(), [end], 0)
handler500 = getattr(views, end)

fake_request = RequestFactory().request(**{"wsgi.input": None})
fake_request = RequestFactory().request(**{"wsgi.input": StringIO("")})
# This is what the utils.json_view decorator sets on views
# that should eventually return JSON.
fake_request._json_view = True
Expand Down
13 changes: 6 additions & 7 deletions webapp/crashstats/settings/base.py
Expand Up @@ -320,11 +320,6 @@ def filter(self, record):
}


# No need to load it because we don't do i18n in this project
USE_I18N = False

USE_L10N = False

# True if old legacy URLs we handle should be permanent 301 redirects.
# Transitionally it might be safer to set this to False as we roll out the new
# django re-write of Socorro.
Expand Down Expand Up @@ -470,6 +465,12 @@ def filter(self, record):
DATABASES = {"default": dj_database_url.parse(DATABASE_URL)}


STORAGES = {
"staticfiles": {
"BACKEND": "pipeline.storage.PipelineManifestStorage",
},
}

STATICFILES_FINDERS = [
"django.contrib.staticfiles.finders.FileSystemFinder",
"django.contrib.staticfiles.finders.AppDirectoriesFinder",
Expand All @@ -479,8 +480,6 @@ def filter(self, record):
"crashstats.crashstats.finders.LeftoverPipelineFinder",
]

STATICFILES_STORAGE = "pipeline.storage.PipelineManifestStorage"

PIPELINE = {
"STYLESHEETS": PIPELINE_CSS,
"JAVASCRIPT": PIPELINE_JS,
Expand Down
4 changes: 2 additions & 2 deletions webapp/crashstats/supersearch/form_fields.py
Expand Up @@ -2,12 +2,12 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

import datetime
import operator

import isodate

from django import forms
from django.utils.timezone import utc
from django.utils.encoding import smart_str

from crashstats.crashstats.utils import parse_isodate
Expand Down Expand Up @@ -206,7 +206,7 @@ class IsoDateTimeField(forms.DateTimeField):
def to_python(self, value):
if value:
try:
return parse_isodate(value).replace(tzinfo=utc)
return parse_isodate(value).replace(tzinfo=datetime.timezone.utc)
except (ValueError, isodate.isoerror.ISO8601Error):
# let the super method deal with that
pass
Expand Down
9 changes: 4 additions & 5 deletions webapp/crashstats/supersearch/tests/test_form_fields.py
Expand Up @@ -6,7 +6,6 @@

import pytest

from django.utils.timezone import utc
from django.forms import ValidationError

from crashstats.supersearch import form_fields
Expand Down Expand Up @@ -127,31 +126,31 @@ def test_gt_datetime(self):
field = form_fields.DateTimeField()
cleaned_value = field.clean([">12/31/2012 10:20:30"])
dt = datetime.datetime(2012, 12, 31, 10, 20, 30)
dt = dt.replace(tzinfo=utc)
dt = dt.replace(tzinfo=datetime.timezone.utc)
assert cleaned_value == [dt]
assert field.prefixed_value == [">2012-12-31T10:20:30+00:00"]

def test_gte_date(self):
field = form_fields.DateTimeField()
cleaned_value = field.clean([">=2012-12-31"])
dt = datetime.datetime(2012, 12, 31)
dt = dt.replace(tzinfo=utc)
dt = dt.replace(tzinfo=datetime.timezone.utc)
assert cleaned_value == [dt]
assert field.prefixed_value == [">=2012-12-31T00:00:00+00:00"]

def test_gte_datetime(self):
field = form_fields.DateTimeField()
cleaned_value = field.clean([">=2012-12-31T01:02:03+00:00"])
dt = datetime.datetime(2012, 12, 31, 1, 2, 3)
dt = dt.replace(tzinfo=utc)
dt = dt.replace(tzinfo=datetime.timezone.utc)
assert cleaned_value == [dt]
assert field.prefixed_value == [">=2012-12-31T01:02:03+00:00"]

def test_duplicate_values(self):
field = form_fields.DateTimeField()
cleaned_value = field.clean(["<2016-08-10", "<2016-08-10"])
dt = datetime.datetime(2016, 8, 10)
dt = dt.replace(tzinfo=utc)
dt = dt.replace(tzinfo=datetime.timezone.utc)
assert cleaned_value == [dt, dt]

def test_invalid_combinations(self):
Expand Down
17 changes: 12 additions & 5 deletions webapp/crashstats/supersearch/tests/test_utils.py
Expand Up @@ -4,7 +4,6 @@

import datetime

from django.utils.timezone import utc

from crashstats.topcrashers.views import get_date_boundaries

Expand All @@ -15,8 +14,12 @@ def test_get_date_boundaries(self):
start, end = get_date_boundaries(
{"date": [">2010-03-01T12:12:12", "<=2010-03-10T00:00:00"]}
)
assert start == datetime.datetime(2010, 3, 1, 12, 12, 12).replace(tzinfo=utc)
assert end == datetime.datetime(2010, 3, 10).replace(tzinfo=utc)
assert start == datetime.datetime(2010, 3, 1, 12, 12, 12).replace(
tzinfo=datetime.timezone.utc
)
assert end == datetime.datetime(2010, 3, 10).replace(
tzinfo=datetime.timezone.utc
)

# Test with messy dates.
start, end = get_date_boundaries(
Expand All @@ -29,5 +32,9 @@ def test_get_date_boundaries(self):
]
}
)
assert start == datetime.datetime(2009, 1, 1, 12, 12, 12).replace(tzinfo=utc)
assert end == datetime.datetime(2010, 3, 11).replace(tzinfo=utc)
assert start == datetime.datetime(2009, 1, 1, 12, 12, 12).replace(
tzinfo=datetime.timezone.utc
)
assert end == datetime.datetime(2010, 3, 11).replace(
tzinfo=datetime.timezone.utc
)
2 changes: 1 addition & 1 deletion webapp/crashstats/supersearch/utils.py
Expand Up @@ -31,7 +31,7 @@ def get_date_boundaries(params):
else:
for param in params["date"]:
d = isodate.parse_datetime(split_on_operator(param)[1]).replace(
tzinfo=timezone.utc
tzinfo=datetime.timezone.utc
)

if param.startswith("<") and (not end_date or end_date < d):
Expand Down
2 changes: 1 addition & 1 deletion webapp/crashstats/tokens/jinja2/tokens/home.html
Expand Up @@ -115,7 +115,7 @@ <h2>Generate a New Token</h2>
<form method="post">
{% csrf_token %}
<table class="data-table">
{{ form }}
{{ form.as_table() }}
<tr>
<th>&nbsp;</th>
<td>
Expand Down
5 changes: 3 additions & 2 deletions webapp/crashstats/topcrashers/tests/test_views.py
Expand Up @@ -9,7 +9,6 @@

from django.urls import reverse
from django.utils.encoding import smart_str
from django.utils.timezone import utc

from crashstats.crashstats.models import Signature, BugAssociation
from socorro.lib.libdatetime import utc_now
Expand Down Expand Up @@ -63,7 +62,9 @@ def test_topcrashers_first_appearance(self, client, db, es_helper):

Signature.objects.create(
signature=signature,
first_date=datetime.datetime(2021, 1, 1, 12, 23, 34, tzinfo=utc),
first_date=datetime.datetime(
2021, 1, 1, 12, 23, 34, tzinfo=datetime.timezone.utc
),
first_build="20000101122334",
)

Expand Down

0 comments on commit e9cecf8

Please sign in to comment.