|
Description
Magnus Enger
2017-05-12 10:57:11 UTC
Created attachment 74809 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Dependencies have been added to reflect other pending patches that touch ill-requests.tt, which has been substantially refactored here Dependencies updated Created attachment 79692 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie Comment on attachment 79692 [details] [review] Bug 18589: Show ILLs as part of patron profile Obsoleted by rebase upon current dependency tree Created attachment 82177 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie Created attachment 82513 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie Created attachment 84197 [details] [review] Bug 18589: (follow-up) Fix QA error Created attachment 84199 [details] [review] Bug 18589: (follow-up) Fix QA failure This patch removes the id and class attributes on the body tag in: koha-tmpl/intranet-tmpl/prog/en/modules/members/ill-requests.tt To test: - Apply the patch - Run the QA tools - Note that the "Failed test '<body> tag with id and class attributes'" no longer is reported Created attachment 84204 [details] [review] Bug 18589: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA errors Created attachment 84792 [details] [review] Bug 18589: (follow-up) Fix rebase errors Minor rebase errors Created attachment 84796 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie Bug 18589: (follow-up) Fix QA error Bug 18589: (follow-up) Fix QA failure This patch removes the id and class attributes on the body tag in: koha-tmpl/intranet-tmpl/prog/en/modules/members/ill-requests.tt To test: - Apply the patch - Run the QA tools - Note that the "Failed test '<body> tag with id and class attributes'" no longer is reported Bug 18589: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA errors Bug 18589: (follow-up) Fix rebase errors Minor rebase errors Bug 18589: (follow-up) More merge errors I had to squash all patches thus far in order to get them to apply. The previous patches contained within can be seen in the obsoleted patches for this bug. Created attachment 86432 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie Bug 18589: (follow-up) Fix QA error Bug 18589: (follow-up) Fix QA failure This patch removes the id and class attributes on the body tag in: koha-tmpl/intranet-tmpl/prog/en/modules/members/ill-requests.tt To test: - Apply the patch - Run the QA tools - Note that the "Failed test '<body> tag with id and class attributes'" no longer is reported Bug 18589: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA errors Bug 18589: (follow-up) Fix rebase errors Minor rebase errors Bug 18589: (follow-up) More merge errors Created attachment 86433 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie This is essentially a reimplementation of attachment 84796 [details] [review] to take into account all changes that had happened beneath this bug and also to ensure JS strings are translatable. Created attachment 86456 [details] [review] Bug 18589: (follow-up) Remove redundant-ness Following feedback from Josef via IRC, removing redundant code Andrew, thanks for quick rebase. I just run QA test tools and they are not happy: FAIL koha-tmpl/intranet-tmpl/prog/en/includes/ill-list-table-strings.inc FAIL forbidden patterns forbidden pattern: simple-quote string (line 2) forbidden pattern: simple-quote string (line 3) forbidden pattern: simple-quote string (line 5) forbidden pattern: simple-quote string (line 6) forbidden pattern: simple-quote string (line 7) forbidden pattern: simple-quote string (line 8) forbidden pattern: simple-quote string (line 9) forbidden pattern: simple-quote string (line 10) forbidden pattern: simple-quote string (line 11) forbidden pattern: simple-quote string (line 12) forbidden pattern: simple-quote string (line 14) forbidden pattern: simple-quote string (line 16) forbidden pattern: simple-quote string (line 17) FAIL koha-tmpl/intranet-tmpl/prog/en/includes/ill-list-table.inc FAIL forbidden patterns forbidden pattern: tab char (line 2) forbidden pattern: tab char (line 3) forbidden pattern: tab char (line 4) forbidden pattern: tab char (line 5) forbidden pattern: tab char (line 6) forbidden pattern: tab char (line 7) forbidden pattern: tab char (line 8) forbidden pattern: tab char (line 9) forbidden pattern: tab char (line 10) forbidden pattern: tab char (line 11) forbidden pattern: tab char (line 12) forbidden pattern: tab char (line 13) forbidden pattern: tab char (line 14) forbidden pattern: tab char (line 15) forbidden pattern: tab char (line 16) forbidden pattern: tab char (line 17) forbidden pattern: tab char (line 18) forbidden pattern: tab char (line 19) forbidden pattern: tab char (line 20) forbidden pattern: tab char (line 21) forbidden pattern: tab char (line 22) forbidden pattern: tab char (line 23) forbidden pattern: tab char (line 24) forbidden pattern: tab char (line 25) forbidden pattern: tab char (line 26) forbidden pattern: tab char (line 27) forbidden pattern: tab char (line 28) forbidden pattern: tab char (line 29) forbidden pattern: tab char (line 30) forbidden pattern: tab char (line 31) forbidden pattern: tab char (line 32) forbidden pattern: tab char (line 33) forbidden pattern: tab char (line 34) forbidden pattern: tab char (line 35) forbidden pattern: tab char (line 36) forbidden pattern: tab char (line 37) forbidden pattern: tab char (line 38) forbidden pattern: tab char (line 39) forbidden pattern: tab char (line 40) forbidden pattern: tab char (line 41) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/members/ill-requests.tt FAIL forbidden patterns forbidden pattern: tab char (line 16) forbidden pattern: tab char (line 17) forbidden pattern: tab char (line 19) forbidden pattern: tab char (line 20) forbidden pattern: tab char (line 21) forbidden pattern: tab char (line 22) forbidden pattern: tab char (line 23) forbidden pattern: tab char (line 25) forbidden pattern: tab char (line 27) forbidden pattern: tab char (line 28) forbidden pattern: tab char (line 29) forbidden pattern: tab char (line 30) forbidden pattern: tab char (line 31) forbidden pattern: tab char (line 32) forbidden pattern: tab char (line 33) forbidden pattern: tab char (line 34) forbidden pattern: tab char (line 35) forbidden pattern: tab char (line 36) forbidden pattern: tab char (line 37) forbidden pattern: tab char (line 38) forbidden pattern: tab char (line 39) forbidden pattern: tab char (line 41) forbidden pattern: tab char (line 42) forbidden pattern: tab char (line 43) forbidden pattern: tab char (line 44) forbidden pattern: tab char (line 45) forbidden pattern: tab char (line 46) forbidden pattern: tab char (line 47) forbidden pattern: tab char (line 48) forbidden pattern: tab char (line 49) forbidden pattern: tab char (line 50) forbidden pattern: tab char (line 51) forbidden pattern: tab char (line 52) forbidden pattern: tab char (line 53) forbidden pattern: tab char (line 54) forbidden pattern: tab char (line 55) forbidden pattern: tab char (line 56) forbidden pattern: tab char (line 57) forbidden pattern: tab char (line 58) forbidden pattern: tab char (line 60) forbidden pattern: tab char (line 61) Also, it would be nice to have test for changes in Koha/REST/V1/Illrequests.pm Created attachment 86465 [details] [review] Bug 18589: (follow-up) Fix QA tool failures As per comment #17: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c17 Sorry Josef, I honestly have no idea where those tabs came from :( Every once in a while my copy of Vim decides to embarass me and stick in tabs instead of spaces... <shakes fist at screen> 1) You are setting dateFormat on two places, but not use it anywhere.
2) In the ill requests list tables (both, in ill module and patron ill view) are shown columns placed_unformatted and updated_unformatted, only data, table header is blank.
3) There is error in js console, when load the ill request lista table in ill module
ill-list-table_18.1200024.js:242 Uncaught TypeError: Cannot read property 'hasOwnProperty' of undefined
at getStatusName (ill-list-table_18.1200024.js:242)
at ill-list-table_18.1200024.js:34
at Array.forEach (<anonymous>)
at Object.prep (ill-list-table_18.1200024.js:29)
at a.fn.init.initComplete (ill-list-table_18.1200024.js:460)
at jquery.dataTables.min_18.1200024.js:75
at Function.map (jquery-2.2.3.min_18.1200024.js:2)
at v (jquery.dataTables.min_18.1200024.js:75)
at ta (jquery.dataTables.min_18.1200024.js:49)
at ia (jquery.dataTables.min_18.1200024.js:48)
3) when trying to filter by date placed, there is one more error in console:
ill-list-table_18.1200024.js:519 Uncaught TypeError: Cannot read property 'api' of undefined
at HTMLFormElement.<anonymous> (ill-list-table_18.1200024.js:519)
at HTMLFormElement.dispatch (jquery-2.2.3.min_18.1200024.js:3)
at HTMLFormElement.r.handle (jquery-2.2.3.min_18.1200024.js:3)
(In reply to Josef Moravec from comment #18) > Also, it would be nice to have test for changes in > Koha/REST/V1/Illrequests.pm Test in this attachmend of 21460 is exactly what I mean: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83184&action=diff we could move it here maybe... Created attachment 86504 [details] [review] Bug 18589: (follow-up) Add borrowernumber test As per comment #21: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c21 We now test borrowernumber filter works. This test has come from https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83184&action=diff and will be removed from that bug Created attachment 86506 [details] [review] Bug 18589: (follow-up) Fix merge problem Date format setting got lost in the merge As per item 1) in comment #20: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c20 Created attachment 86507 [details] [review] Bug 18589: (follow-up) Fix merge problem Bug in passing row to getStatusName As per item 3) in comment #20: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c20 (In reply to Josef Moravec from comment #20) > 1) You are setting dateFormat on two places, but not use it anywhere. Good spot, this was a merge error, fixed in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86506 > 2) In the ill requests list tables (both, in ill module and patron ill view) > are shown columns placed_unformatted and updated_unformatted, only data, > table header is blank. These are used for sorting. This is a technique I observed was used elsewhere. Since the formatted dates don't always work well for sorting, hidden unformatted date columns that are used for sorting are used instead. > 3) There is error in js console, when load the ill request lista table in > ill module > ill-list-table_18.1200024.js:242 Uncaught TypeError: Cannot read property > 'hasOwnProperty' of undefined Another merge error, fixed in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86507 > 3) when trying to filter by date placed, there is one more error in console: > ill-list-table_18.1200024.js:519 Uncaught TypeError: Cannot read property > 'api' of undefined > at HTMLFormElement.<anonymous> (ill-list-table_18.1200024.js:519) > at HTMLFormElement.dispatch (jquery-2.2.3.min_18.1200024.js:3) > at HTMLFormElement.r.handle (jquery-2.2.3.min_18.1200024.js:3) I can't replicate this. Can you make sure you do a hard refresh and try again? Created attachment 86714 [details] [review] Bug 18589: Show ILLs as part of patron profile This patch moves the display of a patron's ILL requests to be inline in the Patron Profile page, as per all other patron information. It includes a substantial refactor of koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt in that it moves the display of the illlist table (which is populated by an API request) into it's own separate include file. It also moves the datatable related Javascript for this table into it's own JS file. Doing this allows us to reuse both in both the new members/ill-requests.tt template and the pre-existing ill/ill-requests.tt template. To test: 1) Ensure ILL is enabled and your user has sufficient permissions 2) Ensure your user has some ILL requests 3) Navigate to the user's patron profile page 4) Click on the "Interlibrary loans" tab 5) Observe that the requests table is displayed inline 6) Observe that only your user's requests are displayed Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie This is essentially a reimplementation of attachment 84796 [details] [review] to take into account all changes that had happened beneath this bug and also to ensure JS strings are translatable. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86715 [details] [review] Bug 18589: (follow-up) Remove redundant-ness Following feedback from Josef via IRC, removing redundant code Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86716 [details] [review] Bug 18589: (follow-up) Fix QA tool failures As per comment #17: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c17 Sorry Josef, I honestly have no idea where those tabs came from :( Every once in a while my copy of Vim decides to embarass me and stick in tabs instead of spaces... <shakes fist at screen> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86717 [details] [review] Bug 18589: (follow-up) Add borrowernumber test As per comment #21: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c21 We now test borrowernumber filter works. This test has come from https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83184&action=diff and will be removed from that bug Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86718 [details] [review] Bug 18589: (follow-up) Fix merge problem Date format setting got lost in the merge As per item 1) in comment #20: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c20 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86719 [details] [review] Bug 18589: (follow-up) Fix merge problem Bug in passing row to getStatusName As per item 3) in comment #20: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589#c20 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86720 [details] [review] Bug 18589: (QA follow-up) Rename page to "ILL requests history" for consistancy Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86721 [details] [review] Bug 18589: (QA follow-up) Move template to bootstrap grid Test plan: Go to patrons Ill request history page and ensure the layout is not broken and is consistant with other patron pages (except Circulation and Details pages) Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 86722 [details] [review] Bug 18589: (QA follow-up) Remove dateFormat from javascript on ill requests pages. This is not needed, the right formatting is done in calendar.inc Test plan: Use datepickers on ILL requests list page and ensure they work as expected and the list is right filterred Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Ad changes in Koha/REST/V1/Illrequests.pm: Working with parameters should use objects search plugin and patron_id instead of borrowernumber. But such changes are out of scope of this bug and will be done on bug 22440 and after ILL request api rfc is voted. So, I would consider this as Passed QA Just would like somebody to test and sign off on my three follow-ups Created attachment 86735 [details] [review] Bug 18589: (QA follow-up) Rename page to "ILL requests history" for consistancy Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 86736 [details] [review] Bug 18589: (QA follow-up) Move template to bootstrap grid Test plan: Go to patrons Ill request history page and ensure the layout is not broken and is consistant with other patron pages (except Circulation and Details pages) Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 86737 [details] [review] Bug 18589: (QA follow-up) Remove dateFormat from javascript on ill requests pages. This is not needed, the right formatting is done in calendar.inc Test plan: Use datepickers on ILL requests list page and ensure they work as expected and the list is right filterred Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Thanks Andrew, so now I am passing it
Awesome work all! Pushed to master for 19.05 Enhancement will not be backported to 18.11.x series. |