Make group_by functions use default handler instead of forcing an additional handler to be defined

Or you can pull from https://github.com/boombatower/views/tree/7.x-3.x-groupby.

Edit, forgot to include on patch so one can see:

 handlers/views_handler_field.inc                  |   10 ++++-
 handlers/views_handler_field_group_by_numeric.inc |   43 ---------------------
 plugins/views_plugin_query_default.inc            |    6 +--

I see to get

Fatal error: Call to a member function get_aggregation_info() on a non-object in /home/boombatower/software/views/includes/handlers.inc on line 246

with the combination of #1073378: Add time interval handler.

    if ($this->view->display_handler->use_group_by()) {
      $info = $this->query->get_aggregation_info();
      if (!empty($info[$this->options['group_type']]['method']) && function_exists($info[$this->options['group_type']]['method'])) {
        return $info[$this->options['group_type']]['method']($this->options['group_type'], $field);
      }
    }

Why would $this->query not be set?

We can solve this however everybody likes, but I said as much in the initial post. Yes, there needs to be a way to override and that this code doesn't add it.

Something that I tried to point out multiple times and explained in initial post is that it makes little sense to defaultly force everyone to define an additional class. Yes, I understand that "many" (I still believe most won't since I can envision countless format handlers that have nothing to do with extra fields) will need an additional class, but regardless we already know that not all of them will. So we have two approaches, force 100% to define extra class (or explicitly list themselves as the group by handler [#3]) or default to assuming the same handler thus allowing < 100% to need to deal with either method.

To summarize 100% have to include extra code regardless of whether they need it, or < 100% with a sensibly default. I don't see any argument for why this doesn't make sense, just people dismissing this fact and pointing out that not all of them work with the sensible default which I agree with and thus why it is a default and not only option.

Based on the approach in #3 it would seem one would have to include all the code I moved into the base class in EVERY handler in one form or another. If a single class then it would be in that class, and if two classes (more complex case) then in one of the classes. Again putting the code in the base class allows the complex case to override and everybody else not to have to copy and paste redundant code. Sounds like the entire point of object orientation? If the classes that need the extra class won't want the code that I moved then why would we want to force everyone else to copy-paste it for those that have not additional fields and what not part of all this?

We can use whatever method everyone likes for allowing the override, but again that was not the issue I was trying to solve...and the approach I took does not break the current implementation with the extra group_by class. We should create a separate issue to add the override code mentioned in #3, but they are separate topics.

I would ask the same and not sure what to clarify. Either what I write is not being digested or the responses given are not clear.

Using #3 alone:

Every single handler in existence will have to copy paste the code in views_handler_field_group_by_numeric in order to support group by, even if they list themselves as the group by handler (aka only one classes used for both).

The classes that need the special code like entity handlers and what not will need to write custom code anyway and won't be effected by this since they will override it.

Combination:
So what we need is #3 with the default set to the normal handler and all the code in group_by_numeric classes moved into their respective base classes.

How can I push this along?

Updated original patch against latest. Still need to add #3, but doesn't make anything worse, only improves since those who don't need custom work automatically.

Patch would come in handy.

Of the supported aggregation functions count is an oddball and if you look at the patch I left the numeric override handler on it.

SUM, AVG, MIN, MAX will all keep the same datatype and by definition only make sense when working with numeric data, but if you force the numeric handler then for instance in my usecase a timespan looses its formatting even though it is still a timespan. This will hold true for any numeric data that is formatted.

If I perform any of the operations on miles, time, weight, etc the units have to stay the same and thus using the same handler makes sense.

SUM: x + y (addition has same units)
AVG: (x + y) / n (addition requires same units and n has no units)
MIN: x (simply one element from a set, so no unit change possible)
MAX: x (see above)

COUNT: n (completely bypasses the data type and will never have units so numeric handler makes sense)

Making a note here so I don't forget.
inclues/handlers.inc:256

if ($this->view->display_handler->use_group_by() && $this->query) {

Add the $this->query part since it will not always have a value with patch. Without this you get a fatal error attempting to export a view that is using grouping.

Yea!