Skip to content

fix: hasAtomicOperator check respects toBSON transformation #2696

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 4 commits into from
Jan 19, 2021
Merged

fix: hasAtomicOperator check respects toBSON transformation #2696

merged 4 commits into from
Jan 19, 2021

Conversation

nbbeeken
Copy link
Contributor

Certain documents cannot contain atomic operators
i.e. keys with a leading dollar sign, documents can contain
toBSON transformation functions that would modify such keys the check
now respect the transformation

NODE-2741

@nbbeeken
fix: hasAtomicOperator check respects toBSON transformation
e329373 
Certain documents cannot contain atomic operators
i.e. keys with a leading dollar sign, documents can contain
toBSON transformation functions that would modify such keys the check
now respect the transformation

NODE-2741
@nbbeeken nbbeeken requested review from durran and emadum January 13, 2021 19:07
emadum
emadum approved these changes Jan 13, 2021
View reviewed changes
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

Stylistic nit, I'd prefer a ternary expression inside Object.keys rather than the docToTest variable, but LGTM 👍

durran
durran requested changes Jan 14, 2021
View reviewed changes
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

Although it's a minor fix would it be possible add a test to show the behaviour works please?

@nbbeeken nbbeeken requested a review from durran January 14, 2021 17:32
durran
durran approved these changes Jan 15, 2021
View reviewed changes
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

@nbbeeken nbbeeken merged commit 60936dc into 3.6 Jan 19, 2021
@nbbeeken nbbeeken deleted the NODE-2741/hasAtomicOperators-toBSON-check branch January 19, 2021 15:28
This was referenced Mar 9, 2021
Closed
Merged
Merged
Open
Merged
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@durran durran durran approved these changes

@emadum emadum emadum approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbbeeken @durran @emadum