Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt
Summary: DataTables Warning: Requested unknown parameter from opac-detail.tt
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Bouzid Fergani
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-21 18:56 UTC by Rémi Mayrand-Provencher
Modified: 2017-12-07 22:16 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.42 KB, patch)
2016-01-21 19:02 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.47 KB, patch)
2016-01-21 20:00 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.56 KB, patch)
2016-04-12 13:29 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.43 KB, patch)
2016-05-04 13:08 UTC, Blou
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.47 KB, patch)
2016-07-07 03:19 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 15636 - DataTables Warning: Requested unknown parameter from opac-detail.tt (1.67 KB, patch)
2016-07-07 14:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Rémi Mayrand-Provencher 2016-01-21 18:56:19 UTC
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.
Comment 1 Rémi Mayrand-Provencher 2016-01-21 19:02:05 UTC Comment hidden (obsolete)
Comment 2 Rémi Mayrand-Provencher 2016-01-21 20:00:48 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2016-01-21 21:37:39 UTC
Hi Remi, can you please add a test plan to your commit message?
Comment 4 Blou 2016-01-21 21:54:25 UTC
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...
Comment 5 Marc Véron 2016-03-23 15:32:28 UTC
(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
Comment 6 Jonathan Druart 2016-04-11 09:28:48 UTC
I do not find any occurrence of singleBranchModel, I suspect a typo (for singleBranchMode).
Comment 7 Bouzid Fergani 2016-04-12 13:29:32 UTC Comment hidden (obsolete)
Comment 8 Bouzid Fergani 2016-04-12 13:39:15 UTC
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.
Comment 9 Owen Leonard 2016-04-12 14:25:05 UTC
Please include a test plan in your commit message.
Comment 10 Blou 2016-05-04 13:08:25 UTC Comment hidden (obsolete)
Comment 11 Blou 2016-05-04 13:21:18 UTC
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...
Comment 12 Srdjan Jankovic 2016-07-07 03:19:14 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-07-07 14:55:03 UTC
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.
Comment 14 Kyle M Hall 2016-07-08 13:03:04 UTC
Pushed to master for 16.11, thanks Bouzid!
Comment 15 Frédéric Demians 2016-08-01 13:25:47 UTC
Pushed in 16.05. Will be in 16.05.02.
Comment 16 Julian Maurice 2016-08-16 14:51:10 UTC
Patch pushed to 3.22.x, will be in 3.22.10