Opened 18 years ago

Closed 17 years ago

Last modified 17 years ago

#2135 closed enhancement (wontfix)

[patch] Slug pre-population should be possible in the server

Reported by: greg[at]abbas.org Owned by: Adrian Holovaty
Component: Core (Other) Version:
Severity: normal Keywords: slug unique
Cc: gary.wilson@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

It should be possible to auto-generate a slug from a title, without using the admin's javascript, because as snazzy as the admin app is, perhaps you're creating an object with a remote API (e.g. Atom) or something. This patch includes a Python version of "URLify" and a "prepopulate" model method to call it.

Attachments (4)

urlify.diff (3.2 KB ) - added by greg[at]abbas.org 18 years ago.
Patch for svn trunk
urlify.2.diff (3.4 KB ) - added by greg[at]abbas.org 18 years ago.
oops, uploaded the wrong version the first time
urlify.3.diff (5.6 KB ) - added by greg[at]abbas.org 18 years ago.
made slugify template tag use URLify, too
urlify.4.diff (6.2 KB ) - added by greg[at]abbas.org 18 years ago.
fix strip & doctest

Download all attachments as: .zip

Change History (16)

by greg[at]abbas.org, 18 years ago

Attachment: urlify.diff added

Patch for svn trunk

by greg[at]abbas.org, 18 years ago

Attachment: urlify.2.diff added

oops, uploaded the wrong version the first time

comment:1 by Ramiro Morales, 18 years ago

+1 on this feature

I tried using SlugField and found that the JavaScript onkeyup event (the one used to fill in the field) isn't triggered when the user pastes text (insted
of typing it) in the fields specified in the prepopulate_from list and as a result the slug stays empty. Doing this in the server side by calling greg's prepopulate() from the model save() method could make it more robust.

comment:2 by anonymous, 18 years ago

django.template.defaultfilters.slugify should be changed to use this patch.

by greg[at]abbas.org, 18 years ago

Attachment: urlify.3.diff added

made slugify template tag use URLify, too

comment:3 by greg[at]abbas.org, 18 years ago

good point. :-)

comment:4 by nospam@…, 18 years ago

I was importing data and noticed that I get a "-" at the beginning of the phrase when a word from the removelist is used at the beginning. I used "the test of testers" and noticed the string came out as "-test-testers". I'm still relatively new to python + django so I'm sure there's a better, more regular expression friendly way to do this:

s = inside_space_pat.sub(' ', s) # convert spaces to hyphens
s = str( s ).strip().replace( ' ', '-' )

by greg[at]abbas.org, 18 years ago

Attachment: urlify.4.diff added

fix strip & doctest

comment:5 by greg[at]abbas.org, 18 years ago

thanks web-ology. yeah, your code looks fine; I did it slightly differently just because I wanted to match urlify.js more closely (which I should've done in the first place :-) ). I added a regression test, too.

comment:6 by anonymous, 18 years ago

Cc: gary.wilson@… added

comment:7 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedDesign decision needed

comment:8 by sime <simon@…>, 17 years ago

Agree, javascript should only be fallback. I generate models all the time from code, slugs should def be handled automagically. Historically I only ever trust server side for something as important as slugs.

Two issues I see here though --

  1. Better to hook to the save() method instead of prepopulate()? So if the slug is not blank=True but it's attempting to be saved blank, then django should insert an auto generated one.
  1. If the slug field has unique=True (usually the case??), then it needs to loop until it finds a unique key in the database! Important.

Does anyone want the code??

comment:9 by sime <simon@…>, 17 years ago

Also btw I find it really just a bit inadequate that the current javascript doesn't find a database unique slug. End users shouldn't need to have to figure that out.

I'd even suggest hiding the slug field on initial 'add' form in admin (so the above described function takes care of) then on 'change', show the field, so users can customise if necessary (but warn them they should avoid changing too long after initial creation, since it can harm SEO). Ie, remove the javascript altogether.

My two cents worth. I previously ran one of our country's biggest sites, with millions of slugged records in some tables. Getting slugs right is important.

comment:10 by sime <simon@…>, 17 years ago

Keywords: unique added
Patch needs improvement: set

One last comment. The slugify URLify function here needs to wordwrap rather than truncate.

Here is my version --

def slugify(s, num_chars=50): 
    ''' 
    Changes, e.g., "Petty theft" to "petty-theft". 
    This function is the Python equivalent of the javascript function 
    of the same name in django/contrib/admin/media/js/urlify.js. 
    It can get invoked for any field that has a prepopulate_from 
    attribute defined, although it only really makes sense for 
    SlugFields. 
     
    NOTE: this implementation corresponds to the Python implementation 
          of the same algorithm in django/contrib/admin/media/js/urlify.js 
    ''' 
    # remove all these words from the string before urlifying 
    removelist = ["a", "an", "as", "at", "before", "but", "by", "for", 
                  "from", "is", "in", "into", "like", "of", "off", "on", 
                  "onto", "per", "since", "than", "the", "this", "that", 
                  "to", "up", "via", "with"] 
    ignore_words = '|'.join([r for r in removelist]) 
    ignore_words_pat = re.compile('\b('+ignore_words+')\b', re.I) 
    ignore_chars_pat = re.compile(r'[^-A-Z0-9\s]', re.I) 
    outside_space_pat = re.compile(r'^\s+|\s+$') 
    inside_space_pat = re.compile(r'[-\s]+') 
     
    s = ignore_words_pat.sub('', s)  # remove unimportant words 
    s = ignore_chars_pat.sub('', s)  # remove unneeded chars 
    wordlist = s.split()
    s = ''
    for w in wordlist:
        ns = s + w + ' '
        if len(ns) <= num_chars:
            s = ns
        else:
            break
    s = outside_space_pat.sub('', s) # trim leading/trailing spaces
    s = inside_space_pat.sub('-', s) # convert spaces to hyphens 
    s = s.lower()                    # convert to lowercase 
    return s

And an example of the code I've added to my specific model's save() function. As I mention above, A generic version something like this needs to be incorporated into core in my opinion --

        if self.slug in ('', None):
            slug = slugify(self.name, 47) # allow three chars for upto two digits max uniqueness 
            slug_index = 1
            while True:
                try:
                    check = Item.objects.get(slug=slug)
                    slug_index += 1
                    slug = slug + '-' + str(slug_index)
                except Item.DoesNotExist:
                    break
            self.slug = slug

comment:11 by Malcolm Tredinnick, 17 years ago

Resolution: wontfix
Status: newclosed

A lot of the comments here seem to be based on large misunderstandings of what the slug field population is designed for. It provides a suggested slug that the user can edit if they wish to. Checking that it is unique in the database is a validation-time function, not something the edit field is going to do by default (since the user might still edit it up to the moment they submit the form)

If you don't want the user editing the slug field, don't display it on the admin form and use the Python slugify() function (or your own replacement) to do what you want. This is entirely up to the developer; it's not a policy that Django is going to require, because it depends on an application's slug-creation policy.

There is also no requirement for the slugify filter to be the same as the Javascript (this isn't possible in practice, since we can do much more w.r.t. Unicode handling on the Python side).

So, thanks for the patches, but I don't think we should include this. It doesn't add anything that isn't already possible and only adds extra "policy" that is not always going to suit others' use-cases.

comment:12 by sime <simon@…>, 17 years ago

Have you got an example use-case that this suggestion doesn't suit?

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