I'd like to create an exposed Views filter that allows:

* Show me all nodes
* Show me only nodes I've flagged
* Show me only nodes I've not flagged

It doesn't appear, however, that I cover the "not flagged" portions.

My flag relationship leaves UNCHECKED "Include only flagged content".

Creating a filter for "Flagged" and "Single filter" only gives me "is/is not equal to" "flagged/unflagged", and the resultant UI always defaults to "True" (as opposed to "Any"). Setting "True" or "False" in the filter doesn't seem to change the results - the results are always flagged nodes. Creating a grouped filter, with the "All" option as the default starts the View out with the right results ("all"), but no configuration I've tried ("is FALSE", "is not TRUE", etc.) let's me handle "unflagged" nodes.

CommentFileSizeAuthor
#5 2841067-05.patch593 bytesjhedstrom

Comments

Morbus Iff created an issue. See original summary.

morbus iff’s picture

morbus iff
they/them
commented
Related issues: +#2813503: [regression] restore 'All' option in flag Views filter handler
joachim’s picture

joachim commented
Priority: Normal » Major
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

I dug into this a little today. This code in FlagViewsFilter:

  /**
   * {@inheritdoc}
   */
  public function query() {
    $this->ensureMyTable();

    $operator = $this->options['value'] ? 'IS NOT' : 'IS';
    $operator .= ' NULL';

    $this->query->addWhere($this->options['group'], "$this->tableAlias.uid", NULL, $operator);
  }

is getting the non-exposed option from the filter config (eg, Flagged or Unflagged), and not reading in the submitted value. I'm not sure how to get at that from the query method...

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new 2841067-05.patch593 bytes

Ah, this is how :)

From FilterPluginBase:

  /**
   * Contains the actual value of the field,either configured in the views ui
   * or entered in the exposed filters.
   */
  public $value = NULL;
morbus iff’s picture

morbus iff
they/them
commented
Status: Needs review » Needs work

This patch worked for my needs, but didn't meet every scenario I tried.

The following DID work for me:

  • Single filter (not exposed) "is equal to" "Flagged" (showed flagged items)
  • Single filter (not exposed) "is equal to" "Not flagged" (showed unflagged items).
  • Single filter (exposed, required) user-set to True (showed flagged items)
  • Single filter (exposed, required) user-set to False (showed unflagged items)
  • Single filter (exposed, not required) user-set to True (showed flagged items)
  • Single filter (exposed, not required) user-set to False (showed unflagged items)
  • Single filter (exposed, not required) user-set to -All- (showed all items)
  • Grouped filter (exposed, optional) with "Any" and TRUE/FALSE states.

The following DID NOT work for me - it appears the "operator" (as opposed to the "status") is being ignored. With that said, both of these options can be accomplished with the current "is equal" operator, so this is probably not a huge enough worry to stop this patch from going in.

  • Single filter (not exposed) "is not equal to" "Flagged" (showed flagged items)
  • Single filter (not exposed) "is not equal to" "Not flagged" (showed unflagged items).
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

The operator bit sounds like a separate issue, although it probably is a similar one-liner fix in the same method, so we could tackle that here too...

morbus iff’s picture

morbus iff
they/them
commented

Given that our query() is doing IS or IS NOT NULL, I'm not entirely sure the "is equal" or "is not equal" operators are valid. We might, instead, consider removing the operators option entirely. I still remain inept at Drupal 8, but there does appear to be a $no_operator tweak for filter values...

morbus iff’s picture

morbus iff
they/them
commented
Status: Needs work » Reviewed & tested by the community

Let's get this first patch in, and we'll worry about "is not equal" later.

socketwench’s picture

socketwench commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

I've been trying to replicate the results in #6, but it's just not working for me. Here's what I did:

  1. Fresh Drupal install, enabled flag, flag_bookmark, flag_follower.
  2. Used drupal create:nodes to make 25 article nodes.
  3. Flagged one node.
  4. Created a content view, added the flag bookmark relationship, and an exposed flag/no flagged filter.

The flagged node appears when the filter is set to flagged, but not flagged doesn't work at all.

Furthermore, we really should add some tests so this doesn't slip under the radar again.

joachim’s picture

joachim commented

Confirming that the filter doesn't show unflagged nodes: it still shows flagged content.

The query has a clause:

(flagging_node_field_data.uid IS NOT NULL)

joachim’s picture

joachim commented

> Given that our query() is doing IS or IS NOT NULL, I'm not entirely sure the "is equal" or "is not equal" operators are valid. We might, instead, consider removing the operators option entirely. I still remain inept at Drupal 8, but there does appear to be a $no_operator tweak for filter values...

Filed #2860521: disable operators in Flag views filter to look at this. Good catch!

joachim’s picture

joachim commented

Patch fixes the problem for me with an exposed filter. #6 has extensive testing of other scenarios -- and I think we can ignore the cases using the 'not equal' option, as it doesn't make sense here -- so I think it's ready to go apart from tests.

NodeFieldFilterTest would be a good candidate for something to ape from.

madar’s picture

madar commented

Patch works for me.

joachim’s picture

joachim commented

Regarding tests, I am starting to think that we should commit this, and then add comprehensive Views tests in a separate issue. Looking at node module's views tests, there's a lot to do.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
Issue tags: -Needs tests

then add comprehensive Views tests in a separate issue

I added #2864499: Add comprehensive Views integration tests.

FWIW, I've been using this patch now for months, with the message_subscribe contrib module that utilizes this functionality, and it seems to work as expected.

  • joachim committed 233706c on 8.x-4.x authored by jhedstrom 
    Issue #2841067 by jhedstrom: Fixed Views exposed flag filter can't show...
joachim’s picture

joachim commented
Status: Needs review » Fixed
Related issues: +#2860521: disable operators in Flag views filter, +#2864499: Add comprehensive Views integration tests

This was RTBC earlier, so I'm committing it.

Added the various follow-ups as related issues.

Thanks everyone!

Status: Fixed » Closed (fixed)

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

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

I just opened #2993178: Exposed filter cannot be configured to be <Any> by default for a related issue. We are trying to build the "My favorite sessions" exposed filter for Drupal Europe daily schedule :)

ressinel’s picture

ressinel
Location 🇺🇦 Rivne
commented

This patch does not work for the case "Show me only groups I've not flagged" when we use flag for Group entity type.
Does anyone know about this problem?

Was created a related issue for Group module: https://www.drupal.org/project/group/issues/3219902

ressinel’s picture

ressinel
Location 🇺🇦 Rivne
commented