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 forceId #1454

Closed
wants to merge 1 commit into from
Closed

Fix forceId #1454

wants to merge 1 commit into from

Conversation

jannyHou
Copy link
Contributor

@jannyHou jannyHou commented Aug 7, 2017

Description

Related issue

loopbackio/loopback-connector-couchdb2#13

Test Case TBD in loopback-connector-couchdb2 and juggler

Copy link
Contributor

@rashmihunt rashmihunt left a comment

Choose a reason for hiding this comment

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

@bajtos fyi.. @jannyHou I do not think you need to make this change since I have done a proper fix in my PR for lib/datasource.js changes. See https://github.com/strongloop/loopback-datasource-juggler/pull/1453/files#diff-9e9565730a5d5c0bdf028ccde9dd3a4e The value set in this.settings will be correct since it's set back in to the settings[] after calculation with above change.

@bajtos
Copy link
Member

bajtos commented Aug 8, 2017

+1, let's fix the issue via #1453. It may be better to split #1453 into two pull requests, one fixing forceId and the second adding updateOnly property flag. That way the forceId fix can be landed quicker.

@jannyHou
Copy link
Contributor Author

jannyHou commented Aug 9, 2017

@bajtos @rashmihunt Thank you for reviewing this PR, absolutely the fix in https://github.com/strongloop/loopback-datasource-juggler/pull/1453/files#diff-9e9565730a5d5c0bdf028ccde9dd3a4eR636 is much better. Closing this one.

@jannyHou jannyHou closed this Aug 9, 2017
@jannyHou jannyHou removed the #review label Aug 9, 2017
@jannyHou jannyHou deleted the fix/replaceOrCreate-forceId branch April 12, 2019 19:59
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.

3 participants