Open nabilfreeman opened 7 years ago
@mikec I know this was a while back, but can you advise on the above? If there is a bug I would be happy to PR the fix, otherwise I think I will have to run off a forked version with lines 52 and 53 removed 😟
Here is an example where hooks are broken (they just don't run):
this.kalamata.expose(require('./some_bookshelf_model.js'), {
modelName: 'WorkingHour',
collectionName: 'WorkingHours',
endpointName: 'working_hours'
});
And one where it works properly:
this.kalamata.expose(require('./other_bookshelf_model.js'), {
modelName: 'User',
collectionName: 'Users',
endpointName: 'users'
});
In both situations the hooks are created (and are visible on the kalamata
object), but they only run for ones like the second example.
I have a fixed fork at https://github.com/nabilfreeman/kalamata/blob/master/src/index.js, just removed the code that was overwriting the hooks basically.
@nabilfreeman thanks for finding this, that's a pretty nasty bug!
I think this will fix it https://github.com/mikec/kalamata/pull/7
Try that out for a bit, if it's working for you then I'll merge it in!
Wish that I had added better unit testing and more modularization in this code base. Having everything in index.js is a little ridiculous!
Hey @mikec, thanks for getting back to me! Really appreciate the maintenance. I'm trying your branch today and will let you know how it works 👍
Update: works well! Thanks for the help 👌 @mikec
@nabilfreeman awesome! 🎉
This commit:
https://github.com/mikec/kalamata/commit/bdd45d6a6c92492afed5e3b3d69685fd05e65a20
appears to have broken hooks in situations where the lowercase modelName would not be the same as the endpoint name.
I think this is because on this line, the hooks for all the routes are being retrieved with the key
modelNameLower
:https://github.com/mikec/kalamata/blob/bdd45d6a6c92492afed5e3b3d69685fd05e65a20/src/index.js#L52
Also, it looks like
beforeHooks
andafterHooks
are actually being re-declared and overwriting a different value a few lines up.but in hookFn, all the hooks are being attached with this key: https://github.com/mikec/kalamata/blob/bdd45d6a6c92492afed5e3b3d69685fd05e65a20/src/index.js#L392
So it seems that the code either needs to use modelNameLower in both places, or use the endpointName in both places. I think I managed to fix it simply by deleting the re-declaration of
beforeHooks
andafterHooks
but this might have consequences I haven't recognised.