One of the patches done during the QA process on Bug 18589 moved the strings into a separate file "ill-list-table-strings.inc", an include for this file was added to "ill-requests.tt". This bug, being an enhancement, was not backported to 18.11.x. Some months later, Bug 23529 made some changes to that template, which contained the include, which was by this time in master. Bug 23529 *was* backported to 18.11.x, along with the include. This breaks since the include is referring to a file that never made it into 18.11.x. The original bug, 18589, was definitely an enhancement, so there's no point in trying to backport it. The fix is to just remove the include.
Created attachment 95112 [details] [review] Bug 23980: Remove non-existent include This patch removes the erroneous include
Test plan: - Do not apply the patch - Go to /cgi-bin/koha/ill/ill-requests.pl => TEST: Observe that you receive the error: "Template process failed: file error - ill-list-table-strings.inc: not found at ..." - Apply the patch - Refresh the page => TEST: Observe that you no longer receive the error
Looking at this further, I have noticed that the request for: [% Asset.js("js/ill-list-table.js") | $raw %] is also incorrect since, in 18.11.x, the JS has not been moved out of the template. I will create another patch to address this
OK, I *think* I know what's going on. Bug 23229 should never have been backported to 18.11.x since it contains patches that don't correspond with the ILL template in that version and they add file includes to files that don't exist in 18.11.x. Fortunately, there have been no commits to ill-requests.tt since this was backported, so it should be just a case of reverting the commits. I'll add Lucas to this, since he did the backport of 23229
Andrew, Rmaint for 18.11 is no longer Lucas. I suggest you to provide the revert commits here with a test plan, and follow the usual workflow for integration. Better than letting the RMaint dealing with that.
Thanks for the heads up Jonathan, I'd actually forgotten about this as Lucas said he would revert, I guess he never got a chance. "...follow the usual workflow for integration." - what would that be, apart from me creating the revert patches?
Created attachment 96191 [details] [review] Revert "Bug 23229: (QA follow-up) Fix indentation" This reverts commit d4676ff4532f41abec87ba2d1128e3f219d9c920.
Created attachment 96192 [details] [review] Revert "Bug 23229: Only load ILL table JS when needed" This reverts commit f95749a66bb5dad53fa2cf00224fc38f52c1e788.
Test plan: - Do not apply the patches - Go to /cgi-bin/koha/ill/ill-requests.pl => TEST: Observe that you receive the error: "Template process failed: file error - ill-list-table-strings.inc: not found at ..." - Apply the patches - Refresh the page => TEST: Observe that you no longer receive the error
(In reply to Andrew Isherwood from comment #6) > "...follow the usual workflow for integration." - what would that be, apart > from me creating the revert patches? Patch, submit, SO, ... pushed! :)
Riiiiight, I assumed there was more to it for cases like this :-D
Hi, is this still valid? I tested using 18.11.14 and error message from test plan does not show.
Some weirdness going on here. The error is a result of the "jsinclude" block (which should not be there in 18.11, and which this bug removed) being parsed by TT. jsinclude blocks should only be parsed if the "footerjs" flag is set prior to the "jsinclude" block in the mark up. As far as I can see, the 18.11 version of this template has *never* included a footerjs flag, so I don't see how the jsinclude block can ever have been parsed and the resulting error displayed (scratches head). So, you're right Bernardo, the error is not being displayed. However, the jsinclude block should not be present in the template because it references files that do not exist in 18.11. So we still need to get the patches in this bug merged. I think the only test plan I can offer in that case is: - Do not apply the patch - View koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt - TEST: Verify that it includes a "jsinclude" block near the bottom of the template - Apply the patch - Re-open the template - TEST: Verify that the "jsinclude" block is now gone
Created attachment 99567 [details] [review] Bug 23980: Revert "Bug 23229: (QA follow-up) Fix indentation" This reverts commit d4676ff4532f41abec87ba2d1128e3f219d9c920. https://bugs.koha-community.org/show_bug.cgi?id=23980 Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit title
Created attachment 99568 [details] [review] Bug 23980: Revert "Bug 23229: Only load ILL table JS when needed" This reverts commit f95749a66bb5dad53fa2cf00224fc38f52c1e788. https://bugs.koha-community.org/show_bug.cgi?id=23980 Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit title, no errors Revised test plan: - Do not apply the patch - View koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt - TEST: Verify that it includes a "jsinclude" block near the bottom of the template - Apply the patch - Re-open the template - TEST: Verify that the "jsinclude" block is now gone
Created attachment 100109 [details] [review] Bug 23980: Revert "Bug 23229: (QA follow-up) Fix indentation" This reverts commit d4676ff4532f41abec87ba2d1128e3f219d9c920. https://bugs.koha-community.org/show_bug.cgi?id=23980 Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit title Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 100110 [details] [review] Bug 23980: Revert "Bug 23229: Only load ILL table JS when needed" This reverts commit f95749a66bb5dad53fa2cf00224fc38f52c1e788. https://bugs.koha-community.org/show_bug.cgi?id=23980 Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit title, no errors Revised test plan: - Do not apply the patch - View koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt - TEST: Verify that it includes a "jsinclude" block near the bottom of the template - Apply the patch - Re-open the template - TEST: Verify that the "jsinclude" block is now gone Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Backported to 18.11.x for 18.11.15