Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort by button in search results does not work #9392

Open
stephanegigandet opened this issue Nov 23, 2023 · 7 comments · May be fixed by #10024
Open

Sort by button in search results does not work #9392

stephanegigandet opened this issue Nov 23, 2023 · 7 comments · May be fixed by #10024

Comments

@stephanegigandet
Copy link
Contributor

stephanegigandet commented Nov 23, 2023

What

  • After searching products in the search box, the sort by dropdown does not work:
    • Results are not sorted
    • There is a blank value in the dropdown

Resulting url has 2 sort_by fields, with the last one with a ? instead of & :

http://world.openfoodfacts.localhost/cgi/search.pl?action=process&search_terms=coca&sort_by=unique_scans_n&page_size=24?sort_by=last_modified_t

image

Reported by Olaf Görlitz on Slack:
https://openfoodfacts.slack.com/archives/CUZK65DGE/p1700749241876539 (includes video)

Part of

@teolemon teolemon added 🐛 bug This is a bug, not a feature request. 🔎 Search 🎯 P1 labels Nov 23, 2023
@himanshisrestha
Copy link
Contributor

Can I get to know more about this issue

@anaritadauane
Copy link
Contributor

@himanshisrestha Have you looked at the video?

@himanshisrestha
Copy link
Contributor

Hi @anaritadauane , yes I've looked at the video. Thank you

@himanshisrestha
Copy link
Contributor

himanshisrestha commented Mar 23, 2024

@stephanegigandet , I've refered to the code and found that the related code is in Display.pm around here:
`if ((defined $options{product_type}) and ($options{product_type} eq "food")) {

	push @{$template_data_ref->{sort_options}},
		{
		value => "popularity",
		link => $request_ref->{current_link} . "?sort_by=popularity",
		name => lang("sort_by_popularity")
		};
	push @{$template_data_ref->{sort_options}},`

and this causes the url to contain the '?' instead of '&'
Could you give me some more idea about this to proceed on it

@stephanegigandet
Copy link
Contributor Author

@himanshisrestha Instead of generating an url like https://world.openfoodfacts.org/cgi/search.pl?action=process&search_terms=test&sort_by=unique_scans_n&page_size=24?sort_by=created_t we want to generate an URL like https://world.openfoodfacts.org/cgi/search.pl?action=process&search_terms=test&sort_by=created_t&page_size=24 when the sort button is used on the search results page.

Of course for other pages like the home page, we still want to have URL that work with the search button

@himanshisrestha
Copy link
Contributor

Screenshot 2024-03-24 at 8 41 31 PM Screenshot 2024-03-24 at 8 41 34 PM @stephanegigandet I we changed the code here and in effect - the url after a particular sort option is click turned from https://world.openfoodfacts.org/?sort_by=nutriscore_score to http://world.openfoodfacts.localhost/&sort_by=nutriscore_score&page_size=100 - the url in a particular searched product (eg. sugar ) when a particular sort option is turned on changed from https://world.openfoodfacts.org/cgi/search.pl?action=process&search_terms=sugar&sort_by=unique_scans_n&page_size=24?sort_by=nutriscore_score to http://world.openfoodfacts.localhost/cgi/search.pl?action=process&search_terms=sugar&sort_by=nutriscore_score&page_size=24

Could you review this changes and is it that I should make some changes in the test script files (.t) for this ?

@alexgarel
Copy link
Member

@himanshisrestha I think your change is fine, also did you verify it does not break

This way of adding parameter is in my opinion not quite good (we should instead collect parameters in a dict and at the end create the corresponding url parameters) but it would be maybe beyond the scope of this fix.

Maybe adding a method in Web.pm like add_params($url, $params) where you verify if you already have a '?' to decide wether to concatenate $params with a '?' or a '&' would be great and easy to add. (and we could use it ubiquitously)

Regarding tests you could complete current integration test to verify if the links to sort the page as expected are present in page HTML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment