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

fix: pass relaxed: false to EJSON.deserialize() so values won't be promoted COMPASS-7431 #5090

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Nov 9, 2023

No description provided.

@@ -132,7 +132,7 @@ describe('importJSON', function () {
hasUnboundArray: false,
});

const docs = await dataService.find(ns, {});
const docs = await dataService.find(ns, {}, { promoteValues: false });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to add this so I could assert against the values without the values being promoted.

@lerouxb lerouxb force-pushed the import-json-relaxed-false branch from 863b390 to 6b6af12 Compare November 9, 2023 18:50
@@ -158,7 +158,7 @@ describe('importJSON', function () {
throw err;
}

const expectedResult = EJSON.parse(text);
const expectedResult = EJSON.parse(text, { relaxed: false });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@lerouxb lerouxb force-pushed the import-json-relaxed-false branch from 6b6af12 to 8cb16ef Compare November 9, 2023 18:52
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Nice test add.

@lerouxb lerouxb merged commit 60997d1 into main Nov 10, 2023
5 checks passed
@lerouxb lerouxb deleted the import-json-relaxed-false branch November 10, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants