Skip to content

changes to dao.js and memory.js to add a new method on Upsert based on non-primary key - #995

Closed
mountain1234585 wants to merge 10 commits into
loopbackio:masterfrom
mountain1234585:upsertWithWhere
Closed

changes to dao.js and memory.js to add a new method on Upsert based on non-primary key#995
mountain1234585 wants to merge 10 commits into
loopbackio:masterfrom
mountain1234585:upsertWithWhere

Conversation

@mountain1234585

Copy link
Copy Markdown

Hi Raymond,
Kindly review the basic changes done to loopback-datasource-juggler lib/dao.js and lib/connectors/memory.js to include a new method called "upsertWithWhere" based on non-primary key.
1)This method leverages the existing methods, find, create, findById and updateAttributes.
2)I have not added notifyObserversOf like in other methods , since this method calls them internally.
3)Once you review the code changes, we will add the unit tests as well.
4) I have tested the operation manually by creating an application, memory db datasource and persisted model in case of create and update scenarios.
Thanks,
Sonali Samantaray

@slnode

slnode commented Jul 11, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@slnode

slnode commented Jul 11, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

2 similar comments
@slnode

slnode commented Jul 11, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

@slnode

slnode commented Jul 11, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

Comment thread lib/connectors/memory.js Outdated
});
};

Memory.prototype.upsertWithWhere = function(model,data,options,callback){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The method signature is not correct. The where argument is missing.

@slnode

slnode commented Jul 12, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

2 similar comments
@slnode

slnode commented Jul 12, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

@slnode

slnode commented Jul 12, 2016

Copy link
Copy Markdown

Can one of the admins verify this patch?

Comment thread lib/connectors/memory.js Outdated
self.create(data, function (err, results) {

if (err)
callback(err);

@Amir-61 Amir-61 Jul 12, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We always return err in one line:

if (err) callback(err);

@Amir-61

Amir-61 commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

@mountain1234585
Did you sign CLA? Please add tests and run npm test locally and fix failing tests as well as lint issues. You can type @slnode test please to run tests on CI to make sure you are passing tests.

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.

6 participants