Merge lp:~roadmr/canonical-identity-provider/fix-email-oops into lp:canonical-identity-provider/release

Proposed by Daniel Manrique
Status: Merged
Approved by: Daniel Manrique
Approved revision: no longer in the source branch.
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~roadmr/canonical-identity-provider/fix-email-oops
Merge into: lp:canonical-identity-provider/release
Diff against target: 44 lines (+15/-1)
2 files modified
src/identityprovider/admin.py (+1/-1)
src/identityprovider/tests/test_admin.py (+14/-0)
To merge this branch: bzr merge lp:~roadmr/canonical-identity-provider/fix-email-oops
Reviewer Review Type Date Requested Status
Celso Providelo (community) Approve
Review via email: mp+347072@code.launchpad.net

Commit message

Fix unicode error when building account links using displayname with non-ascii characters and format_html.

UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-6: ordinal not in range(128)
  File "django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "django/core/handlers/base.py", line 249, in _legacy_get_response
    response = self._get_response(request)
  File "django/core/handlers/base.py", line 217, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "django/core/handlers/base.py", line 215, in _get_response
    response = response.render()
  File "django/template/response.py", line 107, in render
    self.content = self.rendered_content
  File "django/template/response.py", line 84, in rendered_content
    content = template.render(context, self._request)
  File "django/template/backends/django.py", line 66, in render
    return self.template.render(context)
  File "django/template/base.py", line 207, in render
    return self._render(context)
  File "django/template/base.py", line 199, in _render
    return self.nodelist.render(context)
  File "django/template/base.py", line 990, in render
    bit = node.render_annotated(context)
  File "django/template/base.py", line 957, in render_annotated
    return self.render(context)
  File "django/template/loader_tags.py", line 177, in render
    return compiled_parent._render(context)
  File "django/template/base.py", line 199, in _render
    return self.nodelist.render(context)
  File "django/template/base.py", line 990, in render
    bit = node.render_annotated(context)
  File "django/template/base.py", line 957, in render_annotated
    return self.render(context)
  File "django/template/loader_tags.py", line 177, in render
    return compiled_parent._render(context)
  File "django/template/base.py", line 199, in _render
    return self.nodelist.render(context)
  File "django/template/base.py", line 990, in render
    bit = node.render_annotated(context)
  File "django/template/base.py", line 957, in render_annotated
    return self.render(context)
  File "django/template/loader_tags.py", line 72, in render
    result = block.nodelist.render(context)
  File "django/template/base.py", line 990, in render
    bit = node.render_annotated(context)
  File "django/template/base.py", line 957, in render_annotated
    return self.render(context)
  File "django/template/loader_tags.py", line 72, in render
    result = block.nodelist.render(context)
  File "django/template/base.py", line 990, in render
    bit = node.render_annotated(context)
  File "django/template/base.py", line 957, in render_annotated
    return self.render(context)
  File "django/template/library.py", line 225, in render
    _dict = self.func(*resolved_args, **resolved_kwargs)
  File "django/contrib/admin/templatetags/admin_list.py", line 340, in result_list
    'results': list(results(cl))}
  File "django/contrib/admin/templatetags/admin_list.py", line 316, in results
    yield ResultList(None, items_for_result(cl, res, None))
  File "django/contrib/admin/templatetags/admin_list.py", line 307, in __init__
    super(ResultList, self).__init__(*items)
  File "django/contrib/admin/templatetags/admin_list.py", line 218, in items_for_result
    f, attr, value = lookup_field(field_name, result, cl.model_admin)
  File "django/contrib/admin/utils.py", line 295, in lookup_field
    value = attr(obj)
  File "identityprovider/admin.py", line 433, in account_link
    return build_account_link(obj.account)
  File "identityprovider/admin.py", line 63, in build_account_link
    account.openid_identifier)
  File "django/utils/html.py", line 94, in format_html
    return mark_safe(format_string.format(*args_safe, **kwargs_safe))

Description of the change

Fix unicode error when building account links using displayname with non-ascii characters and format_html.

To post a comment you must log in.
Revision history for this message
Celso Providelo (cprov) wrote :

Thanks, Daniel.

I cogitate unicode_literals for the entire file, but for the specific bugfix, I am think this specific/precise change is better.

review: Approve
Revision history for this message
Celso Providelo (cprov) wrote :

Looks good if the admin code & testing are already unicode-friendly, unicode_literals is the way to go ...

The problem is not adopting it for the entire project in a timely manner, developers will get confused, for instance changes that work on admin.py will not work in models.py :-/

But let's go for it, since there is really no way back (think python 2.1!)

Minor comment inline about importing style.

review: Approve
Revision history for this message
Daniel Manrique (roadmr) wrote :

I'm working on some cleanups so I may be able to look at unicode_literals'ing the entire thing, and see if it's not too painful.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/identityprovider/admin.py'
--- src/identityprovider/admin.py 2018-05-25 21:08:49 +0000
+++ src/identityprovider/admin.py 2018-05-29 17:14:40 +0000
@@ -1,7 +1,7 @@
1# Copyright 2010,2013 Canonical Ltd. This software is licensed under the1# Copyright 2010,2013 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4from __future__ import absolute_import4from __future__ import absolute_import, unicode_literals
55
6from urllib import urlencode6from urllib import urlencode
77
88
=== modified file 'src/identityprovider/tests/test_admin.py'
--- src/identityprovider/tests/test_admin.py 2018-05-14 14:29:13 +0000
+++ src/identityprovider/tests/test_admin.py 2018-05-29 17:14:40 +0000
@@ -1,7 +1,10 @@
1# -*- coding: utf-8 -*-
1# Copyright 2010, 2012 Canonical Ltd. This software is licensed under2# Copyright 2010, 2012 Canonical Ltd. This software is licensed under
2# the GNU Affero General Public License version 3 (see the file3# the GNU Affero General Public License version 3 (see the file
3# LICENSE).4# LICENSE).
45
6from __future__ import unicode_literals
7
5from django.conf import settings8from django.conf import settings
6from django.contrib import admin9from django.contrib import admin
7from django.contrib.admin.sites import (10from django.contrib.admin.sites import (
@@ -406,6 +409,17 @@
406 openid = email.account.openid_identifier409 openid = email.account.openid_identifier
407 self.assertEqual(self.admin.account_link(email), expected % openid)410 self.assertEqual(self.admin.account_link(email), expected % openid)
408411
412 @patch('identityprovider.admin.reverse')
413 def test_account_link_non_ascii(self, mock_reverse):
414 mock_reverse.return_value = '/foo'
415 displayname = "mr. 北京 año sé 🐈"
416 email = EmailAddress.objects.get(email='test@canonical.com')
417 email.account.displayname = displayname
418
419 expected = '<a href="/foo">mr. 北京 año sé 🐈</a> (OpenID: %s)'
420 openid = email.account.openid_identifier
421 self.assertEqual(self.admin.account_link(email), expected % openid)
422
409423
410class InvalidatedEmailAddressAdminTestCase(EmailAddressAdminTestCase):424class InvalidatedEmailAddressAdminTestCase(EmailAddressAdminTestCase):
411425