|
Description
Tomás Cohen Arazi (tcohen)
2020-04-27 05:31:12 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> 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> 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> 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> 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> 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. Thanks, Frederic. 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> 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> 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 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" (In reply to Jonathan Druart from comment #11) I opened bug 25774 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> 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> 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> 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> (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. Pushed to master for 20.11, thanks to everybody involved! |