Skip to content

Implement operation hooks for EmbedsOne methods#904

Merged
bajtos merged 1 commit into
masterfrom
feature/hooks-for-embeds-one
Apr 20, 2016
Merged

Implement operation hooks for EmbedsOne methods#904
bajtos merged 1 commit into
masterfrom
feature/hooks-for-embeds-one

Conversation

@bajtos

@bajtos bajtos commented Apr 14, 2016

Copy link
Copy Markdown
Member

create() triggers

  • before save
  • after save

udpate() triggers

  • before save
  • after save

destroy() triggers

  • before delete
  • after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

Connect to #497
See the original implementation proposed by @fabien in #516

/to @raymondfeng @Amir-61 please review

Things left out

  • before save and after save hooks should include ctx.isNewInstance
  • Async validations are not supported yet
  • persist and loaded hooks are not triggered at all
  • Updating embedded instances triggers update of the parent (owning) model. The context provided by before save and after save hooks on the parent model is sort of arbitrary and may include wrong/extra data. The same probably applies to the scenario when deleting embedded instances triggers update of parent model.
  • before delete hook does not provide ctx.where property and it's not possible to change the outcome of destroy() using this hook. Note that regular DAO does support this.

var aCtxForModel = contextTestHelpers.aCtxForModel;

var uid = require('../helpers/uid-generator');
var HookMonitor = require('../helpers/hook-monitor');

@Amir-61 Amir-61 Apr 19, 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.

Nitpick: Shouldn't these variables be alphabetical order? First variables starting with uppercase and then the variables with lowercase and alphabetical ordered?

PS: It applies to other places as well

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, we don't have any hard rule for ordering and grouping variables (module imports). Could you please post a code snippet showing how would you like to see L6-L13 ordered?

@Amir-61 Amir-61 Apr 19, 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 don't have any hard rule for ordering and grouping variables (module imports)

Oh I see... yea it was just a nitpick...I just saw some Strongloop folks prefer to have ASCI order of variables...
Something like this:

var ContextRecorder = contextTestHelpers.ContextRecorder;
var HookMonitor = require('../helpers/hook-monitor');
var ValidationError = require('../..').ValidationError;
var aCtxForModel = contextTestHelpers.aCtxForModel;
var contextTestHelpers = require('../helpers/context-test-helpers');
var uid = require('../helpers/uid-generator');

@Amir-61 Amir-61 Apr 19, 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.

So my comment should be ignored if we don't have this rule... I just thought we follow this rule... Thanks Miroslav for clarification :-)

@raymondfeng

Copy link
Copy Markdown
Contributor

LGTM

@bajtos

bajtos commented Apr 20, 2016

Copy link
Copy Markdown
Member Author

@Amir-61 I think all comments have been addressed, I'll go ahead and land this without waiting for your final LGTM.

create() triggers
 - before save
 - after save

udpate() triggers
 - before save
 - after save

destroy() triggers
 - before delete
 - after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.
@bajtos
bajtos force-pushed the feature/hooks-for-embeds-one branch from 9e1b1e4 to b86615e Compare April 20, 2016 07:48
@bajtos
bajtos merged commit 7247b66 into master Apr 20, 2016
@bajtos
bajtos deleted the feature/hooks-for-embeds-one branch April 20, 2016 08:29
@bajtos bajtos removed the #review label Apr 20, 2016
bajtos added a commit that referenced this pull request Apr 20, 2016
create() triggers
 - before save
 - after save

udpate() triggers
 - before save
 - after save

destroy() triggers
 - before delete
 - after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

[back-port of #904]
mitsos1os pushed a commit to mitsos1os/loopback-datasource-juggler that referenced this pull request Jun 3, 2016
create() triggers
 - before save
 - after save

udpate() triggers
 - before save
 - after save

destroy() triggers
 - before delete
 - after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

[back-port of loopbackio#904]
mitsos1os pushed a commit to mitsos1os/loopback-datasource-juggler that referenced this pull request Jun 3, 2016
create() triggers
 - before save
 - after save

udpate() triggers
 - before save
 - after save

destroy() triggers
 - before delete
 - after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

[back-port of loopbackio#904]
@voltuer

voltuer commented Nov 6, 2018

Copy link
Copy Markdown

what about find??

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.

4 participants