Update and move sort criteria

I am working on this issue.

Progress on hold as the new designs should follow claro admin theme components. Designs are under review.

✅ Most installed
Todo: Best Match
✅ A-Z
✅ Z-A

Overall looks good to me. Some nits suggested above.

@chrisfromredfin: Currently no data is shown in case of Best Match sorting. Do you want it the same way or should we show suppose Project Usage data until api is ready?

It is observed during testing that data is not shown correctly in case of A-Z sorting for some of the initial modules due to extra space at the start of module title. I will create another ticket to handle that.

Tests still needs to be written as scope of this tickets proposed solution.

The test can be made more robust once we have the test fixture ready (https://www.drupal.org/project/project_browser/issues/3280721)

Changs looks good to me. One last thing to address is that now direction is obsolete and should be removed from front-end and backend.

Sorting criteria is also changing here https://www.drupal.org/project/project_browser/issues/3280176 as we should no longer tie things up to the mockapi data, because we could and should consider other sources of information.

I think that these two issues are connected and that we should have an agreed "sorting options" that all plugins stick to. That "agreement" is layed out in the MR for that other issue.

Marking this issue as NR so that someone else can review it.
@fjgarlin: Please let me know if something needs to be done as part of this ticket here.
@chrisfromredfin: Best match sort is still not implemented.

@narendraR - I made a few comments in the MR. It's mostly about keeping the `direction` param, as that's pretty common in all api requests (and possible future options) and abstract it in the front-end. Thinks like "a_z" are just "title ASC" so they should be easily translatable.

I don't think that best match sorting needs implementing, you just pass "best_match" to the api and you let the api do the ranking.

I'm planning to have a version of https://www.drupal.org/project/project_browser/issues/3280176 ready for review today.

Reverted commit #80d7b4a7 - Removed unused direction.

The related issue has been fixed and committed to the main branch. So work here can resume.

You might need to do a huge rebase as the mentioned issue changed A LOT of files. It might even be better to start off a fresh branch.

Also, sort options array now lives here: sveltejs/src/constants.js and it is somehow tied to src/Plugin/ProjectBrowserSourceInterface.php. For now that contract is done only via comments but we need to ensure that the changes are done/agreed in the front end and the possible plugins (not just the mock).

Thanks for waiting :-)

Code looks good. The only thing would be to update the comments in src/Plugin/ProjectBrowserSourceInterface.php so other plugins are aware of what they need to implement.

These are what I mean: https://git.drupalcode.org/project/project_browser/-/blob/1.0.x/src/Plug...

Changes look good and the functionality is working as expected too.

However, I do have a question about the processing of the sorting options and where that should live.

The way I see it is that plugins are API's and these usually do not adapt to specific implementations. For example: "a_z" is the way that we interpret "title ASC" in the front-end, and "z_a" is the way that we implement "title DESC", but with these changes we are taking/forcing the whole concept of "a_z" to the plugin, and therefore making it a requirement to be accepted that way.

I think a better approach would be to process that value in the controller, where we actually validate that information first, and then we can "translate" (in the controller) the "a_z" to "title ASC", which is actually more generic. The controller acts like the intermediary between the front-end and the plugins, and it should normally not be bypassed.

I'm happy to adapt the code slightly on the back-end to implement the above suggestion.

Added the logic to the controller instead of the plugin and simplified some comments.

I tested all the changes, but I guess I wouldn't RBTC myself as I did the last commit.

Now that the test fixture landed, this can have automated tests.

Checked the test and all the front-end code. I only did some refactoring in this commit, but didn't alter any other part. So I think this can be marked as RTBC as we now have tests.

Merged, thanks!