pyvideo / richard

video indexing site
Other
216 stars 55 forks source link

increase max_length for title #249

Open willkg opened 10 years ago

willkg commented 10 years ago

I've had a couple of videos before that had long titles and I think we hit another set with SciPy. We should increase the max_length to 100 characters or something.

codersquid commented 10 years ago

For the SciPy videos, I think it is the length of the generated slug, not the title, that is the problem. I'm looking in to it right now.

codersquid commented 10 years ago

Yes, the slug length was the problem. When creating multiple videos with the same title, the unique counter appended to the slug bumps the entire thing over the max_length of 50 chars.

One way to avoid this is to slice out 3 more characters than before, to save room for the slug counter. That's what https://github.com/willkg/richard/commit/4249b8b7226fdf6676e7ed1641452d52edefde7b is doing.

willkg commented 10 years ago

Two things:

  1. we should create a PR so it's easier to comment on the changes
  2. instead of slicing the description and then going through the counter, it's probably better to do both at the same time so we're only slicing off as many characters as we need for the counter
codersquid commented 10 years ago
  1. done
  2. not done yet (but wanted to make a PR to be helpful).
codersquid commented 10 years ago

Hey, I was wondering if you'd rather do something like a query filter where you search for anything where the slug_field starts with the potential slug? If the results are 0, then you know you can use the slug, otherwise, you need to increment the length of the results (unless they are at the sanity check limit you have).

willkg commented 10 years ago

I'm not sure what you mean by that. Can you put that in code?

codersquid commented 10 years ago

Sure, I was playing around with it last night.

def generate_unique_slug(obj, slug_from, slug_field='slug'):
    """ generate slug with a trailing counter to prevent name collision """

    max_length = obj.__class__._meta.get_field(slug_field).max_length
    text = getattr(obj, slug_from)[:max_length-3]
    slug = slugify(text_type(text))

    d = {'{}__startswith'.format(slug_field): slug}
    collisions = len(obj.__class__.objects.filter(**d))

    if collisions == 0:
        return slug

    if collisions > 98:
        # sanity check: prevent new slug if 99 previous slugs already exist.
        raise ValueError('No valid slugs available.')

    return u'{}-{}'.format(text, collisions+1)
codersquid commented 10 years ago

also, when I have error messages sometimes I like to provide information about the invalid input, so in this case I might suggest

raise ValueError('No valid slugs available for: {}.'.format(text))

But the drawback to this kind of error is that it can throw exceptions itself, etc. so sometimes I include that kind of thing in a debug log statement instead.

codersquid commented 10 years ago

let me know if #269 is satisfactory