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

keep native errors for umanaged collections #457

Conversation

DmytroSoninLinguahouse
Copy link
Contributor

Table of Contents generated with DocToc

What

Change method monkey patching code to allow to return unaltered error message when collection isn't being validated and doesn't have any simplschema attached.

Why

collection2 monkey patches Collection class globally changing collection instances for the whole project even if they don't use collection2 functionalities.
For new collections we were using mongodb's native json schema validation, but errors returned by mongo are getting changed by collection2 and end up being unhelpful (it leaves the text message but strips any actual useful data from mongo error).
This pr makes presence of collection2 less intrusive for collections that don't use collection2 for validation

@harryadel
Copy link
Member

harryadel commented Oct 29, 2024

The change itself is alright but 1) Why did you change the entire package setup? 2) Can you please add tests for your changes?

@DmytroSoninLinguahouse
Copy link
Contributor Author

@harryadel oh, my bad. Second commit wasn't meant for pr, it was for my fork only

This reverts commit e5cb830.
@harryadel
Copy link
Member

harryadel commented Oct 29, 2024

if the current tests suit passes and you add a test case for your change then I'll gladly accept it :)

@harryadel harryadel mentioned this pull request Nov 10, 2024
@harryadel
Copy link
Member

Thank you @DmytroSoninLinguahouse I'm adding the tests and continuing the work here
#460

@harryadel harryadel closed this Nov 10, 2024
@DmytroSoninLinguahouse
Copy link
Contributor Author

@harryadel much appreciated, you're a star :)
I had environment issues, couldn't run meteor locally to do tests, planned to make a dockerfile as workaround, but didn't find time for it.

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

Successfully merging this pull request may close these issues.

2 participants