Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Status: Active » Closed (won't fix)

There's a module that will save exposed filters. I forget its name, though, but if you look around for...hmm. saved searched? saved filters? I forget exactly.

slashrsm’s picture

Do you think this module: http://drupal.org/project/views_savefilter ?

I do not if I was clear enough. I am totally happy with default views implementation of exposed filters, when we save selection to $_SESSION. I have problem when I try to use Varnish, since I get session cookie for anonymous users. This makes caching impossible.

I'd like to have ability to save filter values just for registered users. That would allow me to use reverse proxy for anonymous users and to give a bit better experience to registered users.

merlinofchaos’s picture

Category: feature » bug
Status: Closed (won't fix) » Active

OH. Hm.

Indeed those probably should not remember the values for anonymous users in any case. That might actually be considered a bug.

slashrsm’s picture

I agree this should not be default and only behaviour, but could be a nice feature from the other hand, IMO.

I think we should allow user to choose which user roles should have their selection saved and which not. My idea is to show another set of checkboxes in UI when "Save selection" checkbox is selected. This would be a list like this:

Which user roles should have their selection saved?

x Anonymous user
x Authenticated user
x Editor
x Admin
x etc...

Does this sound reasonable?

slashrsm’s picture

Category: bug » feature
Status: Active » Needs work
FileSize
1.94 KB

This is my basic idea how this could work. Logic should be mostly done here, while UI part (form element) should be improved. My idea is to make proposed form a bit more dynamic:

  • We could add another checkbox named "All". If user selects this options it automatically selects all other checkboxes and disables them.
  • When "authenticated user" is selected it selects all user roles and disable it's checkboxes.
slashrsm’s picture

Small fix in options_definition().

loganfsmyth’s picture

Status: Needs work » Needs review

I just ran into this too when setting up Varnish. I like your solution and I don't see anything obviously wrong with it.

Did you have more that you wanted to do, or should it be set 'needs review' ?

tim.plunkett’s picture

Status: Needs review » Needs work
+++ b/handlers/views_handler_filter.incundefined
@@ -71,6 +71,7 @@ class views_handler_filter extends views_handler {
+        'remember_roles' => array('default' => array(DRUPAL_AUTHENTICATED_RID => (string)DRUPAL_AUTHENTICATED_RID)),

Why cast to a string? And if it is necessary, put a space after the (string) cast.

+++ b/handlers/views_handler_filter.incundefined
@@ -336,6 +337,17 @@ class views_handler_filter extends views_handler {
+      '#title' => t('Roles to remember section for'),

selection, not section, and if this can be reworded to not end in a preposition, that'd be great

+++ b/handlers/views_handler_filter.incundefined
@@ -336,6 +337,17 @@ class views_handler_filter extends views_handler {
+        'edit-options-expose-remember' => array(1)

missing trailing comma

+++ b/handlers/views_handler_filter.incundefined
@@ -546,6 +558,15 @@ class views_handler_filter extends views_handler {
+    $allowed_rids = array_values($this->options['expose']['remember_roles']);

This doesn't check if the array is empty first, which will throw notices. In addition, there doesn't seem to be a way to know if this value was never set, or if it's purposefully emptied out.

+++ b/handlers/views_handler_filter.incundefined
@@ -546,6 +558,15 @@ class views_handler_filter extends views_handler {
+    $intersect_rids = array_intersect($allowed_rids, $user_rids);

Wouldn't there be no need for the array_values/array_keys functions if array_intersect_key was used instead?

slashrsm’s picture

Status: Needs work » Needs review
FileSize
2.11 KB

Attached patch is re-roll, as #6 didn't apply any more. I also fixed comments from #8.

I changed Roles to remember section for to User roles. I think this should be explanatory enough. If not, we still have description.

I am now using array_intersect_key(), but array_filter() has to be used before that. It looks like default settings are used, if nothing was configured at all. I changed default settings to mimic current behaviour.

dawehner’s picture

It would be cool if some validation would filter out empty values, so unchecked roles isn't stored. There is options_validate which you can use. Just did that.

paranojik’s picture

The patch from #10 does not allow to save the role selection when all values are emptied out. The patch from #9 is OK. I have only rephrased the description and sent user_roles() through check_plain() (as done in core - block.admin.inc).

I think this is RBTC, but someone else should take another look.

slashrsm’s picture

Looks OK. I'd rather save for authenticated users only by default. That would be in accordance with #3.

Primsi’s picture

Status: Needs review » Reviewed & tested by the community

Looks OK to me.

paranojik’s picture

Rerolled.

dawehner’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

The code looks good now. Thanks for all your work, committed to 7.x-3.x

As things got changed it has to be ported.

slashrsm’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.09 KB

Whoud this work?

dawehner’s picture

Status: Needs review » Needs work
+++ b/handlers/views_handler_filter.incundefined
@@ -387,6 +390,18 @@ class views_handler_filter extends views_handler {
+      '#dependency' =>  array(
+        'edit-options-expose-remember' => array(1),

D8 uses #states now so this part has to be changed.

slashrsm’s picture

Status: Needs work » Needs review
FileSize
2.16 KB

Like this?

slashrsm’s picture

Fixed whitespace.

dawehner’s picture

Status: Needs review » Fixed

Thanks for the port! Committed to 8.x-3.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.