fix: loosing the second facet at the end of the url on redirection #8307
Conversation
tests/unit/display.t
Outdated
| @@ -150,4 +151,59 @@ $product_ref = { | |||
| $expected = lang('to_do_status') . separator_before_colon($lc) . q{:}; | |||
| like(display_field($product_ref, 'states'), qr/$expected/); | |||
|
|
|||
| # should not loose the second facet at the end of the url on redirection | |||
| my $facets_ref = { | |||
| 'current_link' => 'category/en:lemonades/data-quality', | |||
There was a problem hiding this comment.
current_link will be set by display_tag(), so the value you set here will be overwritten, we can remove it.
we should add a tagtype => "categories" though, otherwise you get a //lemonade/data-quality link, instead of /category/lemonade/data-quality
Can you use "bread" instead of "lemonades"? There's an issue in the categories taxonomy, the canonical name should be in plural (lemonades), but currently it's in singular.
tests/unit/display.t
Outdated
|
|
||
| display_tag($facets_ref); | ||
|
|
||
| unlike($facets_ref->{'current_link'}, qr/en:lemonades\/data\-quality/); # verify that link has been processed |
There was a problem hiding this comment.
instead of like() , unlike(), I think it's best to use is() in order to check the exact value and verify it's the one we expect
e.g. is($facets_ref->{'current_link'}, '/category/bread/data-quality')
| @@ -3060,6 +3060,7 @@ sub display_tag ($request_ref) { | |||
| } | |||
|
|
|||
| if (defined $request_ref->{groupby_tagtype}) { | |||
| $request_ref->{current_link} .= "/" . $tag_type_plural{$request_ref->{groupby_tagtype}}{$lc}; | |||
|
Hi @jnsereko , thanks a lot for adding a test with the mock functions. Your fix looks good, and for the test I made some suggestions. Thanks! |
|
Kudos, SonarCloud Quality Gate passed!
|
|
Please retry analysis of this Pull-Request directly on SonarCloud. |
What
I have appended
$request_ref->{groupby_tagtype}to the$request_ref->{current_link}so that there is no truncation in redirection.I have also added tests to verify that,
cc @yuktea @alexgarel @stephanegigandet
Screenshot
redirect.new.mov
Related issue(s) and discussion