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

Support ordering on associated models #53

Open
jadhavmanoj opened this issue May 30, 2022 · 2 comments
Open

Support ordering on associated models #53

jadhavmanoj opened this issue May 30, 2022 · 2 comments

Comments

@jadhavmanoj
Copy link
Contributor

jadhavmanoj commented May 30, 2022

As of now, this lib does not support ordering on associated models. But this can be achieved quickly imo.

Proposed solution:

We should just look for the omitPrimaryKeyFromOrder argument, and then add primaryKeyField to the 'orderOption' variable and give that object to the model.paginate function.

We should not parse anything else. paginate parses the order argument and constructs the query.

Below code block is from normalizedOrder function -

const normalizeOrder = (order, primaryKeyField, omitPrimaryKeyFromOrder) => {

We should remove this. ordering parameter for associated model can be nested many level.
ex. ordering = [[{model:user, as: "users"}, "name", "ASC" ]]

 if (Array.isArray(order)) {
    normalized = order.map((o) => {
      if (typeof o === 'string') {
        return [o, 'ASC'];
      }

      if (Array.isArray(o)) {
        const [field, direction] = o;

        return [field, direction || 'ASC'];
      }

      return o;
    });
  }

Let me know if you have better approach to implement this feature.

I tried implementing this approach, worked for me.

@Kaltsoon
Copy link
Owner

Sounds tempting. Order normalization can't be removed (or at least it would be a very breaking change), because Sequelize's order value has many supported formats. I wonder how createCursor and recursivelyGetPaginationQuery would work in this case.

Would it be possible for you to open a pull request of your implementation with a few test cases?

@jadhavmanoj
Copy link
Contributor Author

jadhavmanoj commented May 31, 2022

@Kaltsoon you are right! 😥. the above mentioned functions does not support ordering on associated model.

I am working on the solution. I will create PR once it is ready.

@jadhavmanoj jadhavmanoj mentioned this issue Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants