https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11708
Julian Maurice <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #249 from Julian Maurice <[email protected]> --- (In reply to Jonathan Druart from comment #245) > 1. aqbasketgroups.closedate is set to NOW, maybe better to use > basket.closedate (the latest)? Fixed > 2. ->bookseller should be ->vendor (not blocker as we already have both in > Koha::*...) Fixed > 3. a. I would not "cache" ->baskets, we already deal with the context in > Koha::Objects->search I don't know what is the context you are talking about, but I removed the "caching" > b. ->baskets_count - no need to have it, it should be replaced with > baskets->count > => Maybe you want to keep them for performance purpose, but is it really a > performance improvement? It's used only in templates, because TT seems to force the list context even with a directive like this: [% basketgroup.baskets.count %] It tries to call count on the first item of @{ $basketgroup->baskets } I'd happily remove this sub if you have a better idea. > 5. basketgroup.tt has a [% IF booksellerid %], it should not be useful now. Fixed > We may want to display a friendly message if called without a [valid] id Is returning a 404 error friendly ? Because it's probably what should be done if the id is invalid. > 6. basketgroups.tt should have JS at the bottom Fixed > 7. Some links still point to basketgroup.pl?booksellerid=X without the > basketgroupid, they should be replaced right? > i.e. code related to 'sub displaybasketgroups' in basketgroup.pl should be > removed (?) Fixed > 8. Are you sure you need the changes done to C4::Acquisition? No. They're not needed anymore. I removed them. > 9. acqui/basketgroup.pl > + use List::MoreUtils qw/uniq/; > => Not needed Fixed > 10. "kohaDataTable": We already have KohaTable. You are adding a 3rd way to > init a table. We already have 3 ways to initialize DT: 1. The "legacy API" way: $("#table_id").dataTable($.extend(true, {}, dataTablesDefaults, { ... }); 2. The "legacy API + column_settings" way: KohaTable('table_id', ...); 3. The "1.10+ API" way $('#table_id').DataTable(...) kohaDataTable is just a wrapper around the 3rd that provides the same defaults than the 1st and 2nd methods. I don't see how this is bad. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://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/
