nystudio107 / retour

DEPRECATED Retour allows you to intelligently redirect legacy URLs, so that you don't lose SEO value when rebuilding & restructuring a website.
Other
168 stars 24 forks source link

Remove limit of 100 results in admin UI #76

Closed iainhenderson closed 7 years ago

iainhenderson commented 7 years ago

I recently migrated a site with an insane URL structure which required about 1100 redirects. (Yeah, I know).

I managed to import the rules from a text file, but was concerned when the Retour admin UI reported there were only 100 redirect records.

Looking at the 'craft_retour_static_redirects' table, it looks like all the redirects successfully imported, it's just the admin UI which is only showing 100 results.

jamesmacwhite commented 7 years ago

@iainhenderson I was just looking at this in a site I manage because it has approx 900 redirect rules (yeah, similar reaction, the old URL structure was evil!)

Its actually a configuration option:

https://github.com/nystudio107/retour/blob/56d3773a0a368651c20e6823effd691505709bcb/config.php#L35

So if you place your own config file for retour in the usual Craft location, you can override it.

3noch commented 7 years ago

Just ran into this myself!

iainhenderson commented 7 years ago

Thanks @jamesmacwhite that sorted it.

SimonEast commented 7 years ago

Why would this default to only 100? Seems like a cause for confusion for a number of us.

Just to describe the workaround a little more plainly, you need to create the file craft/config/retour.php containing the following content:

<?php
/**
 * Custom configuration for retour plugin
 */
return [
    // The plugin defaults to only showing a max of 100 redirects. Until they fix it...
    'staticRedirectDisplayLimit' => 5000,
];

However my vote would be to increase the default unless there's some serious performance reason for why this shouldn't be.

jamesmacwhite commented 7 years ago

I guess it's subjective based on the size of site. Some sites may only have <= 100 static redirects setup. For larger sites obviously this default is low, but it can be changed as per the guidance above.

In terms of performance, I have had it to 1000 for about a week on a production site and I see no performance issues or otherwise. The data is already returned paginated and adequately optimised by the looks of things.

I'm sure raising the default value for staticRedirectDisplayLimit wouldn't hurt, but given it can be overridden, it may not necessarily be the "fix". Maybe documentation might be the answer here to highlight the existence of this.

3noch commented 7 years ago

At the very least I think the plugin should show you that it is only displaying the top 100. I learned this after adding several hundred redirects to the database and scratching my head for a while wondering why the GUI did not show them. The GUI doesn't tell you it's limiting the query. Even a simple wording change of "Showing ... of top 100 entries".

jamesmacwhite commented 7 years ago

I agree with that. That's what led me to review the source here to understand where the limit was coming from. It just so happened that this issue was opened only an hour before I stumbled on the same "problem". Having the true total redirects following after the staticRedirectDisplayLimit value would be nice. Probably needs to be conditional based on when total returned rows from the DB is greater than staticRedirectDisplayLimit.

I do think highlighting this limit var in the README wouldn't hurt though.

timprint commented 7 years ago

+1 Would be really helpful if this was documented in the ui. I've just spent an hour wondering what was going on, why can't I find entries that I thought I had added (I needed to amend them). It was only when I noticed the total was exactly 100 that it started to make me wonder if there was a limit.