https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38122 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #98 from Jonathan Druart <jonathan.druart@gmail.com> --- 1. I like the UI, but the code seems too complicated IMO. a. Duplicating the status logic at the DB level (order by) is a bad idea. b. The coded value (01 for On hold, 00 for Available) it not a good pattern c. Before following up on the other remarks, please explain why the first patch is not working correctly. I feel like we should do baby steps here instead of massive changes not covered by tests. 2. prove xt/tt_kohaTable_tidy.t xt/tt_kohaTable_tidy.t .. 1/879 # Failed test 'koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc has 'kohaTable' instances in a <script> tag with Template::Toolkit tags. You have reverted most of the changes done on bug 41566. 3. Missing tests 4. I don't think we need to group_by_status in the global swagger.yaml. What's the reason? api/v1/swagger/swagger.yaml + group_by_status: 5. - ajax: { url: item_table_url }, + ajax: { url: "/api/v1/biblios/[% biblio.biblionumber | uri %]/items?" + group_by_status() }, => Then the item_table_url is not longer used and can be removed at 303 let item_table_url = `/api/v1/biblios/${biblionumber}/items?`; 6. Performances - did you benchmark this approach on a biblio with lot of items? 7. Why is filter_out_restricted ordering items? I don't think it should. 8. You removed a FIXME, did you fix it? - # FIXME We need to order_by serial.publisheddate if we have _order_by=+me.serial_issue_number 9. Did you see bug 41254 and bug 41602? -- You are receiving this mail because: You are watching all bug changes.