Bug 25288

Summary: Make the libraries list use the API
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: System AdministrationAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: agustinmoyano, f.demians, gmcharlt, jonathan.druart, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 25279, 25287    
Bug Blocks:    
Attachments: Bug 25288: Add general query parameters to the /libraries route
Bug 25288: Use the API to list libraries
Bug 25287: Add columns_settings capabilities to API datatables wrapper
Bug 25288: Add general query parameters to the /libraries route
Bug 25288: Use the API to list libraries
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour
Bug 25288: (follow-up) Use .escapeHtml()
Bug 25288: Add general query parameters to the /libraries route
Bug 25288: Use the API to list libraries
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour
Bug 25288: (follow-up) Use .escapeHtml()

Description Tomás Cohen Arazi 2020-04-27 05:31:12 UTC

    
Comment 1 Tomás Cohen Arazi 2020-04-27 05:39:19 UTC
Created attachment 103729 [details] [review]
Bug 25288: Add general query parameters to the /libraries route

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2020-04-27 05:39:23 UTC
Created attachment 103730 [details] [review]
Bug 25288: Use the API to list libraries

This patch makes the general libraries datatable use the API for rendering.
To test:
1. Test the datatable behaviour
2. Apply this patch
3. Repeat your tests
=> SUCCESS: Things work! Filtering and sorting specially

Bonus: Use the browser inspector to notice each interaction with thex
datatable triggers an API call with the right query parameters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Frédéric Demians 2020-04-28 06:38:04 UTC
Created attachment 103824 [details] [review]
Bug 25287: Add columns_settings capabilities to API datatables wrapper

This patch adds the code that is used for handling columns settings on
datatables and allows passing the columns_settings information to the
API-centric datatable. To test, you need bug 25288, which uses this features.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Frédéric Demians 2020-04-28 06:39:19 UTC
Created attachment 103825 [details] [review]
Bug 25288: Add general query parameters to the /libraries route

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Comment 5 Frédéric Demians 2020-04-28 06:39:26 UTC
Created attachment 103826 [details] [review]
Bug 25288: Use the API to list libraries

This patch makes the general libraries datatable use the API for rendering.
To test:
1. Test the datatable behaviour
2. Apply this patch
3. Repeat your tests
=> SUCCESS: Things work! Filtering and sorting specially

Bonus: Use the browser inspector to notice each interaction with thex
datatable triggers an API call with the right query parameters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Comment 6 Frédéric Demians 2020-04-28 06:42:52 UTC
After applying in order patches 24561 and 25287, I was able to see this patch working. Seen the requests sent to /api/v1/. For example:

/api/v1/libraries?_page=1&_per_page=20&_match=starts_with&_order_by=%2Bme.name

With very descent response time.
Comment 7 Tomás Cohen Arazi 2020-04-28 13:01:13 UTC
Thanks, Frederic.
Comment 8 Tomás Cohen Arazi 2020-05-06 21:58:21 UTC
Created attachment 104461 [details] [review]
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour

After the QA round on bug 25279, some things needed to get fixed:
- Data needs to be escaped
- When no library is defined, we should keep the original behaviour and
  message.

This patch does that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2020-05-06 22:28:25 UTC
Created attachment 104462 [details] [review]
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour

After the QA round on bug 25279, some things needed to get fixed:
- Data needs to be escaped
- When no library is defined, we should keep the original behaviour and
  message.
- aria-hidden="true" for FontAwesome coding guideline enforced
- Added role="button" as well.

This patch does that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2020-05-07 14:09:23 UTC
Created attachment 104515 [details] [review]
Bug 25288: (follow-up) Use .escapeHtml()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=25388
Comment 11 Jonathan Druart 2020-06-16 14:14:45 UTC
In the search box, search "❤":

[2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73)


172.18.0.1 - - [16/Jun/2020:14:11:37 +0000] "GET /api/v1/app.pl/api/v1/libraries?_page=1&_per_page=20&q=%5B%7B%22me.name%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.library_id%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.marc_org_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.ip%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.pickup_location%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address1%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address2%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address3%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.city%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.state%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.postal_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.country%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.opac_info%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.notes%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%5D&_match=contains&_order_by=%2Bme.name HTTP/1.1" 500 - "http://kohadev-intra.mydnsname.org:8081/cgi-bin/koha/admin/branches.pl" "Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0"
Comment 12 Jonathan Druart 2020-06-16 14:21:43 UTC
(In reply to Jonathan Druart from comment #11)

I opened bug 25774
Comment 13 Jonathan Druart 2020-06-16 14:40:35 UTC
Created attachment 105923 [details] [review]
Bug 25288: Add general query parameters to the /libraries route

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2020-06-16 14:40:38 UTC
Created attachment 105924 [details] [review]
Bug 25288: Use the API to list libraries

This patch makes the general libraries datatable use the API for rendering.
To test:
1. Test the datatable behaviour
2. Apply this patch
3. Repeat your tests
=> SUCCESS: Things work! Filtering and sorting specially

Bonus: Use the browser inspector to notice each interaction with thex
datatable triggers an API call with the right query parameters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2020-06-16 14:40:42 UTC
Created attachment 105925 [details] [review]
Bug 25288: (QA follow-up) Escape data and restore empty list behaviour

After the QA round on bug 25279, some things needed to get fixed:
- Data needs to be escaped
- When no library is defined, we should keep the original behaviour and
  message.
- aria-hidden="true" for FontAwesome coding guideline enforced
- Added role="button" as well.

This patch does that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2020-06-16 14:40:45 UTC
Created attachment 105926 [details] [review]
Bug 25288: (follow-up) Use .escapeHtml()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2020-06-16 14:41:45 UTC
(In reply to Jonathan Druart from comment #16)
> Created attachment 105926 [details] [review] [review]
> Bug 25288: (follow-up) Use .escapeHtml()

This is a quite ugly way of doing it. We must provide a better alternative before starting using it elsewhere.
Comment 18 Jonathan Druart 2020-06-24 13:32:29 UTC
Pushed to master for 20.11, thanks to everybody involved!