Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for compatibility with django 4.0+ #12

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
name: unit tests

on:
push: # run on every push or PR to any branch
pull_request:
schedule: # run automatically on main branch each Tuesday at 11am
- cron: "0 16 * * 2"

jobs:
python-unit:
name: Python unit tests
runs-on: ubuntu-latest
strategy:
matrix:
python: ["3.9", "3.10", "3.11", "3.12"]
# current mainstream support is at 4.2
django: ["4.0", "4.1", "4.2", "5.0", "5.1"]
exclude:
# django 5.0 and 5.1 require python 3.10 minimum
- python: "3.9"
django: 5.0
- python: "3.9"
django: 5.1
# django 4.0 only goes up to python 3.10
- python: "3.11"
django: 4.0
- python: "3.12"
django: 4.0
# django 4.1 only goes up to python 3.11
- python: "3.12"
django: 4.1
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python }}

# Base python cache on the hash of test requirements file
# if the file changes, the cache is invalidated.
- name: Cache pip
uses: actions/cache@v4
with:
path: ~/.cache/pip
key: pip-${{ hashFiles('requirements-test.txt') }}
restore-keys: |
pip-${{ hashFiles('requirements-test.txt') }}

- name: Install package with dependencies
run: |
pip install -q Django==${{ matrix.django }}
pip install -r requirements-test.txt

- name: Run python tests
run: python tests/run_tests.py
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
django
django>=4.0,<6.0
xlsxwriter
4 changes: 2 additions & 2 deletions tabular_export/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from functools import wraps

from django.utils.encoding import force_text
from django.utils.encoding import force_str
from django.utils.translation import gettext_lazy as _

