Rename Views method add_groupby() to addGroupBy()

Thank you for helping out here.

There is also add_groupby on the search module integration (you missed that).
While we are here we should mark the method as "public"

Working on a patch for renaming on search module integration

@zschmid Please assign this to yourself and update http://core.drupalofficehours.org/node/2711/edit to indicate that you're working on this (add new attempt, set to "Work in progress").

Patch works, methods names changed.

Is there a standard for variable names?

- Submitting patch for search module integration and making groupBy function public. I included the previous changes in the patch as well.

Applied patch successfully and did some cursory unit testing of addGroupby() function.

I'm not sure why the method was originally called add_groupby() instead of add_group_by(), but it should become addGroupBy() now, not addGroupby(). "Groupby" isn't a word (nor is it an SQL keyword).

[accidental duplicate post]

Whoops.

Fixed!

Sorry for the duplicate patch upload - they're both identical.

#11: views-groupby-rename.patch queued for re-testing.

This function is referenced in the following comment in /core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.php

    /**
     * -- removing, this should be taken care of by field adding now.
     * -- leaving commented because I am unsure.
      // If grouping, all items in the order by must also be in the
      // group by clause. Check $table to ensure that this is not a
      // formula.
      if ($this->groupby && $table) {
        $this->add_groupby($as);
      }
     */

The entire comment should be removed as it is pre-views in core.

updated patch removing comment block

considering all the suggestions in comments, #16 views-addGroupBy-2001320-16.patch looks good.

Sorry, no longer applies!

Nice work @zschmid, thank you for the patches:) So this should only need a re-roll.
Instructions here: http://drupal.org/patch/reroll

@digitalpulse I'm un-assigning you for now but if you happen to get around to the re-roll, please assign yourself again.

Attaching a reroll of Comment #16, views-addGroupBy-2001320-16.patch.

Fix to my previous reroll.

#26: views-addGroupBy-2001320-26.patch queued for re-testing.

I am not sure why this is failing.

I had 1 fail after the first test attempt so I added the patch to the queue for re-testing and now I am getting 65 fail(s).

#26: views-addGroupBy-2001320-26.patch queued for re-testing.

#26: views-addGroupBy-2001320-26.patch queued for re-testing.

Re-roll

Patch from #26 is now passing all the test.

Patch from #34 is missing the request from #15 to remove:

 /**
     * -- removing, this should be taken care of by field adding now.
     * -- leaving commented because I am unsure.
      // If grouping, all items in the order by must also be in the
      // group by clause. Check $table to ensure that this is not a
      // formula.
      if ($this->groupby && $table) {
        $this->add_groupby($as);
      }
     */

26!! is rtbc

That's what I thought:) nice work @chrisguindon!

#26 is good to go.

Committed 9ce3e06 and pushed to 8.x. Thanks!