-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(NODE-5102): fix listDatabases nameOnly setting is sent as NaN #3742
Conversation
…oolean. Also assigning that directly to the cmd object in execute. Added tests for listDatabases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @redixhumayun, thanks so much for this PR. I have left some specific comments on the files below. In addition to the unit tests you have written, before we can approve this change, we would need to have integration tests that would test this behaviour against a mongod instance. It would be good to create a new file for these in test/integration/enumerate_databases.test.ts
.
Please see the running tests locally section section of our test readme for details on how to get integration tests running locally.
For an example of how to write an integration test in this repo, please see the findAndModify tests.
@redixhumayun We do want to merge this change fairly soon, so we will take over the remaining code changes on this PR. Thanks again for your submission! |
Description
Fixes the issue described in NODE-5102 where
nameOnly
was being coerced intoNaN
using theNumber
constructor.What is changing?
Now, the
nameOnly
property is coerced to a boolean following the example laid out bylistCollections
. Accompanying unit tests have also been added.Is there new documentation needed for these changes?
No
What is the motivation for this change?
Fixes NODE-5102
Release Highlight
listDatabases
nameOnly
option bug fixThe
listDatabases
API exposes thenameOnly
option which allows you to limit its output to only the names of the databases on a given mongoDB deployment:Prior to this fix, the option was not being set properly on the command, so the output was always given in full.
Thanks to @redixhumayun for submitting this fix!
See #3742 for details.
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript