Remove GroupBy for Find Count queries

[gmercey]

An update for PR #150 as it wasn't clearing groupBy introduced by $modify queries

[gmercey]

I fixed the tests params.modifierFiltersResults=true applies count from modify query and params.modifierFiltersResults=undefined applies count from modify query as the query response for both tests was as follow :

{
  total: 1,
  limit: 2,
  skip: 0,
  data: [
    Company {
      id: 41,
      name: 'Google',
      ceo: 1,
      size: 'medium',
      jsonObject: null,
      jsonArray: null,
      jsonbObject: null,
      jsonbArray: null,
      employees: [Array]
    },
    Company {
      id: 42,
      name: 'Apple',
      ceo: null,
      size: 'large',
      jsonObject: null,
      jsonArray: null,
      jsonbObject: null,
      jsonbArray: null,
      employees: [Array]
    }
  ]
}

Clearing the groupBy for the count query fixes the issue

[dekelev]

Hi @alex-all3dp , can you please review this PR? it may break the behavior you've added here.

@gmercey I'm reverting PR #150 until this PR is resolved.

[alex-all3dp]

@dekelev @gmercey The tests that were changed in this PR are actually the ones I added to ensure backwards compatibility for the previous behaviour, where the total was the count result of the the applied GROUP BY clause. If this changes now, it may be breaking expected behaviour for other users?

Regardless of this, I tested this branch with my setup and it breaks some find queries with modifiers again:

"DBError: select count("id") as "total", "TableName"."id" from "TableName" where (("TableName"."id" = $1)) - column "TableName.id" must appear in the GROUP BY clause or be used in an aggregate function

[gmercey]

@alex-all3dp with which DB Engine do you get the error? Maybe we need to have different behaviour depending on the engine used?

[alex-all3dp]

@alex-all3dp with which DB Engine do you get the error? Maybe we need to have different behaviour depending on the engine used?

@gmercey PostgreSQL

[gmercey]

We could wrap the clear('groupBy) around if (params.modifierFiltersResults !== false) to accommodate? Because the example you are giving is not a proper count query for pagination as it returns a non-aggregated column whereas the count query should return 1 value

[alex-all3dp]

We could wrap the clear('groupBy) around if (params.modifierFiltersResults !== false) to accommodate? Because the example you are giving is not a proper count query for pagination as it returns a non-aggregated column whereas the count query should return 1 value

I guess that could work :)

[gmercey]

@dekelev @alex-all3dp PR updated

[alex-all3dp]

@gmercey The if clause does not fix the issue. My bad, I didn't look closely enough the first time.

So also without the if clause my find queries with modifiers still work as expected. The problem I am having on this branch is that service.get with a.modifier (GET /entity/:id?$modify=myModifier) throws the error message. Any ideas?

[gmercey]

@alex-all3dp is it failing because the service.get is a wrapper for _find :

  _get (id, params = {}) {
    // merge user query with the 'id' to get
    const findQuery = Object.assign({}, { $and: [] }, params.query);
    findQuery.$and.push(this.getIdsQuery(id)); // BUG will fail with composite primary key because table name will be missing

    return this._find(Object.assign({}, params, { query: findQuery }))
      .then(page => {
        const data = page.data || page;

        if (data.length !== 1) {
          throw new errors.NotFound(`No record found for id '${id}'`);
        }

        return data[0];
      });
  }

What we should do in this case, is to pass paginate=false as a param because we don't care about pagination in that case.