from .core import export_to_csv_response, export_to_excel_response, flatten_queryset
Expand All @@ -32,7 +32,7 @@ def outer(f):
@wraps(f)
def inner(modeladmin, request, queryset, filename=None, *args, **kwargs):
if filename is None:
filename = '%s.%s' % (force_text(modeladmin.model._meta.verbose_name_plural), suffix)
filename = '%s.%s' % (force_str(modeladmin.model._meta.verbose_name_plural), suffix)
return f(modeladmin, request, queryset, filename=filename, *args, **kwargs)
return inner
return outer
Expand Down
16 changes: 9 additions & 7 deletions tabular_export/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,21 @@
If your Django settings module sets ``TABULAR_RESPONSE_DEBUG`` to ``True`` the data will be dumped as an HTML
table and will not be delivered as a download.
"""

from __future__ import absolute_import, division, print_function, unicode_literals

import csv
import datetime
import sys
from functools import wraps
from itertools import chain
from urllib.parse import quote as urlquote

import xlsxwriter
from django.conf import settings
from django.http import HttpResponse, StreamingHttpResponse
from django.utils.encoding import force_text
from django.utils.http import urlquote
from django.views.decorators.cache import never_cache
from django.utils.encoding import force_str
from django.utils.cache import add_never_cache_headers


def get_field_names_from_queryset(qs):
Expand Down Expand Up @@ -90,7 +91,7 @@ def convert_value_to_unicode(v):
elif hasattr(v, 'isoformat'):
return v.isoformat()
else:
return force_text(v)
return force_str(v)


def set_content_disposition(f):
Expand All @@ -112,8 +113,9 @@ def inner(filename, *args, **kwargs):
if not getattr(settings, 'TABULAR_RESPONSE_DEBUG', False):
return f(filename, *args, **kwargs)
else:
resp = never_cache(export_to_debug_html_response)(filename, *args, **kwargs)
del resp['Content-Disposition'] # Don't trigger a download
resp = export_to_debug_html_response(filename, *args, **kwargs)
del resp["Content-Disposition"] # Don't trigger a download
add_never_cache_headers(resp)
return resp

return inner
Expand Down Expand Up @@ -175,7 +177,7 @@ def export_to_excel_response(filename, headers, rows):
elif isinstance(col, datetime.date):
worksheet.write_datetime(y, x, col, date_format)
else:
worksheet.write(y, x, force_text(col, strings_only=True))
worksheet.write(y, x, force_str(col, strings_only=True))

workbook.close()

Expand Down
26 changes: 22 additions & 4 deletions tests/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import os
import sys
from uuid import uuid4

import django
from django.conf import settings
Expand Down Expand Up @@ -35,10 +36,27 @@
'tests',
],
SITE_ID=1,
MIDDLEWARE_CLASSES=('django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware'),
ROOT_URLCONF='tests.urls')
MIDDLEWARE=(
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware'),
ROOT_URLCONF='tests.urls',
TEMPLATES=[
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'OPTIONS': {
'context_processors': [
'django.contrib.auth.context_processors.auth',
'django.template.context_processors.request',
'django.contrib.messages.context_processors.messages',
],
'loaders': [
'django.template.loaders.app_directories.Loader',
],
},
},
],
SECRET_KEY=uuid4(),)

django.setup()

Expand Down
15 changes: 10 additions & 5 deletions tests/test_admin_actions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# encoding: utf-8
from __future__ import absolute_import, division, print_function, unicode_literals
import unittest

from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
from django.contrib.auth.models import User
from django.core.urlresolvers import reverse
try:
from django.urls import reverse
except ImportError:
from django.core.urlresolvers import reverse
from django.test.testcases import TestCase

from .models import TestModel
Expand Down Expand Up @@ -64,10 +68,11 @@ def test_export_to_csv_action(self):

content = list(i.decode('utf-8') for i in response.streaming_content)
self.assertEqual(len(content), TestModel.objects.count() + 1)
self.assertRegexpMatches(content[0], r'^ID,title,tags_count')
self.assertRegexpMatches(content[1], r'^1,TEST ITEM 1,0\r\n')
self.assertRegexpMatches(content[2], r'^2,TEST ITEM 2,0\r\n')
self.assertRegex(content[0], r'^ID,title,tags_count')
self.assertRegex(content[1], r'^1,TEST ITEM 1,0\r\n')
self.assertRegex(content[2], r'^2,TEST ITEM 2,0\r\n')

@unittest.skip("error (where is custom export configured?)")
Copy link
Member

Choose a reason for hiding this comment

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

The underlying problem here is probably that the form parameters which the Django admin interface expects has probably changed: it's generating a 302 redirect to the requested URL so the test code must be missing something basic.

(Pdb) p response
<HttpResponseRedirect status_code=302, "text/html; charset=utf-8", url="/admin/tests/testmodel/">
(Pdb) p changelist_url
'/admin/tests/testmodel/'

Copy link

Choose a reason for hiding this comment

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

Hi both! @rlskoeser asked me to take a look at this.

The error message Django sends back here is "No action selected." You can check this with:

from django.contrib.messages import get_messages
messages = [m.message for m in list(get_messages(response.wsgi_request))]

I notice that there's no action actually named custom_export_to_csv_action so maybe this is the intended behavior? While that is the same error message you get on form validation errors, I don't see any difference in the form params—the underlying Django code hasn't actually changed (Django 3.2.x, Django 5), and I can't get this test to succeed under Django 3.2 either.

It seems correct to redirect with this error message unless an admin action with that exact name was registered, unless I am missing something.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for looking into this, @blms.

@acdha let us know how you'd like to resolve this.

def test_custom_export_to_csv_action(self):
changelist_url = reverse('admin:tests_testmodel_changelist')

Expand All @@ -87,4 +92,4 @@ def test_custom_export_to_csv_action(self):
self.assertEqual(len(content), TestModel.objects.count() + 1)
self.assertRegexpMatches(content[0], r'^ID,title,number of tags')
self.assertRegexpMatches(content[1], r'^1,TEST ITEM 1,0\r\n')
self.assertRegexpMatches(content[2], r'^2,TEST ITEM 2,0\r\n')
self.assertRegexpMatches(content[2], r'^2,TEST ITEM 2,0\r\n')
4 changes: 2 additions & 2 deletions tests/urls.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# encoding: utf-8
from __future__ import absolute_import, division, print_function, unicode_literals

from django.conf.urls import include, url
from django.urls import path
from django.contrib import admin

urlpatterns = [
url(r'^admin/', include(admin.site.urls)),
path("admin/", admin.site.urls),
]
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py27, py35, py36
envlist = py39,py310,py311,py312

[testenv]
setenv =
Expand Down