[Koha-bugs] [Bug 11708] Display all basketgroups on one page, and new column aqbasketgroups.closeddate
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Fri Sep 14 18:05:11 CEST 2018
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11708
Julian Maurice <julian.maurice at biblibre.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Failed QA |Signed Off
--- Comment #249 from Julian Maurice <julian.maurice at biblibre.com> ---
(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.
More information about the Koha-bugs
mailing list