fabiocaccamo/django-redirects

Add an order field

ndunn219 opened this issue · 9 comments

ndunn219 commented

It would be great if you could assign an order to the record to indicate the order in which the old paths should be evaluated.

fabiocaccamo commented

@ndunn219 what are you trying to do that doesn't work as expected?

ndunn219 commented

Thanks for the quick reply! Maybe I jumped the gun, as I just started with this, but looking at the code, I see it orders by old_path. So, let's say I have the following two old paths:

/foo.+
/foo/bar

And I want the second one to take precedence. Currently, I don't see a way to do that.

I think this feature can be added without sacrificing backward compatibility by adding this field:

order_of_evaluation = models.IntegerField(
    blank=True,
    default=0,
    verbose_name=_('Order of Evaluation'),
    help_text=_(
        'Determines the relative order in which old paths are evaluated.'))

And changing the ordering to:

ordering = ['order_of_evaluation', 'old_path']

fabiocaccamo commented

@ndunn219 actually are you experiencing the "wrong" behaviour?
If you look at the middleware, the redirects are matched by their type in this order:

  1. Redirect.MATCH_EXACT
  2. Redirect.MATCH_PREFIX
  3. Redirect.MATCH_REGEX

In your case, the second redirect ( /foo/bar ) already has the precedence.

In case the current behaviour doesn't work correctly, please provide a failing test.

  • 👍1
ndunn219 commented

I see. That's probably perfect. Thanks! Might be useful to others to add that to the documentation.

  • ❤️1
ndunn219 commented

OK. I ran into a use case. We have a lot of pages with this URL structure:

/[\w\-]+-training/(.+).cfm

In general, we want those to redirect to:

/catalog/$1/

But there are some exceptions for specific "xyz" strings where we want:

/foo-training/page-name.cfm -> /foo-training/page-name

Because both use RegExs, it always uses the first pattern. An order (or priority) property could fix this.

I think priority might be better:

priority = models.IntegerField(
    blank=True,
    default=0,
    verbose_name=_('Priority'),
    help_text=_('Higher priority redirects are evaluated first.')
)


ordering = ['-priority', 'old_path']
  • 👍1
fabiocaccamo commented

@ndunn219 what do you think about adding priority choices with a range interval [0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0] just like for sitemaps?

fabiocaccamo commented

or even more simply just 4 priority choices:

  • Low
  • Normal
  • High
  • Highest
ndunn219 commented

That would be fine for my use case, but I think an integer is just as clear and provide more flexibility and control.

  • 👍1
fabiocaccamo commented

@ndunn219 pip install django-redirects -U (and run migrations) 💫

  • 👍1