https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #2 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Benjamin I'm not aware of anywhere where we query that table on type or value, it's only ever the illrequest_id we query via the join. So I'm not sure of the value of adding these indexes. I might be missing something though, so please put me straight if so! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #4 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 76688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76688&action=edit Bug 20996: Fix unit tests - Remove unit tests relating to TO_JSON - Add tests for new explicit embedding - Modify tests relating to now deprecated brw_cat -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Bug 20996 depends on bug 7317, which changed state. Bug 7317 Summary: Add an Interlibrary Loan Module to Circulation and OPAC https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7317 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #7 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Since this is a performance enhancement, it's very hard to test without a substantial number of ILL requests in the database. I have therefore attached the script I created to populate large number of requests (add_ill.pl). To test: - Before applying patches, add a large number of requests, maybe 5000, using attached add_ill.pl - Make a request to: http://your-koha/api/v1/illrequests?embed=metadata,patron,capabilities,libra... - Time how long the response takes - Apply patches - Make another request to the same URL - Time the response => TEST: The response time should be substantially reduced -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #9 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Excellent point, thanks Magnus. I'll create a patch to add these -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #11 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Interesting, I'd not noticed the weight increase. I'll investigate! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #13 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- It's not related to this bug, it's due to an item not having an author, I've seen it before. I'll create a bug if I've not already created one. Thanks for mentioning it! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #14 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 79379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79379&action=edit Bug 20996: (follow-up) Add <branch> to conf tpl Since we now require the <branch> block, we should add it to the config templates -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #16 from Magnus Enger <magnus@libriotech.no> --- No bug 21079 applied, no. At least I think I found the difference in the JSON delivered, thanks to the "Response preview" in the Chromium developer tools. Without the patches applied, each request described in the JSON data has a data element "metadata" which contains just the properties that are set up by "sub metadata" in Base.pm. With the patches applied, this same data element contains *all* the request attributes, but not the data from "sub metadata" (this results in Title and Author not being populated in the table, as well as the errors/warnings I mentioned earlier). I have not looked into what is causing this difference. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18589 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589 [Bug 18589] Show ILLs as part of patron profile -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #19 from Magnus Enger <magnus@libriotech.no> --- Not quite that simple, I'm afraid... When I am on the master branch everything looks fine. Title and Author are displayed for the requests that have them. When I apply the patches from this bug title and author are not displayed, and this warning pops up, both on initial rendering of the "View ILL requests" page and whenever I use the Next and Previous links in the datatable to see more requests. DataTables warning: table id=ill-requests - Requested unknown parameter 'metadata_Author' for row 367, column 0. For more information about this error, please see http://datatables.net/tn/4 Could this be because we are only getting the full requestattributes, and not the data from "sub metadata"? I am testing with my in-progress backend for Libris, so I guess this might be an issue with that backend. Especially if things work as they should for you. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #21 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- OK, I know what it is. In the FreeForm backend's metadata method (and, I presume, your backend's metadata method), there is this line: https://github.com/PTFS-Europe/koha-ill-freeform/blob/master/Base.pm#L124 Which, as you can see, title cases the name of the property (Author etc), which breaks the JS when we use the alternative method of getting our metadata, which doesn't title case it. I've no idea why the properties are title cased, but it seems to have been the case since day 1. I'm going to fix it by changing what the JS expects from "metadata_Author" to "metadata_author", same for title, and leave it at that. This whole issue is made irrelevant by bug 21063 which changes how the datatable JS works anyway and has no expectation of column names from the API. I'll attach a patch momentarily. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #22 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 79576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79576&action=edit Bug 20996: (follow-up) Fix column name expectation This bug changes how request metadata is obtained. Previously the appropriate backend's metadata function would return it. For at least the FreeForm backend, the metadata property names we title cased. The datatable JS was expecting this and breaks with the new metadata getting which is performed in the API controller, which just returns the property names as they appear in the DB. This patch changes the expectation of the JS to match what's coming back from the API -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76687|0 |1 is obsolete| | --- Comment #23 from Magnus Enger <magnus@libriotech.no> --- Created attachment 79583 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79583&action=edit Bug 20996: Remove prefix use of borrower category This patch removes the potential use of borrower category as a ILL request ID prefix. It makes no sense. We provide the ability for a site to define a request prefix based on branch, there is no use case for using the borrower category. Add to this that the borrower for every request was being retrieved in order to get the category, it's a huge performance hit also. We also now require the <branch> block in the <interlibrary_loans> block and complain if it's not present. The request prefix should be defined in this block. This patch also improves the performance of the API request that returns all requests, optionally including additional data. It also deprecates the overloaded TO_JSON method and moves the request augmentation code into the API route's controller. It may be that we want to shift it out of there at some point, but it is fine where it is for now. Signed-off-by: Magnus Enger <magnus@enger.priv.no> https://bugs.koha-community.org/show_bug.cgi?id=20797 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76705|0 |1 is obsolete| | --- Comment #25 from Magnus Enger <magnus@libriotech.no> --- Created attachment 79585 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79585&action=edit Bug 20996: Further enhance performance We were immediately unblessing all request objects after retrieval then, at a later stage, re-instantiating each one so we could call methods on that. This was a very bad idea. We now preserve the objects for as long as we can. Signed-off-by: Magnus Enger <magnus@enger.priv.no> https://bugs.koha-community.org/show_bug.cgi?id=20797 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79576|0 |1 is obsolete| | --- Comment #27 from Magnus Enger <magnus@libriotech.no> --- Created attachment 79587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79587&action=edit Bug 20996: (follow-up) Fix column name expectation This bug changes how request metadata is obtained. Previously the appropriate backend's metadata function would return it. For at least the FreeForm backend, the metadata property names we title cased. The datatable JS was expecting this and breaks with the new metadata getting which is performed in the API controller, which just returns the property names as they appear in the DB. This patch changes the expectation of the JS to match what's coming back from the API Signed-off-by: Magnus Enger <magnus@enger.priv.no> Tested by repeatedly loading the API-call http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,librar... in Chromium and watching the reported time to load the URL. There is a siginificant decrease in the time with the patches compared to the time it takes without the patches. I had some concerns along the way, but they have all been fixed. https://bugs.koha-community.org/show_bug.cgi?id=20797 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79584|0 |1 is obsolete| | --- Comment #29 from Magnus Enger <magnus@libriotech.no> --- Created attachment 79589 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79589&action=edit Bug 20996: Fix unit tests - Remove unit tests relating to TO_JSON - Add tests for new explicit embedding - Modify tests relating to now deprecated brw_cat Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79586|0 |1 is obsolete| | --- Comment #31 from Magnus Enger <magnus@libriotech.no> --- Created attachment 79591 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79591&action=edit Bug 20996: (follow-up) Add <branch> to conf tpl Since we now require the <branch> block, we should add it to the config templates Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21079 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21079 [Bug 21079] Unify metadata schema across backends -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20548 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20548 [Bug 20548] Remove copyright clearance workflow from staff created ILL requests -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79588|0 |1 is obsolete| | Attachment #79589|0 |1 is obsolete| | Attachment #79590|0 |1 is obsolete| | Attachment #79591|0 |1 is obsolete| | Attachment #79592|0 |1 is obsolete| | --- Comment #33 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81715 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81715&action=edit Bug 20996: Remove prefix use of borrower category This patch removes the potential use of borrower category as a ILL request ID prefix. It makes no sense. We provide the ability for a site to define a request prefix based on branch, there is no use case for using the borrower category. Add to this that the borrower for every request was being retrieved in order to get the category, it's a huge performance hit also. We also now require the <branch> block in the <interlibrary_loans> block and complain if it's not present. The request prefix should be defined in this block. This patch also improves the performance of the API request that returns all requests, optionally including additional data. It also deprecates the overloaded TO_JSON method and moves the request augmentation code into the API route's controller. It may be that we want to shift it out of there at some point, but it is fine where it is for now. Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #35 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81717&action=edit Bug 20996: Further enhance performance We were immediately unblessing all request objects after retrieval then, at a later stage, re-instantiating each one so we could call methods on that. This was a very bad idea. We now preserve the objects for as long as we can. Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #37 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81719 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81719&action=edit Bug 20996: (follow-up) Fix column name expectation This bug changes how request metadata is obtained. Previously the appropriate backend's metadata function would return it. For at least the FreeForm backend, the metadata property names we title cased. The datatable JS was expecting this and breaks with the new metadata getting which is performed in the API controller, which just returns the property names as they appear in the DB. This patch changes the expectation of the JS to match what's coming back from the API Signed-off-by: Magnus Enger <magnus@enger.priv.no> Tested by repeatedly loading the API-call http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,librar... in Chromium and watching the reported time to load the URL. There is a siginificant decrease in the time with the patches compared to the time it takes without the patches. I had some concerns along the way, but they have all been fixed. Signed-off-by: Magnus Enger <magnus@libriotech.no> Tested by repeatedly loading the API-call http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,librar... in Chromium and watching the reported time to load the URL. There is a siginificant decrease in the time with the patches compared to the time it takes without the patches. I had some concerns along the way, but they have all been fixed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #39 from Josef Moravec <josef.moravec@gmail.com> --- Rebased on master and add small follow-up -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81715|0 |1 is obsolete| | --- Comment #41 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Comment on attachment 81715 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81715 Bug 20996: Remove prefix use of borrower category One of these patches in the rebased set is failing with "error: sha1 information is lacking or useless (Koha/Illrequest.pm)", so I'm obsoleting them all, un-obsoleting their predecessors, then going to rebase and re-attach patches (including josef's POD fix) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81717|0 |1 is obsolete| | --- Comment #43 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Comment on attachment 81717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81717 Bug 20996: Further enhance performance One of these patches in the rebased set is failing with "error: sha1 information is lacking or useless (Koha/Illrequest.pm)", so I'm obsoleting them all, un-obsoleting their predecessors, then going to rebase and re-attach patches (including josef's POD fix) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81719|0 |1 is obsolete| | --- Comment #45 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Comment on attachment 81719 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81719 Bug 20996: (follow-up) Fix column name expectation One of these patches in the rebased set is failing with "error: sha1 information is lacking or useless (Koha/Illrequest.pm)", so I'm obsoleting them all, un-obsoleting their predecessors, then going to rebase and re-attach patches (including josef's POD fix) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79588|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79590|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79592|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79589|0 |1 is obsolete| | --- Comment #48 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 81748 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81748&action=edit Bug 20996: Fix unit tests - Remove unit tests relating to TO_JSON - Add tests for new explicit embedding - Modify tests relating to now deprecated brw_cat Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79591|0 |1 is obsolete| | --- Comment #50 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 81750 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81750&action=edit Bug 20996: (follow-up) Add <branch> to conf tpl Since we now require the <branch> block, we should add it to the config templates Signed-off-by: Magnus Enger <magnus@libriotech.no> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #52 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 81752 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81752&action=edit Bug 20996: (follow-up) Fix merge problems Merge conflicts resolved -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #54 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Rebased, fixed merge problems (including the "comments" embed in the API response) and added Josef's POD fix. Ready for QA again -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81747|0 |1 is obsolete| | Attachment #81748|0 |1 is obsolete| | Attachment #81749|0 |1 is obsolete| | Attachment #81750|0 |1 is obsolete| | Attachment #81751|0 |1 is obsolete| | Attachment #81752|0 |1 is obsolete| | Attachment #81753|0 |1 is obsolete| | --- Comment #55 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81755 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81755&action=edit Bug 20996: Remove prefix use of borrower category This patch removes the potential use of borrower category as a ILL request ID prefix. It makes no sense. We provide the ability for a site to define a request prefix based on branch, there is no use case for using the borrower category. Add to this that the borrower for every request was being retrieved in order to get the category, it's a huge performance hit also. We also now require the <branch> block in the <interlibrary_loans> block and complain if it's not present. The request prefix should be defined in this block. This patch also improves the performance of the API request that returns all requests, optionally including additional data. It also deprecates the overloaded TO_JSON method and moves the request augmentation code into the API route's controller. It may be that we want to shift it out of there at some point, but it is fine where it is for now. Signed-off-by: Magnus Enger <magnus@libriotech.no> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #57 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81757&action=edit Bug 20996: Further enhance performance We were immediately unblessing all request objects after retrieval then, at a later stage, re-instantiating each one so we could call methods on that. This was a very bad idea. We now preserve the objects for as long as we can. Signed-off-by: Magnus Enger <magnus@libriotech.no> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #59 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81759&action=edit Bug 20996: (follow-up) Fix column name expectation This bug changes how request metadata is obtained. Previously the appropriate backend's metadata function would return it. For at least the FreeForm backend, the metadata property names we title cased. The datatable JS was expecting this and breaks with the new metadata getting which is performed in the API controller, which just returns the property names as they appear in the DB. This patch changes the expectation of the JS to match what's coming back from the API Signed-off-by: Magnus Enger <magnus@enger.priv.no> Tested by repeatedly loading the API-call http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,librar... in Chromium and watching the reported time to load the URL. There is a siginificant decrease in the time with the patches compared to the time it takes without the patches. I had some concerns along the way, but they have all been fixed. Signed-off-by: Magnus Enger <magnus@libriotech.no> Tested by repeatedly loading the API-call http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,librar... in Chromium and watching the reported time to load the URL. There is a siginificant decrease in the time with the patches compared to the time it takes without the patches. I had some concerns along the way, but they have all been fixed. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #61 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 81761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81761&action=edit Bug 20996: (QA follow-up) Fix pod Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|7317 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7317 [Bug 7317] Add an Interlibrary Loan Module to Circulation and OPAC -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #63 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- t/db_dependent/Illrequest/Config.t is failing. New code added to Koha/Illrequest/Config.pm but no tests? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #65 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 81909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81909&action=edit Bug 20966: (follow-up) Add template variable filter; Fix table export This patch makes to corrections: 1. The "$raw" filter has been added to the column configuration 2. The columns configuration now specifies which columns are exported in Copy, Excel, etc. operations. To test, apply the patch and test the various table export buttons on a both a closed and open basket. Confirm that the 'Modify' and 'Cancel' columns are not exported. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #81909|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #67 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Jonathan Druart from comment #63)

