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

Sorting for "See more Recommended extensions" #7339

Closed
devaneymoz opened this issue Feb 21, 2020 · 16 comments · Fixed by mozilla/addons-server#13592
Closed

Sorting for "See more Recommended extensions" #7339

devaneymoz opened this issue Feb 21, 2020 · 16 comments · Fixed by mozilla/addons-server#13592

Comments

@devaneymoz
Copy link

Describe the problem and steps to reproduce it:

From the AMO homepage click "See more Recommended extensions" text link within its shelf.

What happened?

You're taken to a "search results" type page, with pagination that currently runs to four pages.

What did you expect to happen?

Since the Search results page tells me there are (currently) "97 extensions found" I expect the four pages to list them in some type of consistent order in case I want to peruse the entire list of Recommended extensions. However when you click from page to page you find an inconsistent ordering and often see extensions you already saw on previous pages. User is never certain if they've seen the entire list.

@AlexandraMoga
Copy link

@devaneymoz this issue has been discussed in more detail in #1074 but we marked that as a wontfix.
In any case, I still believe this is a valid issue.

@diox diox transferred this issue from mozilla/addons-server Feb 21, 2020
@bobsilverberg
Copy link
Contributor

@devaneymoz You weighed in on #1074 yourself and agreed with wontfixing it. It seems that the question here is whether we should have a random sort or not. If we keep it random then this "problem" will remain, or we can change it to a different sort sequence. What do you see as a good way forward?

@devaneymoz
Copy link
Author

True I agreed with "wontfix" in the prior discussion but after more usage with this behavior I've changed my mind. Over time I've fielded quite a few requests from folks asking where they can see the entire Recommended list, and when I point them here it's obviously not an ideal overview.

@muffinresearch
Copy link
Contributor

muffinresearch commented Feb 24, 2020

@diox it looks like ES supports this case with seeding see elastic/elasticsearch#1170 - would be good to estimate what that would take to expose via the API.

Thinking aloud (and assuming we add API support), from a UX perspective we can do something like this:

  • If the random search page requested is the first one ignore any stored seed and store a new one for the client for subsequent requests.
  • If the random search page requested is not the first and there's no stored seed, make a search request and store the seed for the client.
  • For pages that aren't the first use the seed from the client if present.

In practice this would mean you'd be able to navigate all random results, but when you return to page one a new random search is performed. It would also maintain a random result set based on starting from somewhere that isn't page one.

A nit/issue would be that if you started from anywhere that wasn't page 1 based on the logic described above, you'd never see the correctly paginated page 1 if it pulls a new set of random data and ignores the seed provided.

Another solution, which was mentioned here which is quite elegant and avoids all the client storage issues by regenerating a random seed on the server at an interval (all users would see the same "random" results in a given time-frame). This would make this change purely server-side.

@muffinresearch muffinresearch changed the title sorting for "See more Recommenddd extensions" Sorting for "See more Recommended extensions" Feb 24, 2020
@bobsilverberg
Copy link
Contributor

Another solution, which was mentioned here which is quite elegant and avoids all the client storage issues by regenerating a random seed on the server at an interval (all users would see the same "random" results in a given time-frame). This would make this change purely server-side.

@muffinresearch It looks like you neglected to add the URL for the "other solution" mentioned above.

@muffinresearch
Copy link
Contributor

Another solution, which was mentioned here which is quite elegant and avoids all the client storage issues by regenerating a random seed on the server at an interval (all users would see the same "random" results in a given time-frame). This would make this change purely server-side.

@muffinresearch It looks like you neglected to add the URL for the "other solution" mentioned above.

Thanks @bobsilverberg that's fixed now.

@bobsilverberg
Copy link
Contributor

Thanks @muffinresearch. It sounds like there is a possibility of solving this on the server, or a combination of the client and server. @diox do you think we should transfer this back to addons-server, or create an additional issue for it there, or shall we just discuss the options here and then figure out where the issue should live once we have decided on a solution?

@diox
Copy link
Member

diox commented Feb 25, 2020

I think providing a seed to the random query is definitely the best approach. We could do it purely on the server side without even needing a special field, just making the seed dependent on specific time periods. We would need to determine what's an acceptable period of time after which changing the seed.

Only drawback of that approach is there would still be an edge case where a client sees the randomized list and then clicks on the link to see all the results (or go from one result page to another) exactly at a time that would coincide with a seed change: they would experience the same issue they are seeing currently. I think that is fine if it doesn't happen too often, so we should factor that in the decision regarding the period of time to use the same seed for.

@diox
Copy link
Member

diox commented Feb 25, 2020

@devaneymoz mentioned on slack we could rotate every 24 hours. So we could force the seed on addons-server to be dependent on the date and we'd be okay without any other changes, it would be transparent to the frontend.

@diox diox transferred this issue from mozilla/addons-frontend Feb 25, 2020
@bobsilverberg
Copy link
Contributor

Just wondering, are we going to implement this for any search with random as its sort sequence, or just for ones that are for recommended add-ons?

@devaneymoz
Copy link
Author

Just wondering, are we going to implement this for any search with random as its sort sequence, or just for ones that are for recommended add-ons?

That's a @jvillalobos call. I was just focused on having a place we can direct people to see a comprehensive list of Recommended extensions. Should probably exist for Recommended themes as well.

@diox
Copy link
Member

diox commented Feb 25, 2020

random should only be available as a sort parameter when searching for recommended add-ons (and no query).

@bobsilverberg
Copy link
Contributor

random should only be available as a sort parameter when searching for recommended add-ons (and no query).

That's not currently the case. I can request search results using random as a sort sequence and not request recommended add-ons only. I'm not sure this is a big issue, and I think it's fine to use this new approach for any search that is sorted by random, but I wanted to raise the question, which is why I did it at #7339.

@diox
Copy link
Member

diox commented Feb 27, 2020

Can you file a new issue for this on addons-server, with the exact API request you're making ? I correctly get this error message when I try:
["The \"sort\" parameter \"random\" can only be specified when the \"featured\" or \"recommended\" parameter is also present, and the \"q\" parameter absent."]

@bobsilverberg
Copy link
Contributor

Can you file a new issue for this on addons-server, with the exact API request you're making ? I correctly get this error message when I try:
["The \"sort\" parameter \"random\" can only be specified when the \"featured\" or \"recommended\" parameter is also present, and the \"q\" parameter absent."]

Ah, you're right, my mistake. I tried changing the URL in addons-frontend, and forgot that we added code to make sure we do not send an API request like that to addons-server, so while I thought it was issuing the API request, it in fact was not, and was removing the random sort param.

@ioanarusiczki
Copy link

@diox I checked on AMO dev the following:

I'm waiting the time to expire and will re-check once again. But so far this looks fixed to me on dev with FF73(Win10).

@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
@KevinMind KevinMind added the repository:addons-server Issue relating to addons-server label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants