Add JS/AJAX to append the total count to all issue queue search links automatically

Re: #2: My concern on just using the full URL is that this result counter thing is potentially expensive, and I didn't want us to automatically convert any issue queue link we see. I thought folks should have to go out of their way to someone opt-in to that behavior. If we decide we don't care about the performance implications, I agree that auto-conversion of anything that matches an issue queue link would be easier for everyone to use.

Note that my proposed syntax is already (mostly) a copy/paste of the URL, since the MACHINE_NAME part is already in the issue queue links in question. E.g.:

https://www.drupal.org/project/issues/search/drupal?status%5B0%5D=Open&priorities%5B0%5D=400&version%5B0%5D=any_9.&version%5B1%5D=any_8.&order=last_comment_timestamp&sort=asc

You just copy from "drupal" onward and you're set.

Re: caching -- I'm reluctant to introduce a separate bespoke cache for this. a) More complexity b) Totals would be even more stale. Would rather there was an easy way to refresh all the totals on a page -- if it's just the filter cache, all you have to do is edit the page and it'll re-compute all the referenced totals.

Re: #3: Yeah, I thought so, too. ;) I was searching for prior issues on the topic of "saved searches" but came up blank. There are probably older issues floating around, but I doubt they're talking about this specific implementation that can be embedded anywhere on the site, so I figured it was safe to open a fresh issue about it when inspiration struck late last night...

Re: #4:
A) Interesting idea on trying to do this as a generic, stand-alone thing. However, how would it know what view(s) to use for any given link? If it's generic, then the author has to specify the view as part of the embed code, which seems really clumsy and yucky. If it's something like "parse all links, do a route match to figure out if it's a link pointing to a view, if so make sure the view either shows all results or uses a full pager, if so load the view, set the args, run the query, etc." that seems really complicated and expensive. I was hoping that keeping it project_issue-specific would simplify how it works and how you embed them.

B) The more "real-time" the updating happens, the more expensive this gets. I don't think it can be truly "real-time", or we'd have to invalidate every one of these links on every issue update (which probably happen at least every minute, if not more frequently than that). That'd effectively make it not cached at all, which would probably add a noticeable strain on our infra. I think something like the filter cache is about right -- the totals are computed once, then not updated at all until the page containing the links is edited again. Issue updates would only cause any new queries if the issue itself references issue queue links...

If you can work on this on your sandbox, great! Yes, please.

The JS / AJAX replacement sounds great, but if/how does it degrade?

Maybe #4 could still work? A "Views Summary Link" module that provides the text filter, settings, AJAX callback, JS...

Perhaps admins could define shortcuts for which view a given slug uses, or you can spell out the full machine name if you like/need (replacing the "issue-queue" part of my bogus syntax).

If it's too complicated to do generically, I'm ok with the original project_issue-only proposal.

Yay!
-Derek

Great. Regular link with a class that the JS finds to append the count. Love it.

Thoughts on issue-specific vs. letting this work for any view and the rest of #7?

Thanks!
-Derek

Fantastic! So happy to see this taking shape.

This is now a more accurate title for the proposed feature, right? Updated summary accordingly.

Thanks!!
-Derek

Mostly looks great, thanks! Super excited to see this coming to life!!

1. Any objection to adding this directly to project_issue instead of making it a separate module? I guess someone might not want it, so it's nice to be able to enable it separately. At the very least, how about a submodule of project_issue?

2. +++ b/js/project-issue-count.js
   @@ -0,0 +1,50 @@
   +          && el.getAttribute('href').match(/\/project\/issues\/search\/[a-z0-9_]+\?/)
   

   So we have to have a project machine name in the link? So e.g. links to issue tags that span projects won't work, huh?

   That's because we can't invalidate the cache in any reasonable way for those?

3. +++ b/project_issue_count.info
   @@ -0,0 +1,8 @@
   +name = Project issue counts
   

   Project Issue Counts

4. +++ b/project_issue_count.info
   @@ -0,0 +1,8 @@
   +description = Adds issue counts to project issue search links
   

   Wants a period.

