Opened 7 years ago

Closed 7 years ago

#8322 closed (invalid)

Admin fails to add a slash between MEDIA_URL and upload_to

Reported by: to.roma.from.djbug@… Owned by: nobody
Component: contrib.admin Version:
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

With MEDIA_URL="/media" and upload_to="somefolder" the admin, while uploading files properly, shows the link to the newly uploaded file as “/mediasomefolder/filename” without the slash.

Expected: to add a slash.

For consistency, the trailing slash should never be required on paths.

Change History (4)

comment:1 Changed 7 years ago by brosner

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed
  • Version 1.0-alpha deleted

MEDIA_URL must have a trailing slash per the documentation http://www.djangoproject.com/documentation/settings/#media-root.

comment:3 Changed 7 years ago by to.roma.from.djbug@…

  • Resolution invalid deleted
  • Status changed from closed to reopened

I believe that requirement should be dropped.

The 1.0 alpha code mentions MEDIA_URL six times:

/usr/share/python-support/python-django/django :) egrep '\<MEDIA_URL\>' -r .
./db/models/base.py:            return urlparse.urljoin(settings.MEDIA_URL, getattr(self, field.attname)).replace('\\', '/')
./conf/global_settings.py:MEDIA_URL = ''
./conf/project_template/settings.py:MEDIA_URL = ''
./core/context_processors.py:    return {'MEDIA_URL': settings.MEDIA_URL}
./contrib/admin/widgets.py:                (_('Currently:'), settings.MEDIA_URL, value, value, _('Change:')))
./forms/widgets.py:        return urljoin(settings.MEDIA_URL,path)

See how it uses urljoin in all places but one, django/contrib/admin/widgets.py:89:

            output.append('%s <a target="_blank" href="%s%s">%s</a> <br />%s ' % \
                (_('Currently:'), settings.MEDIA_URL, value, value, _('Change:')))

If it’s changed to read:

            output.append('%s <a target="_blank" href="%s">%s</a> <br />%s ' % \
                (_('Currently:'), urlparse.urljoin(settings.MEDIA_URL, value), value, _('Change:')))

then it won’t matter whether MEDIA_URL ends in a slash or not.

It’s a convention that comes perhaps from shell scripts that paths should not have trailing slashes, $PREFIX/bin where PREFIX=/usr is more readable and self-documenting than ${PREFIX}bin where PREFIX=/usr/. To my mind, ‘href="%s/%s"’ is very clear while ‘href="%s%s"’ is much less so (the latter could have been filename and extension, for example). And urljoin is even more readable.

comment:4 Changed 7 years ago by to.roma.from.djbug@…

  • Resolution set to invalid
  • Status changed from reopened to closed

Please ignore the last comment, I misunderstood how urljoin works.

Though it would be quite fine not to require the URLs to end in slashes.

Note: See TracTickets for help on using tickets.
Back to Top