the jQuery that generates the DataTable for the holdings (in opac-detail.tt) can bug depending on the instance's system preferences. The header's columns are generated depending on various system preferences, but the jquery that tries to build the DataTable is using a different test for one of the columns, so it isn't calculating how many columns it has to build right.
Created attachment 47141 [details] [review] Bug15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt Fixes the jQuery that builds the holdingst DataTable in opac-detail.tt
Created attachment 47142 [details] [review] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt Fixes the jQuery that generates the holdingst DataTable by adding the missing UNLESS and IF tests to it. The IF and UNLESS in opac-detail.tt need to be the same in the header declaration of the DataTable and the jquery that builds it or else there can be an error depening on the system preferences.
Hi Remi, can you please add a test plan to your commit message?
Hello Cate, We'll need your help on that. You see, we cannot reproduce the situation without hacking the code. It arose because we're adding an extra column in there for ebooks, in a custom development. It causes a jqury message to pop up in the UI because of the imbalance in column count. The origin of it is commit 6d32d66f, by bug #7720. You can see in there that it's missing the javascript part for the test. So, not sure how to test it. Except that by looking at the code (and 7720's code), it's "obvious". Just need a way to translate it into a scenario...
(In reply to Blou from comment #4) > Hello Cate, > We'll need your help on that. You see, we cannot reproduce the situation > without hacking the code. It arose because we're adding an extra column in > there for ebooks, in a custom development. It causes a jqury message to pop > up in the UI because of the imbalance in column count. > > The origin of it is commit 6d32d66f, by bug #7720. You can see in there > that it's missing the javascript part for the test. > > So, not sure how to test it. Except that by looking at the code (and 7720's > code), it's "obvious". Just need a way to translate it into a scenario... Blou, Maybe you could add a separate patch with the "hack" that is necessary for testing (marked with something like FOR TESTING ONLY - DO NOT PUSH) The scenario would then be something like: - apply the "Hack" patch - verify the bug - apply the patch with the jQuery fix - verify that the error is gone - remove both patches and apply the jQuery fix patch only - verify that nothing is broken - sign-off the patch with the jQuery fix Marc
I do not find any occurrence of singleBranchModel, I suspect a typo (for singleBranchMode).
Created attachment 50146 [details] [review] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt
the order of the conditions is not the same in both places, which causes an error display, and the javascript stops. Now, the order is correct.
Please include a test plan in your commit message.
Created attachment 51186 [details] [review] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt
I rebased it. Now, I've asked bouzid to find a way to reproduce it, but that's low on his priority list. As he said previously, the fix is just reproducing the same conditions for the header as for the entries in the table. No need of big research to imagine a scenario where the unfixed code might break, even though it seems all of you have been lucky so far :-) So in that sense, I was under the hope this would go smoother...
Created attachment 53157 [details] [review] [SIGNED-OFF] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Created attachment 53177 [details] [review] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> This patch adds the same tests in the aoColumns DT definition than the ones in the creation of the table. That's sound good.
Pushed to master for 16.11, thanks Bouzid!
Pushed in 16.05. Will be in 16.05.02.
Patch pushed to 3.22.x, will be in 3.22.10