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 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 commentedComment #15
quietone 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 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 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 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 commentedFix Custom command failures.