t/db_dependent/Illrequest/Config.t is failing.

New code added to Koha/Illrequest/Config.pm but no tests?

I added a follow-up patch with test changes -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #69 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 18.11(In reply to Tomás Cohen Arazi from comment #68)

Created attachment 81967 [details] [review] Bug 20996: (follow-up) Fix test

- Fixes the t/db_dependent/Illrequest/Config.t - Adds tests for Koha::Illrequest::Config->has_branch

0) Do not apply the path 1) Run test t/db_dependent/Illrequest/Config.t 2) It fails - it is because the Koha/Illrequest/Config.pm prefixes handling changed 3) Apply the patch 4) Run test again, it should pass now 5) Read through the test code and confirm it makes sense

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

Followup pushed to master for 18.11 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #71 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Added a follow-up for the RM that should fix the last remaining failing test. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #73 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I get a warning with the last patch: t/db_dependent/api/v1/illrequests.t .. Un-mocked method 'status_graph()' called at /home/vagrant/kohaclone/Koha/Illrequest.pm line 439. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #75 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 81998 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81998&action=edit Bug 20996: Remove warning 'Un-mocked method' t/db_dependent/api/v1/illrequests.t .. Un-mocked method 'status_graph()' called at /home/vagrant/kohaclone/Koha/Illrequest.pm line 439. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 --- Comment #77 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Odd.. I didn't get that warning at all when I tested the patch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20996 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21835 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21835 [Bug 21835] Request ID is displayed as NaN -- You are receiving this mail because: You are watching all bug changes.