5. +++ b/project_issue_count.module
   @@ -0,0 +1,43 @@
   +function project_issue_count_node_update($node) {
   +  if (project_issue_node_type_is_issue($node->type)) {
   +    // Invalidate the "Issue count" cache for this project.
   +    $pid = $node->field_project[LANGUAGE_NONE][0]['target_id'];
   +    cache_clear_all($pid . ':', 'cache_issue_search_counts', TRUE);
   +  }
   +}
   

   Probably enough of an edge case that we don't care, but issues can move from project to project, and this wouldn't notice that fact to invalidate the count for both the old and new projects...

6. +++ b/project_issue_count.pages.inc
   @@ -0,0 +1,132 @@
   + * Contains functionality for the issue count input filter callback.
   

   Input filter?

7. +++ b/project_issue_count.pages.inc
   @@ -0,0 +1,132 @@
   + * @see \_project_issue_count()
   

   what's \_project_issue_count()?

8. +++ b/project_issue_count.pages.inc
   @@ -0,0 +1,132 @@
   +    $parameters = array_filter($parameters, function ($value, $key) {
   +      if (empty($value)) {
   +        return FALSE;
   +      }
   +      $valid_fields = array(
   +        'participants' => 'string',
   +        'text' => 'string',
   +        'submitted' => 'string',
   +        'assigned' => 'string',
   +        'status' => 'numbers',
   +        'priorities' => 'numbers',
   +        'categories' => 'numbers',
   +        'component' => 'strings',
   +        'version' => 'strings',
   +        'project_issue_followers' => 'strings',
   +        'issue_tags' => 'string',
   +        'issue_tags_op' => 'string',
   +      );
   +      if (!array_key_exists($key, $valid_fields)) {
   +        return FALSE;
   +      }
   +      switch ($valid_fields[$key]) {
   +        case 'string':
   +          return is_string($value);
   +
   +        case 'numbers':
   +          // These are either numbers or 'Open'.
   +          return $value == is_array($value) && array_filter($value, 'is_scalar');
   +
   +        case 'strings':
   +          return $value == is_array($value) && array_filter($value, 'is_string');
   +
   +        default:
   +          return FALSE;
   +      }
   +    }, ARRAY_FILTER_USE_BOTH);
   

   Do we actually need to do all this? Can't we just hand the params to set_exposed_input() and lets Views worry about invalid things?

9. +++ b/project_issue_count.pages.inc
   @@ -0,0 +1,132 @@
   +      return;
   

   continue not return?

Re: #16

Would have liked to use GET so we could cache stuff, but the request could easily be too big.

At first, I was confused by this. "But isn't the view itself relying on GET? How could this be too big?" Then I realized this is a single request for all the links on a page, so yeah. Now I get it. ;)

x-posts, sorry. Re: #17.2: I think your fallback stuff should work well. At most we could have a setting to define which view/display to use for this. But I don't think this needs to be d.o-specific. I'm still in favor of putting it in project_issue (either directly or a sub module).

Couple more thoughts:

1. +++ b/js/project-issue-count.js
   @@ -0,0 +1,50 @@
   +          // Exclude click-sort.
   +          && !el.getAttribute('href').match(/sort=/))
   

   Drat, I guess that means we can't use this for links to searches with specific sort orders. E.g. on the Bug Smash Initiative page, we have "Most stale major and critical bugs (sorted by least recently updated)" and the like. Not sure how to have those work without screwing up click-sort (which we obviously don't want). Can we only exclude /sort/ if we're inside a <th> or something?

