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.
Hello,
Thanks for this wonderful module !
I was wondering if its possible to configure exposed operators.
Problem:
When we expose operator, all available operators are visible to users. I want only specified operators to be visible to users. For example, for a field_xyz I have following operators available:
- Is one of
- Is all of
- Is none of
- Is empty (NULL)
- Is not empty (NOT NULL)
I only want users to see follwings:
- Is one of
- Is all of
- Is none of
I don't want users to see NULL and NOT NULL operators.
It would be great if we have an option to do so.
Thanks in advance!
Comment | File | Size | Author |
---|---|---|---|
#95 | 1886018-95.patch | 57.65 KB | dagmar |
#95 | interdiff-1886018-88-95.txt | 1.67 KB | dagmar |
#88 | 1886018-88.patch | 57.66 KB | dagmar |
#88 | interdiff-1886018-87-88.txt | 1005 bytes | dagmar |
#87 | 1886018-87.patch | 57.64 KB | dagmar |
Comments
Comment #1
sunildhimal CreditAttribution: sunildhimal commentedIs it possible to incorporate this feature in Views in D8 Core ?
Comment #2
LendudeMoving to the right queue, and +1 on this.
Comment #4
dagmarI worked on this patch long time ago for views #873872: Allow to limit which exposed operators apply
Comment #7
dagmarThis is an starting patch. Maybe someone want to continue during this weekend sprint.
Comment #8
dagmarHere is the patch with tests. I had to modify the
test_filter_in_operator_ui
to include a filter with more operators to play with.Comment #9
abi_cima CreditAttribution: abi_cima as a volunteer and at Globant commentedThis is not grammatically correct. It could be better if it is as follows:
Limit the operators available to choose from to be shown on the list.
The second sentence will sound much better if it is as follows:
Leave empty to show and choose from the complete list.
Grammatically incorrect. It should be 'Tests the limit of the expose operator fonctionality.
Comment #10
dagmarThanks @abi_cima.
This patch includes the corrections proposed in #9 and the modifications to hide the min and max fields when there are not operators selected that use them.
Comment #11
LendudeThis works really nicely.
Patch looks great.
Any way to position the 'limit checkbox' and select list underneath the 'expose operator' checkbox?
Currently it aligns next to it and that makes it unclear to me that these things are tightly related.
Should we add an upgrade path for this? I don't think it's strictly necessary since we provide defaults, but it does make the change cleaner.
Comment #12
dagmarI've been playing with the
views-ui-expose-filter-form.html.twig
template. It seems it could be possible to organize the settings. However at this moment there is a mix of logic between thetemplates
,views_ui
module and theshowOperatorForm
,buildOptionsForm
and other methods fromFilterPluginBase
.Therefore the solution if much more complex that the code this patch provides.
I created #2860990: Move logic to configure exposed form into a template. as a tentative follow-up.
Comment #13
LendudeThe followup sounds like a good idea.
Manually ran through this again and currently the limiting is also done in the 'Add' dialog, not just on the exposed filter.
In the dialog I would always expect to see all operators. And maybe we need a check that when limiting the operators, the default operator is still available?
And tests for this.
Comment #14
dagmarDone.
Done.
Done.
Also I'm hiding the operators list if the user decide to not expose the operator, that was failing before.
Comment #15
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedThe word 'being' is misspelled as 'bein'.
Fixed and attached new patch.
Comment #16
Lendude@dagmar, this looks really nice. Bit of nitpicking (sorry still not happy with the descriptions :):
Set the default to an excluded operator.
Limit the available operators to be shown on the exposed filter.
Only the selected operators will be shown on the exposed filter.
Comment #17
dagmarThanks @Lendude.
Comment #19
dagmarComment #20
LendudeI really like this. Thanks @dagmar
Comment #22
sunildhimal CreditAttribution: sunildhimal as a volunteer commentedComment #23
LendudeUnrelated fail apparently
Comment #24
alexpottI think given we're adding this functionality to a base class I'm not sure we should risk a method. If we inline the function then there is no risk of method name clash.
Let's make this a strict check ie
in_array(,,TRUE)
Comment #25
dagmarChange record: https://www.drupal.org/node/2869168
Added upgrade path and the suggestions by @alexpott.
Comment #27
dagmarComment #29
dagmarComment #30
dagmarHere is the interdiff between #19 and #25 to make easier the review.
Comment #31
LendudeLooks good and addresses all feedback from @alexpott
one nitpick:
copy/paste leftover
Comment #32
dagmarThanks @Lendude
Comment #33
LendudeAll feedback has been addressed, CR has been created, I think this adds a really great feature.
Comment #34
sunildhimal CreditAttribution: sunildhimal as a volunteer commentedThanks for adding this great feature. It works as requested.
Comment #36
LendudeUnrelated fail, https://www.drupal.org/pift-ci-job/649976.
Opened #2870146: Even more random fails in \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest
Comment #38
tacituseu CreditAttribution: tacituseu commentedSame unrelated fail.
Comment #40
Lendudeneeds a reroll for #2863267: Convert web tests of views
Comment #41
dagmarBefore post a re-roll. @Lendude, do we need to put the update in a post_update hook? Can we use hook_update_N instead? We are not using the entity api there, it should be safe... What do you think?
Comment #42
Lendude@dagmar I would always prefer a post_update when possible, no need to reroll when a different hook_update_N gets committed.
Comment #43
dagmarThanks @Lendude. Here is the new patch with the tests updated.
Comment #44
dagmarAnything missing here @Lendude?
Comment #45
Lendude@dagmar thanks for the poke, dropped of my radar.
Looking at this again, we need to take care of existing module config. So follow #2870724: Define and document best practices for configuration entity updates/bc layers. Sorry, should have thought of that earlier. But since the update here is pretty straight forward, it shouldn't be to hard in this case I hope.
Comment #47
borisson_So, this is blocked by the other issue? Postponing this on the other issue.
Comment #48
dagmarNo it just need an upgrade path. I forgot this ticket sorry. Let me see if I can finish the remaining work in the next days/week.
Comment #50
dagmarAdded support to #2870724: Define and document best practices for configuration entity updates/bc layers.
Comment #52
dagmarRe-saved all the views to include the default configs in them.
Comment #53
dagmarThere is one more thing I would like to test. Date provides a custom implementation of the NumericFilter plugin.
@mpdonadio @jhedstrom where do you think we could add this test?
Comment #54
dagmarRerolled after #2949351: Add a helper class to make updating configuration simple
Comment #55
dagmarComment #56
dagmarAdded test coverage for datetime filter.
Comment #57
dagmar#2865344: Exposed date filters 'empty' and 'not empty' are broken includes some test coverage for exposed filters too.
Comment #58
dagmarAnything else missing for this? @Lendude
Comment #60
dagmarComment #61
dagmarFixed the failing tests.
Comment #63
dagmarComment #65
dagmarComment #68
dagmarRe-rolled for 8.8.x. Re-saved all demo_umami views.
Comment #70
dagmarInstalled media_library and re-saved the view.
Comment #71
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedLooks like (at least) the upgrade path was omitted from the reroll on #68
Comment #72
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedSince we are needing an update path, I suppose we'll need an update path test too
Comment #73
dagmar@Manuel Garcia thanks! Indeed some files where out the original patch due some gitignore rules.
Comment #75
dagmarComment #77
dagmarComment #79
dagmarComment #81
dagmarComment #82
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedExcellent we do have an upgrade path test already :D
Changes since #73 look good to me, and we have a green patch again, yay!
Read through the patch, only found a few nits/suggestions:
A bit of a nit, and perhaps it's because I'm new to the patch, but to me reading this comment doesn't make it clear clear why we need to do this, can we explain also the why in the comment? It'll help a lot when/if we need to modify this code in the future :)
For sanity's sake, let's assert that before the updates run, the view does not have the configuration we're adding in the upgrade path.
These should be in separate lines.
We're not defining the
adminUser
property but we're assigning it.Nit: Extra line
Nit:
$this->t('You selected the %operator operator as the default value but is not included on the list of limited operators.', ['%operator' => $selected_operator]);
Comment #83
dagmarThanks for the review @Manuel Garcia!
Comment #85
dagmarComment #87
dagmarComment #88
dagmarComment #89
LendudeGreat work on this, thank you! Went through this again and this looks great.
We have test coverage for:
- the normal functionality
- the min max changes
- the validation
We have an upgrade path for existing sites and new views getting installed and a test for that.
Comment #90
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedThanks @dagmar, latest changes look great. RTBC++
Comment #92
dagmarThe failure is due #3046697: \Drupal\Tests\views\Kernel\Entity\RowEntityRenderersTest::testRevisionBaseTable() can randomly fail once that issue is fixed this can be back to RBTC.
Comment #93
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedThat got in yesterday.
Comment #94
larowlanShould we say 'selecting none will make all of them available' which is a common pattern?
These are strings here, so should we use the third argument?
Comment #95
dagmarComments from @larowlan makes sense to me. Here is the new patch.
Comment #97
dagmarBack to RBTC. Random test failure #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest
Comment #99
dagmarBack to RTBC.
Comment #100
larowlanadding issue credits for reviews
Comment #102
larowlanFixed on commit
Published change record
Committed 09fdae1 and pushed to 8.8.x. Thanks!
Comment #103
DamienMcKennaIf anyone wants to backport this to D7 I'd be happy to consider it for the next Views release *hint* *hint* :-)
Comment #104
dagmarDone as well :) #873872: Allow to limit which exposed operators apply
Comment #105
DamienMcKennaComment #106
DamienMcKennaDuh, yeah, thanks for the reminder.
Comment #108
xjmThis issue is tagged for the 8.8.0 release notes, but does not have a release note.
Additionally, the change record here seems to describe a feature addition, not a disruptive change. If there are disruptive impacts from this issue, the release note and change record should be updated to describe those disruptive impacts.
Otherwise, the issue should be tagged for the 8.8.0 highlights instead. Thanks!
Comment #109
LendudeI would say this is a highlight and not disruptive (if it is disruptive, we messed up that upgrade path ;)
If this needs a snippet too, let me know and I'll make something up.
Comment #111
Wim Leers3 years ago this needed a release note, and it was changed to a highlight, so 👋 bye bye irrelevant tag.