Problem/Motivation
The default sort order is still "Most Popular" even after removing the Mock as the default plugin and switching to the Drupal.org JSON:API as the default. This backend can sort by Relevance as the default, even without a keyword being used in the query. Relevance can then be tweaked based on what we want (i.e. Popularity), which means we then don't need "Most installed" (Most Popular) any more either.
Steps to reproduce
Open PB. Do a search for the module "field permissions" - note that field permissions is in the fifth spot. Change the sort to relevance and you will see something that aligns more closely.
Proposed resolution
- make Relevance the default sort option
- remove the A-Z and Z-A sort options
- remove the Most popular sort option
Issue fork project_browser-3475056
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
utkarsh_33 commentedComment #5
fjgarlin commentedCopied from slack thread: https://drupal.slack.com/archives/C01UHB4QG12/p1729069688461429
@Utkarsh_33 - @fjgarlin while working on https://www.drupal.org/project/project_browser/issues/3475056 issue on which the
testCategoryFilteringtest is failing I came accross an issue with the json API. The api does not return any data (for the test environment) as we have changed the default sort tobest_matchinstead ofusage_total. What is the best way to fix this issue?@fjgarlin - well, the test is never hitting the jsonapi endpoint (by design), so maybe the issue is with the fixtures
https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/tests/mo...
Checking that file, these are all the fixtures sorts that were generated:
There are 50 queries in that file, and only 3 have no sort specified, which is what
best_matchis supposed to be.So, there are actually two things to do in here:
- Change the queries for whatever is queried by PB
- Regenerate the fixture files based on those queries
(slack message was until here)
--
We didn't write any way to re-generate the fixtures as this is something that we've barely need, but it's relatively easy. You can create a PHP file with the same info as
$path_to_fixturein the linked file above. The key of the array is the query to run againsthttps://www.drupal.org/jsonapi + $key, and the value is the name of the file where to dump the contents, sotests/fixtures/drupalorg_jsonapi/ + $valueUntested code:
I also mentioned this in the MR, but if we want to only change the sortOptions for the DrupalorgJsonApi plugin, we should only change the method there, and not in the base plugin, as that would affect every plugin, where those other sort options might make sense.
Comment #6
utkarsh_33 commentedComment #7
fjgarlin commentedI could help with this and build some tooling to make it easier in the future but I won’t be able to get to it in a few days/weeks.
That should be ok since the change is kind of tied to the improvements needed in the endpoint (which I’ll be doing 😅).
But if anyone wants to give it a go I’m happy to review.
Comment #9
narendrarFixture regenerated, added some comments in
DrupalOrgClientMiddlewareso understand api requests. Moving it to NR.Comment #10
fjgarlin commentedI just added some tiny feedback on the MR about a file that might not be related to these changes but it's here. Also, the MR has conflicts, so it might need a rebase.
Last but not least, it'd be great to add some documentation about the fixture regeneration, either in the readme file or somewhere else, so when we need to do it again in the future we don't need to be going back in the git history or old issues.
Setting back to "Needs work" based on all the above.
Comment #11
lavanyatalwar commentedWorking on the suggested changes.
Comment #12
lavanyatalwar commentedWorked upon the Readme.md and deleted unnecessary files.
Comment #13
fjgarlin commentedThe MR still shows conflicts (part of the feedback given on #10) and it's not mergable. It might need a rebase.
The readme additions don't seem to reflect what needs to be done to regenerate the fixtures. I think it might make more sense to revert those changes and give @NarendraR a clean slate to document what he did, as "creating/editing massive json files" is definitely not the way to do this.
Comment #14
lavanyatalwar commentedApologies for the confusion. I'll just revert the changes to ensure it doesn't interrupt your work.
Comment #15
fjgarlin commentedNo worries. Thanks for reverting.
Comment #16
narendrarI tried to explain the fixture generation process in https://git.drupalcode.org/project/project_browser/-/merge_requests/576/diffs#2c53c9f585a22c6bde4e9c11f8b48e8534303873_33_33
Comment #17
fjgarlin commentedI think we should have something in the readme file about the steps you followed to regenerate the fixtures. Did you do it manually? Did you script it? If you script it, which script did you use? This is something that might be needed in the future, and the process should be relatively issue, without going via git history / issue comments.
Comment #18
narendrarComment #19
fjgarlin commentedI moved the mapping of
query => fileto a class constant so it can easily be reused, and I created a helper script to regenerate the fixtures based on my comment in #5.I also documented how to use the script in the readme file.
Comment #20
fjgarlin commentedA new fixture file is introduced in #3485386: Do not hardcode UUIDs in DrupalJsonApi plugin. If that issue is merged first, we will need to update the fixture generating script in here (as I coded to avoid conflicts with other issues).
If this one gets merged first, I'll take care of the rest on the other issue.
Comment #22
chrisfromredfinThis is going in despite the failing tests since those are going to be fixed (flakiness, not true failures that I can tell) in #3485747: The multi-category filter needs to be an actual set of labeled checkboxes