{"action":"created","issue":{"url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/28","labels_url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/28/labels{/name}","comments_url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/28/comments","events_url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/28/events","html_url":"https://github.com/bsiddiqui/bookshelf-modelbase/pull/28","id":116539405,"number":28,"title":"Throw actual Joi Error object + Don't break initialize inheritance","user":{"login":"eightyfive","id":3809074,"avatar_url":"https://avatars.githubusercontent.com/u/3809074?v=3","gravatar_id":"","url":"https://api.github.com/users/eightyfive","html_url":"https://github.com/eightyfive","followers_url":"https://api.github.com/users/eightyfive/followers","following_url":"https://api.github.com/users/eightyfive/following{/other_user}","gists_url":"https://api.github.com/users/eightyfive/gists{/gist_id}","starred_url":"https://api.github.com/users/eightyfive/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eightyfive/subscriptions","organizations_url":"https://api.github.com/users/eightyfive/orgs","repos_url":"https://api.github.com/users/eightyfive/repos","events_url":"https://api.github.com/users/eightyfive/events{/privacy}","received_events_url":"https://api.github.com/users/eightyfive/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-11-12T12:22:06Z","updated_at":"2015-11-13T08:29:18Z","closed_at":"2015-11-13T05:28:44Z","pull_request":{"url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/pulls/28","html_url":"https://github.com/bsiddiqui/bookshelf-modelbase/pull/28","diff_url":"https://github.com/bsiddiqui/bookshelf-modelbase/pull/28.diff","patch_url":"https://github.com/bsiddiqui/bookshelf-modelbase/pull/28.patch"},"body":"**UPDATE 2**\r\n\r\nWe have to actually return `modelbase` from the factory for this change to work.. If not we get a `RangeError: Maximum call stack size exceeded`, that makes sense.\r\n\r\nSo I changed the last line:\r\n\r\n```js\r\nbookshelf.Model = model\r\n\r\n// To:\r\nreturn model;\r\n```\r\n\r\nIt was originally like that though (returning the `model`), why did you change this behaviour? I think it is cleaner *not* to override `bookshelf.Model`, but what was the reason to do so?\r\n\r\n**UPDATE**\r\n\r\nI did another small commit/fix in that master branch so I edit this pull request to include both changes.\r\n\r\nI hope that's alright. Please advise.\r\n\r\n\r\n## Throw actual Joi Error object\r\n\r\nCommit: https://github.com/eightyfive/bookshelf-modelbase/commit/e82f5ed4f5ef7365f030aa3dc125e94cb6d9c6f6\r\n\r\nI did a small fix in order to throw the actual Joi `Error` object and have access to all detailed information provided by Joi:\r\n\r\n```js\r\n [ValidationError: \"user_id\" must be a number]\r\n  name: 'ValidationError',\r\n  details: \r\n   [ { message: '\"user_id\" must be a number',\r\n       path: 'user_id',\r\n       type: 'number.base',\r\n       context: [Object] } ],\r\n  _object: \r\n   { user_id: 'benoit',\r\n     name: 'benoit sagols',\r\n     ...},\r\n  annotate: [Function] }\r\n```\r\n\r\n## Don't break initialize inheritance\r\n\r\nCommit: https://github.com/eightyfive/bookshelf-modelbase/commit/a0c44a9d12c6846cd41a8a814fccf911cc575c5f\r\n\r\nInstead of using `initialize` for `modelbase` initialization, I made use of `constructor` method as advised in [Bookshelf official documentation](http://bookshelfjs.org/#Model-subsection-construction):\r\n\r\n> In rare cases, if you're looking to get fancy, you may want to override constructor, which allows you to replace the actual constructor function for your model.\r\n\r\nThat way:\r\n\r\n- We can use `initialize` method normally in sub-sequent models extending `modelbase`\r\n- We don't break Bookshelf `initialize` functionality which is [meant to remain empty](https://github.com/tgriesser/bookshelf/blob/master/src/base/model.js#L85) for the base model.\r\n- We stick to common accepted way to extend Backbone-kind Libs. This is how [MarionetteJS extends Backbone](https://github.com/marionettejs/backbone.marionette/blob/master/src/view.js#L11) for instance.\r\n\r\n\r\nBoth changes are non-breaking.\r\n\r\n \r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/comments/156361057","html_url":"https://github.com/bsiddiqui/bookshelf-modelbase/pull/28#issuecomment-156361057","issue_url":"https://api.github.com/repos/bsiddiqui/bookshelf-modelbase/issues/28","id":156361057,"user":{"login":"eightyfive","id":3809074,"avatar_url":"https://avatars.githubusercontent.com/u/3809074?v=3","gravatar_id":"","url":"https://api.github.com/users/eightyfive","html_url":"https://github.com/eightyfive","followers_url":"https://api.github.com/users/eightyfive/followers","following_url":"https://api.github.com/users/eightyfive/following{/other_user}","gists_url":"https://api.github.com/users/eightyfive/gists{/gist_id}","starred_url":"https://api.github.com/users/eightyfive/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eightyfive/subscriptions","organizations_url":"https://api.github.com/users/eightyfive/orgs","repos_url":"https://api.github.com/users/eightyfive/repos","events_url":"https://api.github.com/users/eightyfive/events{/privacy}","received_events_url":"https://api.github.com/users/eightyfive/received_events","type":"User","site_admin":false},"created_at":"2015-11-13T08:29:18Z","updated_at":"2015-11-13T08:29:18Z","body":"I appreciate your responsiveness ;)"}}