interchange / Dancer-Plugin-Interchange6

Dancer Plugin for Interchange6 shop machine
4 stars 1 forks source link

Removing limit from navigation search? #46

Open murwiz opened 9 years ago

murwiz commented 9 years ago

There doesn't seem to be a way to remove the limit from the navigation search, other than to set the configuration navigation.records to an absurdly large number.

racke commented 9 years ago

What's wrong with that limit? Also, I suppose if wouldn't use a limit if you don't set it at all

SysPete commented 9 years ago

This needs to be changed so that if navigation.records is set to ZERO then we don't use pager since if this is set to undef then default of zero applies instead.

murwiz commented 9 years ago

On 2015-06-02 12:00, Peter Mottram wrote:

This needs to be changed so that if navigation.records is set to ZERO then we don't use pager since if this is set to undef then default of zero applies instead.

Peter, is this something I could do? I have some free time now.

SysPete commented 9 years ago

@murwiz please go ahead. Can you also update pod to explain how this setting works plus add tests?

murwiz commented 9 years ago

I started working on this earlier this month, then got distracted and only now I'm returning to it. My obstacle here is in testing: there doesn't seem to be a programmatic way to change up the {navigation}{records} value inside the D::P::IC6::Routes tables. I've decided to add one, but for now (since it's not in the scope of this change), it'll be a local change only.

SysPete commented 9 years ago

Should be possible like this:

setting('plugins')->{"Interchange6::Routes"}->{navigation}->{records} = 20

On 29/06/15 21:14, Jeff Boes wrote:

I started working on this earlier this month, then got distracted and only now I'm returning to it. My obstacle here is in testing: there doesn't seem to be a programmatic way to change up the {navigation}{records} value inside the D::P::IC6::Routes tables. I've decided to add one, but for now (since it's not in the scope of this change), it'll be a local change only.

— Reply to this email directly or view it on GitHub https://github.com/interchange/Dancer-Plugin-Interchange6/issues/46#issuecomment-116800817.

murwiz commented 9 years ago

On 2015-06-30 04:01, Peter Mottram wrote:

Should be possible like this:

setting('plugins')->{"Interchange6::Routes"}->{navigation}->{records} = 20

Thanks, I'll give that a try. I suspect you can't localize this to a block, though.

SysPete commented 9 years ago

You're right: you'll have to reset at end of block to original setting.

On 30/06/15 14:47, Jeff Boes wrote:

On 2015-06-30 04:01, Peter Mottram wrote:

Should be possible like this:

setting('plugins')->{"Interchange6::Routes"}->{navigation}->{records} = 20

Thanks, I'll give that a try. I suspect you can't localize this to a block, though.

— Reply to this email directly or view it on GitHub https://github.com/interchange/Dancer-Plugin-Interchange6/issues/46#issuecomment-117164401.

murwiz commented 9 years ago

On 2015-06-30 04:01, Peter Mottram wrote:

Should be possible like this:

setting('plugins')->{"Interchange6::Routes"}->{navigation}->{records} = 20

Sadly, this does not work: I had identified why in my earlier try, but I gave yours a shot just in case I was wrong.

The route configuration is set up in _setup_routes, around line 222:

 my $routes_config = _config_routes($plugin_config, 

\%route_defaults);

This hash is then referenced in the routes, e.g. line 351:

 $products =
     $products->rows( $routes_config->{navigation}->{records} )
     ->page( $tokens->{page} );

Normally, this is no big deal, but if you want to change this value at run time, it needs to be stored in some kind of way that's accessible from the outside, or re-calculated from scratch within each route handler based on $plugin_config.

murwiz commented 9 years ago

On 2015-06-30 04:01, Peter Mottram wrote:

Should be possible like this:

setting('plugins')->{"Interchange6::Routes"}->{navigation}->{records} = 20

After studying the code some more, I don't think this is easily solved. All of the work in D::P::IC6::Routes::_setup_routes() would have to be done again after the above assignment, and then again after you change it back to the original setting. Each route uses "$routes_config", which is set at the start of _setup_routes.

I figured out a way to do it, basically replacing $routes_config with a function that re-calculates the configuration if necessary, but I'm really reluctant to introduce that kind of change just to support a testing operation. If there's a need for the configuration to be changed on the fly (I think so, but I'm not the expert on that), then we should probably start a different issue to do so.