2. +++ b/project_issue_count.info
   @@ -0,0 +1,8 @@
   +dependencies[] = project_issue
   
   +++ b/project_issue_count.pages.inc
   @@ -0,0 +1,132 @@
   +    if (!($project = project_load($matches['machine_name'])) || $project->status == 0) {
   

   I notice we're using project_load(). project is a dependency of project_issue already, but maybe we should explicitly depend on project, too? /shrug

Don't get me started on views exposed date filters. ;) I guess this is D7, so maybe it's easier. But here's (part of) the patches section of my composer.json for a real D8 site trying to use them:

        "#2419131-139: [PP-1] #states attribute does not work on #type datetime Fix #states for 'datetime' form elements" : "https://www.drupal.org/files/issues/2019-10-29/2419131-139.datetime-states.8_7_x.patch",
        "#2625136-115: Fix label visibility and add wrapper container for exposed numeric/date filters with multiple form elements Fix label visibility and add wrapper container for exposed numeric/date filters with multiple form elements" : "https://www.drupal.org/files/issues/2019-10-28/2625136-115.8_7_x.patch",
        "#2648950-224: [PP-2] Use form element of type date instead textfield when selecting a date in an exposed filter Use form element of type date instead textfield when selecting a date in an exposed filter" : "https://www.drupal.org/files/issues/2019-10-30/2648950-224.8_7_x.patch",
        "#2648950-224: [PP-2] Use form element of type date instead textfield when selecting a date in an exposed filter Fix conflicts between #2648950 and #2625136" : "https://www.drupal.org/files/issues/2019-10-30/2648950-224.changes-for-2625136.DO-NOT-TEST.patch",
        "#2868014-79: [PP-1] Views Date Filter Datetime Granularity Option Views date filter datetime granularity option" : "https://www.drupal.org/files/issues/2019-10-30/2868014-79.patch",
        "#2868014-79: [PP-1] Views Date Filter Datetime Granularity Option Build upon #2648950-224: [PP-2] Use form element of type date instead textfield when selecting a date in an exposed filter" : "https://www.drupal.org/files/issues/2019-10-30/2868014-79.changes-for-everything-else.DO-NOT-TEST.patch",

Okay, then let's move date filtering to a new issue. That's out of scope here. 😜 If/when we get that working in the regular views UI, it'll either be no change, or a fairly small change, to this code.

Thanks,
-Derek

Looks fantastic, thanks! Did some more local testing, and didn't find any trouble. All feedback addressed. Committed and pushed to project_issue 7.x-2.x branch. Huzzah! 🎉

Moving to RTBC and tagging for d.o deployment (since I no longer have access to do that myself).

Thanks again, this is going to be fabulous when it's live. 😀

This is in @drumm's court now.

Thanks,
-Derek

I guess we could try with execute_display() instead of preview(). I believe that would help.

If we want to really dig into the internals of the Views API, we could try loading up the pager handler, and invoking execute_count_query() directly on that.

Messed with this a bit just now. Couldn't get it working by directly trying to initialize the pager and invoking execute_count_query(). :( But this seems to work, using execute() not preview(). It looks like this isn't actually loading and rendering things. @drumm, is this sufficient?

In other news, I realized none of this code works if you have a site installed in a subdir. ;)

1. +++ b/includes/project_issue.count.inc
   @@ -0,0 +1,131 @@
   +    if (!preg_match('@^\/project\/issues\/search\/(?<machine_name>[a-z0-9_]+)$@', trim($parts['path']), $matches)) {
   

   This regexp assumes Drupal is installed at /

2. +++ b/js/project-issue-count.js
   @@ -0,0 +1,48 @@
   +          && el.getAttribute('href').match(/\/project\/issues\/search\/[a-z0-9_]+\?/)
   

   This one is more friendly to subdirs.

3. +++ b/js/project-issue-count.js
   @@ -0,0 +1,48 @@
   +      fetch(`/project/issues/count`, {
   

   But this won't work, either. Not sure the right way to solve this. Perhaps there needs to be a config value pushed down with the JS for the URL to POST to?

Oh yeah, good point. This patch fixes that.

Meanwhile, oh crap, here's another problem:

https://www.drupal.org/project/issues/count

;)

Whoops. So this patch also moves the callback to live at project/autocomplete/* namespace (like other Project* AJAX callbacks) since "autocomplete" is already a reserved word.

Re: #38 - Good call. Fixed here. Tested locally and all working well.

Any final objections / improvements, or can we deploy this?

Thanks!
-Derek

Re: #51: Thanks, good point! Moved that to #3157596: Remove now redundant counts on 'Contributor links' dashboard block since that block comes from drupalorg_project module. Let's continue that discussion over there.

Cheers,
-Derek