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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

query.sort(null) should reset the sort options #14365

Closed
caub opened this issue Feb 20, 2024 · 8 comments
Closed

query.sort(null) should reset the sort options #14365

caub opened this issue Feb 20, 2024 · 8 comments
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class
Milestone

Comments

@caub
Copy link
Contributor

caub commented Feb 20, 2024

馃殌 Feature Proposal

Calling .sort(null) should allow to unset whatever was set as a sort previously (in query.options.sort)

Motivation

Sometimes it's useful to override default behaviors from plugins, later in the query chain

Similar to #14180

Example

q=I.find().sort({ startDate: -1, _id: -1 });

q.options.sort // { startDate: -1, _id: -1 }

q.sort(null); // reset sort (doesn't work currently)
q.sort({ _id: -1 }); // this doesn't overwrite a previous non-null sort

q.options.sort // now is { _id: -1 }, but in current version it still returns { startDate: -1, _id: -1 }

I'm proposing .sort(null), but I've tried using .setOptions(null) or .setOptions({}) or .setOptions({sort: {_id: -1}}) without success. Any way would be fine

@caub caub added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class labels Feb 20, 2024
@caub
Copy link
Contributor Author

caub commented Feb 22, 2024

@vkarpov15 sorry for pinging, but maybe I'm missing something here? I'd like to be able to reset a query's sort so I can set something else on the same query instance

@FaizBShah
Copy link
Contributor

FaizBShah commented Feb 23, 2024

@caub

To change sort options, you can try q.setOptions({}), and to check whether it has changed or not, you can check it by doing console.log(q.getOptions())

For example:

const query = User.find().sort({ age: 1 });
query.setOptions({ sort: { age: -1 } });
console.log(query.getOptions()); // Printing { sort: { age: -1 } }

Just FYI, this is for mongoose@8.1.3

@caub
Copy link
Contributor Author

caub commented Feb 23, 2024

@FaizBShah Doesn't work, for example with a sort with 2 fields, and a reset, the previous fields can't be unset

https://replit.com/@caub/mongoose-sort-up#index.js

const mongoose = require('mongoose'); // 8.2.0

const testSchema = new mongoose.Schema({
  name: String,
});

const Test = mongoose.model('Test', testSchema);

const q = Test.find().select('name').sort({ name: -1, _id: -1 });

console.log(q.getOptions());

// q.setOptions({});
q.setOptions({sort: {_id: 1}});

console.log(q.getOptions());
{ sort: { name: -1, _id: -1 } }
{ sort: { name: -1, _id: 1 } }

@probro27
Copy link

@caub @FaizBShah Hey guys, I am trying to work on some open source issues and I came across this one. I understand the use case you guys want to implement, and I think it can be supported by modifying this function here:

In case you agree on how and if you are going to implement it, please reach out to me and I would love to take this up. Thanks :)

@FaizBShah
Copy link
Contributor

FaizBShah commented Feb 24, 2024

@caub I think I found the solution. To override the current options object, you just need to pass the 2nd parameter (which stands for override) in setOptions() as true.

const query = User.find().sort({ grade: 1, age: 1 });
console.log(query.getOptions()); // Prints { sort: { grade: 1, age: 1 } }

query.setOptions({ sort: { age: -1 } }, true);
console.log(query.getOptions()); // Prints { sort: { age: -1 } }

I hope this helps you!!

@FaizBShah
Copy link
Contributor

@vkarpov15 Currently there is no mention of the override parameter for setOptions() in the doc. Also, there is no mention of what the params do when Im typing the function name in the project, as it only displays this text.

Can I create a PR to add this?

@caub
Copy link
Contributor Author

caub commented Feb 24, 2024

@FaizBShah Amazing find, thanks very much

We should maybe do query.setOptions({...query.getOptions(), sort: newSort}) in case there are other fields in options that we want to keep (limit, skip for example)

Maybe we could add a .setSort(value[, shouldOverride]) method?

@vkarpov15
Copy link
Collaborator

@caub @FaizBShah I put in PR #14375 with a potential fix, can you please let me know what you think?

vkarpov15 added a commit that referenced this issue Feb 27, 2024
feat(query): add `options` parameter to `Query.prototype.sort()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

4 participants