|
Description
Fiona Borthwick
2020-06-09 15:54:07 UTC
Created attachment 105679 [details] [review] Bug 25701: (bug 14419 follow-up) Always display facet groups in the same order It seems that this regression comes from bug 14419, but I have not found a logic reason behind that. This patch restores the behaviour we always had: facet groups must be displayed in a given order: the Zebra index (au, ccode, holdingbranch, etc.) Test plan: Apply this patch, restart all and confirm that the facets are not moving up and down when you refresh your search result page. QA note: I think the following line must be removed @facets_loop = sort {defined $a->{expand} && defined $b->{expand} && $a->{expand} cmp $b->{expand}} @facets_loop; Created attachment 105681 [details] [review] Bug 25701: (bug 14419 follow-up) Always display facet groups in the same order It seems that this regression comes from bug 14419, but I have not found a logic reason behind that. This patch restores the behaviour we always had: facet groups must be displayed in a given order: the Zebra index (au, ccode, holdingbranch, etc.) Test plan: Apply this patch, restart all and confirm that the facets are not moving up and down when you refresh your search result page. QA note: I think the following line must be removed @facets_loop = sort {defined $a->{expand} && defined $b->{expand} && $a->{expand} cmp $b->{expand}} @facets_loop; Signed-off-by: David Nind <david@davidnind.com> (In reply to Jonathan Druart from comment #1) > Created attachment 105679 [details] [review] [review] > Bug 25701: (bug 14419 follow-up) Always display facet groups in the same > order > > It seems that this regression comes from bug 14419, but I have not found > a logic reason behind that. > I think the following line must be removed > @facets_loop = sort {defined $a->{expand} && defined $b->{expand} && > $a->{expand} cmp $b->{expand}} @facets_loop; Afaict the line you change in the patch never worked, we always relied on the secondary sort line you mention above. I removed the 'expand' field in my patches on 14419 set which broke the second (actual) sort I agree we should remove this line and fix the first sort Nick, did you understand why your patches break the sort? Created attachment 105682 [details] [review] Bug 25701: (bug 14419 follow-up) Always display facet groups in the same order It seems that this regression comes from bug 14419, but I have not found a logic reason behind that. This patch restores the behaviour we always had: facet groups must be displayed in a given order: the Zebra index (au, ccode, holdingbranch, etc.) Test plan: Apply this patch, restart all and confirm that the facets are not moving up and down when you refresh your search result page. QA note: I think the following line must be removed @facets_loop = sort {defined $a->{expand} && defined $b->{expand} && $a->{expand} cmp $b->{expand}} @facets_loop; Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 105683 [details] [review] Bug 25701: Remove sort on removed field Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Jonathan Druart from comment #4) > Nick, did you understand why your patches break the sort? Because I removed the 'expand' field - that was the sort that actually sorted. That initial sort did nothing. See bug 3580 Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.01 backported to 19.11.x for 19.11.07 Backported to 19.05.x branch for 19.05.12 |