loopbackio / loopback-connector-elastic-search

Strongloop Loopback connector for Elasticsearch
MIT License
79 stars 56 forks source link

makeId refactoring not applied to save method #37

Closed ulsamern closed 7 years ago

ulsamern commented 8 years ago

Seems as if the refactoring that was done in https://github.com/strongloop-community/loopback-connector-elastic-search/commit/25e902936a749b9eb482d28b9df31259c3a9d761#diff-a27d2580e4157c6999e9327a4a358c21L446 was not applied to ESConnector.prototype.save (marked the line in the link above).

When I try to save an existing model the esConnector.js throws the following error:

TypeError: self.makeId is not a function
transform_1 |         at ESConnector.save (/usr/local/lib/node_modules/loopback-connector-es/lib/esConnector.js:1019:35)
transform_1 |         at /usr/local/lib/node_modules/loopback-datasource-juggler/lib/dao.js:2244:25
transform_1 |         at doNotify (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)
transform_1 |         at doNotify (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)
transform_1 |         at doNotify (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)
transform_1 |         at doNotify (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)
transform_1 |         at Function.ObserverMixin._notifyBaseObservers (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:121:5)
transform_1 |         at Function.ObserverMixin.notifyObserversOf (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)
transform_1 |         at Function.ObserverMixin._notifyBaseObservers (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)
transform_1 |         at Function.ObserverMixin.notifyObserversOf (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)
transform_1 |         at Function.ObserverMixin._notifyBaseObservers (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)
transform_1 |         at Function.ObserverMixin.notifyObserversOf (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)
transform_1 |         at Function.ObserverMixin._notifyBaseObservers (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)
transform_1 |         at Function.ObserverMixin.notifyObserversOf (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)
transform_1 |         at ModelConstructor.<anonymous> (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/dao.js:2238:17)
transform_1 |         at ModelConstructor.trigger (/usr/local/lib/node_modules/loopback-datasource-juggler/lib/hooks.js:70:12)

I will refactor and create a PR for it

ketansp commented 7 years ago

Have we got a solution?

pulkitsinghal commented 7 years ago

@aquid has either already merged a PR or is very close to it. He is in the caught up middle of some philosophical discussions about what save should or should not do according to the folks at loopback. https://github.com/strongloop/loopback-datasource-juggler/issues/1233

You can expect a release by end of this week.

aquid commented 7 years ago

@ketansp Pull request is already merged and you will have this issue resolved once the latest version is released. As far as the save method's expected behavior discussion goes, I will update the save method functionality based on the final conclusion from the discussion. But as of now, es connector save method supports partial update.

cc @pulkitsinghal

pulkitsinghal commented 7 years ago

released in v1.3.2