Opened 13 years ago

Last modified 13 years ago

#36 closed defect (FIXED)

Need a way to generate slugs for media entries, etc

Reported by: Christopher Allan Webber Owned by: Aaron Williamson
Priority: minor Milestone: 0.0.2
Component: Keywords:
Cc: Parent Tickets:

Description

We need to be able to generate URL slugs for media entries.

I'd really like to use
`http://flask.pocoo.org/snippets/5/ <http://flask.pocoo.org/snippets/5/>`_
but have to clear it with the FSF that that public domain assertion
is okay for our codebase and to merge.

Regardless, we need to generate slugs for each entry anyway.
Probably during the submission phase after setting the title there
should be an entry.generate\_slug() method which sets the slug on
the MediaEntry instance.

This way we have a nice way of accessing for example the fictional
address:

[http://mediagoblin.example.org/\ :sub:`cwebber/w/a-walk-in-the-park/](http://mediagoblin.example.org/`\ cwebber/w/a-walk-in-the-park/)

that last bit of the URL being the entry slug.



Attachments (5)

0001-Patch-for-Feature-306.patch (3.7 KB ) - added by Aaron Williamson 13 years ago.
0001-Patch-for-Feature-306.patch
slug-patch.txt (4.1 KB ) - added by Aaron Williamson 13 years ago.
slug-patch.txt
0001-Patch-to-create-slugs-when-new-images-are-added.patch (3.7 KB ) - added by Aaron Williamson 13 years ago.
0001-Patch-to-create-slugs-when-new-images-are-added.patch
0002-Add-id-to-non-unique-slugs-add-unit-test-for-unicode.patch (3.0 KB ) - added by Aaron Williamson 13 years ago.
0002-Add-id-to-non-unique-slugs-add-unit-test-for-unicode.patch
0003-Added-one-more-unicode-test-for-slugify.patch (893 bytes ) - added by Aaron Williamson 13 years ago.
0003-Added-one-more-unicode-test-for-slugify.patch

Download all attachments as: .zip

Change History (16)

comment:1 by Christopher Allan Webber, 13 years ago

Brett Smith says this is fine, we can incorporate :)

If someone has time, this would be a great bitesized task.


-  add necessary dependencies to setup.py if necessary, run
   ./bin/buildout should pull them in if using such a setup
-  put the snippet'ed utility into mediagoblin/util.py
-  add some unit tests in mediagoblin/tests/test\_util.py
-  add a MediaEntry.generate\_slug() method which should generate a
   slug and assign that slug to itself (self['slug'])
-  call MediaEntry.generate\_slug() after title is assigned in the
   submit process and before save



by Aaron Williamson, 13 years ago

0001-Patch-for-Feature-306.patch

comment:2 by Aaron Williamson, 13 years ago

Made an effort at plucking this low-hanging fruit as described
above. One concern though: if a user names several images "A walk
in the park" (which seems quite possible) they will all have
identical slugs. Should we account for this with some sort of
iterator or other device?



comment:3 by Christopher Allan Webber, 13 years ago

Owner: set to Aaron Williamson
Great, Aaron!

I think the best thing to do if such a conflict occurs is maybe to
either not generate a slug (hmmm) or tack a uuid onto the name
(hmmmmm)?



comment:4 by Aaron Williamson, 13 years ago

One way I've solved this in the past is to query the database for
items with identical slugs submitted by the same user. If there is
one, add "-1" (or "-2", etc.) to the end of the slug. However, this
assumes you only want slugs to be unique to users. If you want to
use slugs in other contexts (e.g. sets of photos) this might be an
inadequate solution. It also is probably slower than using a UUID,
because it requires a database hit. (Hmmmmm?)



comment:5 by Christopher Allan Webber, 13 years ago

Yeah I think numbered slugs are just simply impossible with
mongodb.

Aaron, what do you think of tacking on the entry id of the
MediaEntry if there's a conflict? Conflicts will probably be rare
and it probably won't look **too** bad. Prepending it like
:math:`${id}-$`{slug} probably works. Or the reverse. I think I
like the former though. Thoughts? More useful than a uuid.



comment:6 by Aaron Williamson, 13 years ago

Yeah, that's a good idea. Or possibly even :math:`${id}/$`{slug}?
Or would that break url patterns?



comment:7 by Christopher Allan Webber, 13 years ago

It might break the urlpatterns... I'd much rather use a dash.

Here's some pedantic styling comments, thought this looks like a
great start:

::

            'slug':unicode,

do:

::

            'slug': unicode,
    
    
    
        def generate_slug(self):
            self['slug'] = util.slugify(self['title'])

This looks good. Could you add a docstring? It's probably a good
idea that you didn't .save() here, good call.

You might want to add a second argument to util.slugify which is
self['\_id'].

Also, your unit test could really use some examples of non-ascii
characters, for example on the snippet linked above there is the
"u" to "ue".

Aside from this and the conflict resolution we've been discussing,
this patch looks great, like everything I hoped! Make that id
addition change and we can merge this.



comment:8 by Christopher Allan Webber, 13 years ago

Milestone: 0.0.2

by Aaron Williamson, 13 years ago

Attachment: slug-patch.txt added

slug-patch.txt

comment:8 by Aaron Williamson, 13 years ago

Addressed all of Chris's concerns. Decided to do the dupe-checking
in generate\_slug instead of slugify. Added a couple of tests for
unicode characters.



comment:9 by Christopher Allan Webber, 13 years ago

Could you use git format-patch? If you do that it'll keep
information about this commit being done by you and etc. Looks good
though.



by Aaron Williamson, 13 years ago

0003-Added-one-more-unicode-test-for-slugify.patch

comment:10 by Aaron Williamson, 13 years ago

Sure, sorry -- I'm not all that familiar with format-patch and I
was trying to get all of this to you in a single patch file, but I
guess that's unnecessary. Here are all of the applicable patches.



comment:11 by Christopher Allan Webber, 13 years ago

Status: NewClosed
Nice job, Aaron! Merged and pushed. :)



comment:12 by Will Kahn-Greene, 12 years ago

The original url for this bug was http://bugs.foocorp.net/issues/306 .

Note: See TracTickets for help on using tickets.