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

Fixed #26440 -- Added system checks to verify urlpatterns contains valid objects. #6375

Closed
wants to merge 4 commits into from

Conversation

burhan
Copy link

@burhan burhan commented Apr 2, 2016

https://code.djangoproject.com/ticket/26440

  1. Adding new tests for "fake" objects in URLs
  2. Adding new tests for case where user forgets to type url, and ends up with a tuple (r'', foo, name=df)
  3. Added new warnings.

errors = []

if not hasattr(pattern, 'regex'):
errors.append(Warning("url objects should have a property regex",
Copy link
Member

Choose a reason for hiding this comment

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

This message should include some helpful information to debug the error. Currently it gives no information as to where the problem is. This is the idea behind the existing describe_pattern() function.

@timgraham timgraham changed the title Bug 26440 Fixed #26440 -- Added system checks to verify urlpatterns contains valid objects. Apr 2, 2016
"Remove this slash as it is unnecessary.".format(describe_pattern(pattern)),
id="urls.W002",
"Your URL pattern {} is not an object returned from the 'url()'"
"method. If you are using a custom object, it should provide a `pattern`"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit hesitant about exposing the private details of url resolving in user-facing messages, so I would remove the comment about a custom object. Custom url objects are an advanced and unsupported use-case (for now), and I think this warning should target the mistakes made when upgrading/using the new structure, not the ones made when replacing the internal and private details of url resolving.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@timgraham
Copy link
Member

Also, the tests aren't passing, e.g.:

  File "/home/tim/code/django/django/core/checks/urls.py", line 84, in check_pattern_startswith_slash
    "forgotten to add 'url' to your pattern.".format(describe_pattern(pattern)),
  File "/home/tim/code/django/django/core/checks/urls.py", line 40, in describe_pattern
    description = "'{}'".format(pattern.regex.pattern)
AttributeError: 'str' object has no attribute 'pattern'

@alasdairnicol
Copy link
Contributor

alasdairnicol commented May 1, 2016

This can be closed now that #6514 is merged.

@timgraham timgraham closed this May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants