[Koha-bugs] [Bug 20212] Slowness in receiving in acquisitions

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20212

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #117 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
1. We need a delay between the different AJAX request when filtering the table,
otherwise we face weird behaviour and lag (server and browser)

2. Prior to this patch, the "pending orders" table contained orders with status
(orderstatus) set to "ordered" or "partial". Now we see them all.
See 'pending' and 'ordered' flag of SearchOrders

3. Not blocker but worth noting, we lost the column filter on "Order cost"

4. I don't understand how works the column filters. It's the first view where
we introduce it. It would have been interesting to have it on a simpler view
(cities?) to understand how it is working.

There is this code:
793             // column filter events handling
794             $(".column-filter").each(function () {
795                 $(this).on( 'keyup change', function () {

It's like we are not using the DT standard way of using column filtering.

5. Code to build the URI is c/p in
794             $(".column-filter").each(function () {
and
809             $("#filterform").on("submit", function(e) {

6. In the schema:

+__PACKAGE__->has_many(
+  "biblioitem",
+  "Koha::Schema::Result::Biblioitem",
+  { "foreign.biblionumber" => "self.biblionumber" },
+  { cascade_copy => 0, cascade_delete => 0 },
+);

Why has_many? Why cascade_delete => 0?

7. Not blocker for now, but I think we need some JS functions to display
variables coming from the REST API.
For instance this code appears a lot:
  if ( data != null ) {
    return data.escapeHtml().format_price();
  }
  else {
    return "";
  }

or

  if ( data == null ) {
    return "";
  }
  else {
    return data;
  }

8. In the "render" you have
  if ( type != 'display' ) {
Why are you testing that?
The different values are sorting, filtering and display. According to the doc 
https://datatables.net/reference/option/columns.render

9. There are several places where strings are not escaped

At least 2 examples:

+                            return "<a
href=\"/cgi-bin/koha/acqui/basketgroup.pl?op=add&booksellerid="
+                                    + row.basket.vendor_id +
"&basketgroupid="
+                                    + row.basket.basket_group_id + "\">"
+                                    + row.basket.basket_group.name + " (" +
row.basket.basket_group_id + ")</a>";


+                        return "<a
href=\"/cgi-bin/koha/acqui/basket.pl?basketno=" + row.basket.basket_id + "\">"
+ data + " (" + row.basket.basket_id + ")</a>";


Side notes (and my opinion), I don't think we are going into the right
direction. The JS code we are adding in our template is not robust, hard to
read, and so hard to maintain.
Also we will have to rewrite things we have done in TT, like display of
patron's name for instance (patron-title.inc).
I really would like to see some efforts in improving the readability of the
code we have already before adding more.

-- 
You are receiving this mail because:
You are watching all bug changes.