https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26297
--- Comment #36 from Tomás Cohen Arazi <[email protected]> --- I've reviewed this and have a few remarks. * I felt tempted to submit all this squashed (minor/tiny thing) * The tests shouldn't delete all data, this is bad. They should probably use `q=` to constraint the resultset instead. And if we are trying to test the `search_with_library_limits` nature of the implementation, then we need to properly build the tests by adding a couple libraries, several categories limited to one and the other, and then crafting a `q=` query that searches for them all, and we only get the right ones for the branch we are logged at. * I don't like it that it returns the categories limited by branch by default. That would work for rendering dropdowns, but not for a CRUD page for categories. And that use case is vital for implementing `GET /patron_categories`. * If we need a way to restrict the returned categories by branch, then we should add a switch for that. * The standard parameters we add to all `list()` endpoints are missing on the spec (i.e. no pagination, no q=, etc). * No `use Koha::Patron::Categories;` * We should use Koha::V1::Patron::Categories, double plural sounds wrong (i.e. Patrons). -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
