search

psecio / gatekeeper

Gatekeeper: An Authentication & Authorization Library
366 stars 23 forks source link

Added Count Feature #48

Closed Swader closed 8 years ago

Swader commented 8 years ago

As per #45, here's a PR that adds the count feature.

The feature is implemented via a custom handler and supports where, so that certain conditions can be applied if needed.

The feature was manually tested because handlers have no unit tests. This should probably be rectified some time in the future.

Please review.

enygma commented 8 years ago

So this is to count the number of items in the matching table? Ah, I see - it's using a new function on the data source, missed that at initial look. Any chance at all of adding maybe just one test for it?

Swader commented 8 years ago

Sure thing - but since handlers don't seem have tests in general, any advice on that? Any example you can point me to which I could use for inspiration? Don't really feel like writing a ton of mocks etc.

Swader commented 8 years ago

I've added another fix while I'm at it - the creation of related records causes an error and breaks one's app if the original record's creation failed. It's just a block moved inside an IF check, so I didn't make a separate PR.

enygma commented 8 years ago

Re: the tests, that's fine - can always circle back around on it later. Thanks for the other fixes too. Merging.