Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Given a view that has two sorts: 1. A non-exposed sort on field foo that sorts DESC. 2. An exposed sort on field bar that sorts ASC by default.
The expected default sort (i.e. without any query arguments in the request) would be: foo DESC, bar ASC
The actual default sort is: foo DESC, bar DESC
In other words, the sort order of the exposed sort is being set incorrectly, in fact it is being taken from the non-exposed sort (thus to expose this bug the sort orders must be different).
The problem is that \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase::exposedFormAlter() uses the configured sort order of the first sort plugin as the default sort order, when it should be using the configured sort order of the first exposed sort plugin.
Proposed resolution
Fix \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase::exposedFormAlter() to use the correct sort order.
Remaining tasks
Update the patch
Write tests.
Review patch.
User interface changes
None.
API changes
None.
Data model changes
None. This does not change the Views configuration, it only has an effect during runtime.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | 2737395-26.patch | 8.46 KB | vsujeetkumar |
| #23 | interdiff_20-23.txt | 4.08 KB | sahil.goyal |
| #23 | 2737395-23.patch | 8.35 KB | sahil.goyal |
| #20 | 2737395-20.patch | 4.53 KB | ameymudras |
Issue fork drupal-2737395
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:
compare
Comments
Comment #2
tstoecklerHere we go. This fixes the issue for me. Needs a test of course.
Comment #3
tstoecklerHere's a test. If we had a nicely decoupled test suite I would have added a dedicated test method for this, but there is only a single test class for all of the views exposed form stuff, so adding this little bit seemed fair in this case.
Also the test view was not up to snuff anymore (as clearly visible by the dependency on node), so updating that. I added the additional sort in the UI and then exported.
Comment #10
kducharm CreditAttribution: kducharm at CivicActions commentedRe-roll of patch for 8.5.x, confirmed the patch does set the order correctly but I'm not very familiar with the test so someone else should probably review that.
Comment #19
quietone CreditAttribution: quietone at PreviousNext commentedI tested this on Drupal 10.1.x, umami install and was able to reproduce the problem. I then copied the few lines of the patch to ExposedFormPluginBase and retested. The changes suggested fix the problem in the Issue Summary,
Comment #20
ameymudras CreditAttribution: ameymudras at Salsa Digital commentedRe rolled to 10.1.x
Comment #23
sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedtrying to resolve the error and uploading interdiff.
Comment #25
vsujeetkumar CreditAttribution: vsujeetkumar at Srijan | A Material+ Company for Drupal India Association commentedNeed re-roll againts 11.x.
Comment #26
vsujeetkumar CreditAttribution: vsujeetkumar at Srijan | A Material+ Company for Drupal India Association commentedRe-roll patch created, Keeps as "Needs Work", having some fail tests.