{"action":"created","issue":{"url":"https://api.github.com/repos/dadi/api/issues/512","repository_url":"https://api.github.com/repos/dadi/api","labels_url":"https://api.github.com/repos/dadi/api/issues/512/labels{/name}","comments_url":"https://api.github.com/repos/dadi/api/issues/512/comments","events_url":"https://api.github.com/repos/dadi/api/issues/512/events","html_url":"https://github.com/dadi/api/issues/512","id":382730007,"node_id":"MDU6SXNzdWUzODI3MzAwMDc=","number":512,"title":"Add `runHooks` parameter to model methods","user":{"login":"eduardoboucas","id":4162329,"node_id":"MDQ6VXNlcjQxNjIzMjk=","avatar_url":"https://avatars3.githubusercontent.com/u/4162329?v=4","gravatar_id":"","url":"https://api.github.com/users/eduardoboucas","html_url":"https://github.com/eduardoboucas","followers_url":"https://api.github.com/users/eduardoboucas/followers","following_url":"https://api.github.com/users/eduardoboucas/following{/other_user}","gists_url":"https://api.github.com/users/eduardoboucas/gists{/gist_id}","starred_url":"https://api.github.com/users/eduardoboucas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eduardoboucas/subscriptions","organizations_url":"https://api.github.com/users/eduardoboucas/orgs","repos_url":"https://api.github.com/users/eduardoboucas/repos","events_url":"https://api.github.com/users/eduardoboucas/events{/privacy}","received_events_url":"https://api.github.com/users/eduardoboucas/received_events","type":"User","site_admin":false},"labels":[{"id":301294235,"node_id":"MDU6TGFiZWwzMDEyOTQyMzU=","url":"https://api.github.com/repos/dadi/api/labels/type:%20discussion","name":"type: discussion","color":"795548","default":false},{"id":301294232,"node_id":"MDU6TGFiZWwzMDEyOTQyMzI=","url":"https://api.github.com/repos/dadi/api/labels/type:%20enhancement","name":"type: enhancement","color":"3f51b5","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-20T15:56:55Z","updated_at":"2018-11-20T19:49:58Z","closed_at":null,"author_association":"MEMBER","body":"Currently, any configured hooks will always run as part of create, update and delete operations. Whilst this is expected for requests to the RESTful endpoints, it's not always desirable to have hooks running as part of calls to the model methods, as it can lead to hooks triggering other hooks in unexpected ways, potentially leading to infinite loops. This was reported in #258.\r\n\r\nIf we introduce a `runHooks` parameter to `model.create`, `model.delete`, `model.get` and `model.update`, we can bypass hook execution when needed. What I'm not sure about is what the default value for this new parameter should be.\r\n\r\nOn the one hand, it makes sense to make it `true`. This is fully backwards-compatible, as no changes are required to retain the current functionality. Sending `{runHooks: false}` to the model methods would change the default behaviour and stop hooks from running.\r\n\r\nOn the other hand, we could default to `false` and change the controller methods to include `{runHooks: true}` in their own calls to the model methods. In practice, this means that requests made to the RESTful endpoints will continue to work in the same way, as it's not possible for end users to bypass hooks (and it shouldn't be, for security reasons), but any calls to the model methods would need to *actively enable* hooks.\r\n\r\n**Pros of option 1**: Maintains backwards-compatibility\r\n**Pros of option 2**: Solves #258 in itself\r\n\r\nThoughts @mingard @jimlambie?"},"comment":{"url":"https://api.github.com/repos/dadi/api/issues/comments/440407402","html_url":"https://github.com/dadi/api/issues/512#issuecomment-440407402","issue_url":"https://api.github.com/repos/dadi/api/issues/512","id":440407402,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MDQwNzQwMg==","user":{"login":"mingard","id":1352469,"node_id":"MDQ6VXNlcjEzNTI0Njk=","avatar_url":"https://avatars1.githubusercontent.com/u/1352469?v=4","gravatar_id":"","url":"https://api.github.com/users/mingard","html_url":"https://github.com/mingard","followers_url":"https://api.github.com/users/mingard/followers","following_url":"https://api.github.com/users/mingard/following{/other_user}","gists_url":"https://api.github.com/users/mingard/gists{/gist_id}","starred_url":"https://api.github.com/users/mingard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mingard/subscriptions","organizations_url":"https://api.github.com/users/mingard/orgs","repos_url":"https://api.github.com/users/mingard/repos","events_url":"https://api.github.com/users/mingard/events{/privacy}","received_events_url":"https://api.github.com/users/mingard/received_events","type":"User","site_admin":false},"created_at":"2018-11-20T19:49:58Z","updated_at":"2018-11-20T19:49:58Z","author_association":"MEMBER","body":"Of the two I would favour option 1. The current default behaviour makes sense, and under most circumstances running hooks is fine.\r\n\r\nIdeally, at least for my experience with the issue, a flag for the source of the call would be great. Some type of _internal/external_ parameter.\r\n\r\nTo extend this to solve #258, perhaps introducing some way of attaching data to the calls from within the hook would be a more generalised feature to help understand more about the call. Being able to set `callCounter` on a metadata parameter that can be received by the Model.\r\n\r\n```js\r\ncalledByBeforeCreate ++\r\nawait Model('foo').somePassthroughMethod({calledByBeforeCreate})\r\n  .update({\r\n    query: {\r\n      title: 'Bacon Bites'\r\n    },\r\n    update: {\r\n      deliciousRating:  5.0\r\n    }\r\n  })\r\n```"}}
{"actor":{"display_login":"mingard"}}