Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting should detect attributes in service model #171

Open
miguelpeixe opened this issue Oct 26, 2017 · 2 comments
Open

Sorting should detect attributes in service model #171

miguelpeixe opened this issue Oct 26, 2017 · 2 comments

Comments

@miguelpeixe
Copy link

When sorting a sequelize query using query built attributes, we need to use sequelize.col('attribute_name') instead of a string, so sequelize won't prefix with the table name ("table_name"."attribute_name").

getOrder util method should look inside the model for the sorted attributes to choose between using string or sequelize.col method.

Steps to reproduce

Lets say you have users with a hasMany association to votes that you'd like to populate the count through a hook and let the client choose sorting parameters.

const association = {
  attributes: [
    'id',
    'name',
    [
      sequelize.fn('COUNT', sequelize.col('votes.id')),
      'count'
    ]
  ],
  include: [
    {
      model: sequelize.models.votes,
      attributes: [],
      duplicating: false
    }
  ],
  group: ['users.id']
}

Part of the generated query will contain the following selection: COUNT("votes"."id") AS count.

Client sorting by count would look like this:

service.find({
  query: {
    '$sort': {
      count: -1
    }
  }
});

The generated query will ORDER BY "users"."count" DESC which does not exist.

Expected behavior

Detect if sorting attributes are part of the model, using sequelize.col method when they are not.

Actual behavior

Every sort attribute is built to sequelize query as a string.

@miguelpeixe miguelpeixe changed the title Sorting should find attributes in service model Sorting should detect attributes in service model Oct 26, 2017
@maiconmichelon
Copy link

Is there any update about this issue, or is there a workarround to solve this ?
I am trying to order by a literal, and it's building the query as if the literal were a column of the table.

@DaddyWarbucks
Copy link
Contributor

I like this. It is definitely a limitation of the current implementation. Although you can handle this my manually setting the params.sequelize.orderBy, I do think feathers-sequelize should support this OOTB. We will need to test how the $nested.query.syntax$ is affected and work around that. That is currently supported, so we will need to be sure that the update does not break it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants