Code

Opened 8 years ago

Closed 7 years ago

Last modified 7 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
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: UI/UX:

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

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by greg[at]abbas.org

Patch for svn trunk

Changed 8 years ago by greg[at]abbas.org

oops, uploaded the wrong version the first time

comment:1 Changed 8 years ago by ramiro

+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 Changed 8 years ago by anonymous

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

Changed 8 years ago by greg[at]abbas.org

made slugify template tag use URLify, too

comment:3 Changed 8 years ago by greg[at]abbas.org

good point. :-)

comment:4 Changed 8 years ago by nospam@…

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( ' ', '-' )

Changed 8 years ago by greg[at]abbas.org

fix strip & doctest

comment:5 Changed 8 years ago by greg[at]abbas.org

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 Changed 8 years ago by anonymous

  • Cc gary.wilson@… added

comment:7 Changed 7 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Design decision needed

comment:8 Changed 7 years ago by sime <simon@…>

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 Changed 7 years ago by sime <simon@…>

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 Changed 7 years ago by sime <simon@…>

  • 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 Changed 7 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 7 years ago by sime <simon@…>

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.