Problem/Motivation
Follow up to #2369119: Fatal error when trying to save a View with grouped filters using other than string values.
When adding a grouped filter that uses the \Drupal\views\Plugin\views\filter\InOperator
the generated query contains '0' values for every not selected value.
Steps to reproduce
Steps to reproduce:
- Apply the patch
- Make two content types
- Make a grouped filter for content type
- Add one group 'is one of' for one content type
- Add one group 'is one of' for the second content type
- Save
- Filter using one of these groups and you wil see IN('node_type_1' ,'0', '0') in the query.
Proposed resolution
\Drupal\views\Plugin\views\filter\InOperator::opSimple
doesn't clean the values to remove the empty checkboxes. So either the values need to be cleaned then building the query, or the values need to be cleaned when being set.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Problem/Motivation
Comment | File | Size | Author |
---|---|---|---|
#24 | interdiff_22-24.txt | 4.94 KB | Nitin shrivastava |
#24 | 2857740-24.patch | 4.94 KB | Nitin shrivastava |
#23 | 2857740-nr-bot.txt | 2.97 KB | needs-review-queue-bot |
#22 | 2857740-22.patch | 4.94 KB | Lendude |
#21 | 2857740-nr-bot.txt | 144 bytes | needs-review-queue-bot |
Comments
Comment #2
LendudeComment #3
tacituseu CreditAttribution: tacituseu commentedThey are added in
Drupal\views\Plugin\views\filter\FilterPluginBase::buildGroupSubmit()
, there seems to be a provision for filtering empty values, check:Drupal\views\Plugin\views\filter\FilterPluginBase::hasValidGroupedValue()
:so it could be as simple as using it in
buildGroupSubmit()
.Attached, needs tests...
Comment #5
tacituseu CreditAttribution: tacituseu commentedComment #15
quietone CreditAttribution: quietone at PreviousNext commentedTested on Drupal 9.5.x and this is still applicable. The patch does not apply. I did not review the patch.
Adding tags.
Comment #16
immaculatexavier CreditAttribution: immaculatexavier as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedRerolled patch against #3.
Attached reroll diff
Comment #17
LendudeHere is a test for this. Test only patch is the interdiff
Comment #18
LendudeThe fact that the added code calls
$this->operators()
which is not defined on the abstract class smells a bit, but it is something that is already done at other points in FilterPluginBase so it's not doing anything new.Comment #21
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #22
LendudeTricky reroll since the new test class is no longer new :)
Comment #23
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #24
Nitin shrivastava CreditAttribution: Nitin shrivastava at OpenSense Labs commentedFix Custom command failures.