All core entity lists should be filterable. A typical use case is the one introduced in #539110-61: TF #4: Translatable fields UI: per-language comment filtering.
We should try to generalize this ability.

CommentFileSizeAuthor
#6 filterable_lists-634262-6.patch22.85 KBplach
#7 filterable_lists-634262-7.patch23.02 KBplach
#3 filterable_lists-634262-3.patch22.85 KBplach
#1 filterable_lists-634262-1.patch23.4 KBplach

Comments

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Active » Needs review
StatusFileSize
new filterable_lists-634262-1.patch23.4 KB

The current patch adds to each relevant query a "{$entity_type}_filter" tag and a context metadata. It also DBTNGs Aggregator's relevant queries.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Issue tags: +translatable fields
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
StatusFileSize
new filterable_lists-634262-3.patch22.85 KB

fixed newlines

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

So who needs Views in core? :P Does Views even support dynamically altered views? ;)

We need to ensure that the added query tags and meta data makes sense. For example, I spot the 'node_filter' context "feed" in there, but for Blog module's feed.

In addition, we want to prefix "context".... i.e. "node_filter_context". And shouldn't that be an array?

I.e. the query tag for a potentially-to-be-filtered blog query has the contexts: 'node', 'blog', 'feed'.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

So who needs Views in core? :P Does Views even support dynamically altered views? ;)

While rolling this one I've seen things you people wouldn't believe :)
Views In Core must be the next Big Thing.

Agreed on the rest, I'll reroll the patch asap.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
StatusFileSize
new filterable_lists-634262-6.patch22.85 KB

I implemented contexts as arrays, however, since I thought they might be shared between tags as they describe the environment the query is run in, I did not prefix the 'context' key.

ATM the context array has the following structure:

<?php
$context = array(
  'module name',    // required
  'context',        // optional
  // ...            // optional subcontexts
);
?>

I did not introduce a specific 'entity type' context key as the "entity" type is provided by the tag itself, and in some cases it might clash with the module name (e.g. node).

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
StatusFileSize
new filterable_lists-634262-7.patch23.02 KB

I forgot to add context metadata to comment queries.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

After reading your follow-up in #6, I somehow expected a keyed array now, i.e.

$context = array(
  'module' => 'blog',
  'build_mode' => 'feed',
  // ...
);

...but it's an indexed one. Would that make sense?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

I somehow expected a keyed array now [...]. Would that make sense?

I was tempted to use a keyed array, but I could not find an appropriate common key for most of the values currently held by the context arrays. Perhaps every query should define its own?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

plach asked me to look at this. No offense, but this looks like an absolute nightmare. :( Humans would have to provide information that could easily be programmatically derived via debug_backtrace() or similar on *every* single "query that lists stuff," which (as you can see by the size of the patch) is A LOT of queries (and far more in contrib).

This API change is way too massive at this stage, at any rate, and talking to plach he doesn't need anything this fancy; just a tag identifier for comment so that TF UI could work from contrib. Let's scale this back.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs review » Closed (won't fix)

Totally agreed, just a big misunderstanding :)

@sun: please move to #641034: Make comment lists filterable.