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

Allow passing null to base model ctor #1492

Merged
merged 1 commit into from
Oct 4, 2017

Conversation

zbarbuto
Copy link
Contributor

Description

Redis connector will pass null back if entity is not found causing data.constructor to throw an error. Rather than updating the connector, I figured it's probably best to check for null as well since undefined is already being checked.

Related issues

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

@zbarbuto
Copy link
Contributor Author

Wasn't exactly sure where to put the test - can be moved anywhere that makes sense.

@zbarbuto
Copy link
Contributor Author

@slnode test please

@zbarbuto
Copy link
Contributor Author

Can't seem to view what is causing the failures. Detail links aren't loading for me.

@zbarbuto
Copy link
Contributor Author

zbarbuto commented Oct 4, 2017

Anyone able to help with why this may be failing?

@zbarbuto
Copy link
Contributor Author

zbarbuto commented Oct 4, 2017

@slnode test please

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

Hello @zbarbuto, thank you for the pull request. Your proposal to fix the issue here in juggler makes perfect sense to me.

I checked the failed CI builds of our downstream consumers, they seem to be unrelated to your changes.

I think this pull request is good to be merged 👍

@bajtos bajtos merged commit 3a6ddf9 into loopbackio:master Oct 4, 2017
@bajtos
Copy link
Member

bajtos commented Oct 4, 2017

Landed, thank you! 🎉

@jannyHou jannyHou mentioned this pull request Oct 13, 2017
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.

None yet

3 participants