Bug 34913 - Upgrade DataTables from 1.10.18 to 1.13.6
Summary: Upgrade DataTables from 1.10.18 to 1.13.6
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Martin Renvoize
URL: https://gitlab.com/joubu/Koha/-/commi...
Keywords:
Depends on:
Blocks: 28058 33568 35249 35922 36035 36563 36640
  Show dependency treegraph
 
Reported: 2023-09-26 12:56 UTC by Martin Renvoize
Modified: 2024-04-26 20:28 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02,23.05.10


Attachments
Bug 34913: Fix a selenium test (1.01 KB, patch)
2023-10-03 07:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34913: Move item groups table to kohaTable (3.21 KB, patch)
2023-10-20 12:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34913: Adjust C4::Utils::DataTables::VirtualShelves (25.06 KB, patch)
2023-10-24 08:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34913: Adjust "Manage staged MARC records" (3.85 KB, patch)
2023-10-24 08:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34913: WIP - item search (4.95 KB, patch)
2023-10-24 09:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34913: Fix Activate/Deactive filters with table_filters.js (1.15 KB, patch)
2023-11-02 01:38 UTC, David Cook
Details | Diff | Splinter Review
Bug 34913: Fix perlcritic for VirtualShelves.pm (935 bytes, patch)
2024-01-29 09:42 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 Martin Renvoize 2023-09-26 12:56:01 UTC
A recent penetration test found hat the version of Datatables we're using is vulnerable and should be updated to the latest release.

It did not however, detail an official CVE.
Comment 2 David Cook 2023-09-26 23:47:49 UTC
It looks like a newer bundle of DataTables would also fix a Content-Security-Policy issue I talk about in bug 20397
Comment 3 Martin Renvoize 2023-09-27 13:49:53 UTC
Excellent, thats good news.. 

Also, thanks for looking into this Owen.. just wanted to show my appreciation and support.
Comment 4 Owen Leonard 2023-09-28 17:57:16 UTC
Here's remote branch, ready for testing, which updates the staff interface:

https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913

It touches 126 files... Would it be better if I split it up into smaller patches?
Comment 5 Jonathan Druart 2023-10-03 07:17:56 UTC
I have not found any regressions.
The only thing I noticed is the 3 blue dots when a table is loading (so when using the REST API). We do not have much blue on the UI, could be nice to adjust or remove. Not blocker however.
Comment 6 Jonathan Druart 2023-10-03 07:20:56 UTC
(Note that it's not saying "Loading" but "Processing")
Comment 7 Jonathan Druart 2023-10-03 07:31:20 UTC
Created attachment 156480 [details] [review]
Bug 34913: Fix a selenium test

t/db_dependent/selenium/administration_tasks.t ....... 3/3 Cannot show all entries from table //div[@id="libraries_wrapper"] at /kohadevbox/koha/t/lib/Selenium.pm line 247.
Comment 8 Jonathan Druart 2023-10-04 08:06:27 UTC
I am wondering if we should/can add a check in the QA script to catch introduction of old Hungarian notation to prevent new occurrences. That does not seem as easy as listing them all and grep the different words.

For instance
var is_searchable = table_dt.settings()[0].aoColumns[i].bSearchable;

bSearchable can be replaced with seachable

But aoColumns[1] cannot (simply) be replaced with columns[1].

There are still a lot of occurrences of Hungarian notation after this patch, should not we replace them all?
Comment 9 Owen Leonard 2023-10-06 14:58:42 UTC
(In reply to Jonathan Druart from comment #8)

> There are still a lot of occurrences of Hungarian notation after this patch,
> should not we replace them all?

Eventually I think we should, but this patch specifically addresses the problem we will face with in-page configurations not playing well with our default configuration. If I have missed cases of that please let me know or provide a follow-up.

Changing the use of these variables in other places is a much bigger task with more complicated testing requirements.
Comment 10 Martin Renvoize 2023-10-12 16:14:25 UTC
This is looking great to me.. also can't spot any regressions and the selenium test works for me after Jonathans patch.

I'm happy for you to add my Signoff.
Comment 11 Katrin Fischer 2023-10-14 16:58:09 UTC
I also couldn't spot any regressions. I filed 3 bugs for stuff I found in the process of testing, but none of these were related to this patch set :D
Comment 12 Katrin Fischer 2023-10-14 16:59:44 UTC
Can you please add my SO lines? :) 

I tested with the patch here on top and rebasd on current master.
Comment 13 Martin Renvoize 2023-10-17 12:09:12 UTC
Awesome to see this in PQA :)
Comment 14 Jonathan Druart 2023-10-20 12:53:30 UTC
Created attachment 157511 [details] [review]
Bug 34913: Move item groups table to kohaTable
Comment 15 Owen Leonard 2023-10-20 13:17:59 UTC
The remote branch has been updated (https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913) but the bug fixed in Comment 14 affects other pages too:

- Item search
- Lists
- Manage staged MARC records (batch view)
Comment 16 Jonathan Druart 2023-10-24 07:29:38 UTC
(In reply to Owen Leonard from comment #15)
> The remote branch has been updated
> (https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913) but the bug fixed
> in Comment 14 affects other pages too:
> 
> - Item search
> - Lists
> - Manage staged MARC records (batch view)

Those are messy. We need to adjust the controller scripts... Not trivial.
Comment 17 Jonathan Druart 2023-10-24 08:27:11 UTC
Created attachment 157725 [details] [review]
Bug 34913: Adjust C4::Utils::DataTables::VirtualShelves

And remove C4::Utils::DataTables, which should no longer be reused
anyway.
Comment 18 Jonathan Druart 2023-10-24 08:39:33 UTC
Created attachment 157726 [details] [review]
Bug 34913: Adjust "Manage staged MARC records"
Comment 19 Jonathan Druart 2023-10-24 09:26:02 UTC
Created attachment 157729 [details] [review]
Bug 34913: WIP - item search

This is not working, the columnfilter plugin seems to break the sorting.
If you sort twice on a colum, the request will be GET (instead of
POSTed, which is the historical behaviour) and columns will contain
[object Object]

There are more to do here, but this problem blocks the other steps.
Comment 20 Jonathan Druart 2023-10-24 09:26:26 UTC
(In reply to Jonathan Druart from comment #19)
> Created attachment 157729 [details] [review] [review]
> Bug 34913: WIP - item search
> 
> This is not working, the columnfilter plugin seems to break the sorting.
> If you sort twice on a colum, the request will be GET (instead of
> POSTed, which is the historical behaviour) and columns will contain
> [object Object]
> 
> There are more to do here, but this problem blocks the other steps.

I am stuck on this one, asking for help.
Comment 21 David Cook 2023-10-25 00:05:45 UTC
(In reply to Jonathan Druart from comment #20)
> (In reply to Jonathan Druart from comment #19)
> > Created attachment 157729 [details] [review] [review] [review]
> > Bug 34913: WIP - item search
> > 
> > This is not working, the columnfilter plugin seems to break the sorting.
> > If you sort twice on a colum, the request will be GET (instead of
> > POSTed, which is the historical behaviour) and columns will contain
> > [object Object]
> > 
> > There are more to do here, but this problem blocks the other steps.
> 
> I am stuck on this one, asking for help.

Is that the ColVis plugin? It looks like it's been retired: https://datatables.net/extensions/colvis/

I'm wondering if this bug needs to be broken up into a few different more manageable chunks?
Comment 22 Jonathan Druart 2023-10-25 05:48:52 UTC
(In reply to David Cook from comment #21)
> (In reply to Jonathan Druart from comment #20)
> > (In reply to Jonathan Druart from comment #19)
> > > Created attachment 157729 [details] [review] [review] [review] [review]
> > > Bug 34913: WIP - item search
> > > 
> > > This is not working, the columnfilter plugin seems to break the sorting.
> > > If you sort twice on a colum, the request will be GET (instead of
> > > POSTed, which is the historical behaviour) and columns will contain
> > > [object Object]
> > > 
> > > There are more to do here, but this problem blocks the other steps.
> > 
> > I am stuck on this one, asking for help.
> 
> Is that the ColVis plugin? It looks like it's been retired:
> https://datatables.net/extensions/colvis/

No, columFilter (filters on the top of the colums).

> I'm wondering if this bug needs to be broken up into a few different more
> manageable chunks?

It's the only missing bit apparently, better spending time on fixing it than splitting everything...
Comment 23 David Cook 2023-10-25 22:48:23 UTC
(In reply to Jonathan Druart from comment #22)
> (In reply to David Cook from comment #21)
> > (In reply to Jonathan Druart from comment #20)
> > > I am stuck on this one, asking for help.
> > 
> > Is that the ColVis plugin? It looks like it's been retired:
> > https://datatables.net/extensions/colvis/
> 
> No, columFilter (filters on the top of the colums).

Ahhh ok. 

--

So I've checked out https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913 and then applied these Bugzilla patches...

koha-plack --reload kohadev

perl build-resources.PL

When I visit http://localhost:8081/cgi-bin/koha/mainpage.pl, I'm getting this error:

DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'lang' in 'where clause' at /kohadevbox/koha/Koha/Objects.pm line 426
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

But I can go to http://localhost:8081/cgi-bin/koha/catalogue/itemsearch.pl
Comment 24 David Cook 2023-10-25 23:20:21 UTC
(In reply to Jonathan Druart from comment #19)
> This is not working, the columnfilter plugin seems to break the sorting.
> If you sort twice on a colum, the request will be GET (instead of
> POSTed, which is the historical behaviour) and columns will contain
> [object Object]
> 
> There are more to do here, but this problem blocks the other steps.

This one is super interesting.

The initial load uses POST and sends form data. I've got a breakpoint in the Javascript which shows me it's running our "data" handler in the "ajax" object.

I click the column to sort and the breakpoint is triggered again, but this time it's a GET and sends query string data. 

I click the column a 2nd time to get the sort to change and the breakpoint is not activated, a GET request is sent with the columns containing [object Object], and the dataTable just hangs.

--

Looking at ./koha-tmpl/intranet-tmpl/lib/jquery/plugins/jquery.dataTables.columnFilter.js it looks like it's very old code.

It looks like we don't have an entry for it in http://localhost:8081/cgi-bin/koha/about.pl

It looks like it's an abandoned project.

This person claims to have fixed some issues with it but that was also 9 years ago: https://github.com/RobinDev/jquery.dataTables.columnFilter.js

I think we might have to either remove this plugin or fork it...
Comment 25 Jonathan Druart 2023-10-26 05:47:15 UTC
(In reply to David Cook from comment #23)
> (In reply to Jonathan Druart from comment #22)
> > (In reply to David Cook from comment #21)
> > > (In reply to Jonathan Druart from comment #20)
> > > > I am stuck on this one, asking for help.
> > > 
> > > Is that the ColVis plugin? It looks like it's been retired:
> > > https://datatables.net/extensions/colvis/
> > 
> > No, columFilter (filters on the top of the colums).
> 
> Ahhh ok. 
> 
> --
> 
> So I've checked out https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913
> and then applied these Bugzilla patches...
> 
> koha-plack --reload kohadev
> 
> perl build-resources.PL
> 
> When I visit http://localhost:8081/cgi-bin/koha/mainpage.pl, I'm getting
> this error:
> 
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Unknown column 'lang' in 'where clause' at
> /kohadevbox/koha/Koha/Objects.pm line 426
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77

Not related with this patch set.

(In reply to David Cook from comment #24)
> (In reply to Jonathan Druart from comment #19)
> > This is not working, the columnfilter plugin seems to break the sorting.
> > If you sort twice on a colum, the request will be GET (instead of
> > POSTed, which is the historical behaviour) and columns will contain
> > [object Object]
> > 
> > There are more to do here, but this problem blocks the other steps.
> 
> This one is super interesting.
> 
> The initial load uses POST and sends form data. I've got a breakpoint in the
> Javascript which shows me it's running our "data" handler in the "ajax"
> object.
> 
> I click the column to sort and the breakpoint is triggered again, but this
> time it's a GET and sends query string data. 
> 
> I click the column a 2nd time to get the sort to change and the breakpoint
> is not activated, a GET request is sent with the columns containing [object
> Object], and the dataTable just hangs.
> 
> --
> 
> Looking at
> ./koha-tmpl/intranet-tmpl/lib/jquery/plugins/jquery.dataTables.columnFilter.
> js it looks like it's very old code.
> 
> It looks like we don't have an entry for it in
> http://localhost:8081/cgi-bin/koha/about.pl
> 
> It looks like it's an abandoned project.
> 
> This person claims to have fixed some issues with it but that was also 9
> years ago: https://github.com/RobinDev/jquery.dataTables.columnFilter.js
> 
> I think we might have to either remove this plugin or fork it...

Good catch, this version fixed the problem we faced.

I am suggesting to go with this version for now. It's not conceivable to remove the column filtering, we rely on it in several places.
Comment 26 Jonathan Druart 2023-10-26 05:50:23 UTC
No sure what is going on right now but I don't manage to attach patches to bugzilla.

I have pushed a remote branch - https://gitlab.com/joubu/Koha/-/commits/bug_34913

Ready for testing!
Comment 27 David Cook 2023-10-26 22:55:20 UTC
(In reply to Jonathan Druart from comment #25)
> Good catch, this version fixed the problem we faced.
> 
> I am suggesting to go with this version for now. It's not conceivable to
> remove the column filtering, we rely on it in several places.

(In reply to Jonathan Druart from comment #26)
> Ready for testing!

Sounds like a plan to me. The problem with item search certainly seems to be fixed. I'm trying to find other pages with column filtering so I can test them too...

Column filtering in lists looks fine: http://localhost:8081/cgi-bin/koha/virtualshelves/shelves.pl?op=list&public=0

Acquisitions also looks good: http://localhost:8081/cgi-bin/koha/acqui/parcel.pl?invoiceid=2

--

Found a typo causing a Javascript error:

koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt : 2348
"pagingType""full",
Comment 28 Jonathan Druart 2023-10-27 05:31:46 UTC
(In reply to David Cook from comment #27)
> koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt :
> 2348
> "pagingType""full",

Fixed (on my branch, in Owen's commit)
Comment 29 David Cook 2023-10-27 06:08:19 UTC
(In reply to Jonathan Druart from comment #28)
> (In reply to David Cook from comment #27)
> > koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt :
> > 2348
> > "pagingType""full",
> 
> Fixed (on my branch, in Owen's commit)

Looks good to me.

I think I'm happy to move this to Signed Off
Comment 30 Victor Grousset/tuxayo 2023-10-28 22:31:42 UTC
Is the vulnerability particularly bad of are those XSS just a subpart of a number others currently in Koha?

To know about how bad it's not have backporting because the CSS and option names changes seem way to many to be confident for backporting to be reliably done. Plus a fair number of changes in the JS and the controllers.
Comment 31 Katrin Fischer 2023-10-30 16:04:26 UTC
Back here :)
Comment 32 Katrin Fischer 2023-10-30 16:18:44 UTC
(In reply to Katrin Fischer from comment #31)
> Back here :)

Ok, a little confused which branch has the most recent work. I believe it's yours Joubu - could you rebase, just in case? I can have a look tomorrow or Wednesday probably.
Comment 33 Jonathan Druart 2023-10-31 08:08:27 UTC
(In reply to Katrin Fischer from comment #32)
> (In reply to Katrin Fischer from comment #31)
> > Back here :)
> 
> Ok, a little confused which branch has the most recent work. I believe it's
> yours Joubu - could you rebase, just in case? I can have a look tomorrow or
> Wednesday probably.

Yes, my branch. See my last comments, sorry if it was not meaningful enough.
Comment 34 Katrin Fischer 2023-11-01 10:52:29 UTC
The column filters on the items table in the staff detail page didn't work or look as expected to me.

I tested with http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=146

Activated the column filters: formatting is off, there is no placeholder text in the columns.
Typed "94" into the barcode filter, nothing was found (it works without this patch set)

If you type 94 into the top search input field, the result is as expected.
Comment 35 Jonathan Druart 2023-11-01 11:32:22 UTC
(In reply to Katrin Fischer from comment #34)
> The column filters on the items table in the staff detail page didn't work
> or look as expected to me.
> 
> I tested with
> http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=146
> 
> Activated the column filters: formatting is off, there is no placeholder
> text in the columns.
> Typed "94" into the barcode filter, nothing was found (it works without this
> patch set)
> 
> If you type 94 into the top search input field, the result is as expected.

Is it working on top of bug 33568? If so there is no need to fix it here.
Comment 36 David Cook 2023-11-02 00:17:41 UTC
(In reply to Katrin Fischer from comment #34)
> The column filters on the items table in the staff detail page didn't work
> or look as expected to me.
> 
> I tested with
> http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=146
> 
> Activated the column filters: formatting is off, there is no placeholder
> text in the columns.
> Typed "94" into the barcode filter, nothing was found (it works without this
> patch set)
> 
> If you type 94 into the top search input field, the result is as expected.

I just tried as well and I see what you mean about the formatting/placeholder text. (That is, there is no placeholder text, and the column filters appear above the table headers rather than below them.)

(Actually, if you click "Activate" and "Deactivate" a few times, sometimes the filters are above the headers and sometimes they're below the headers.)

However, the "Search:" and the barcode column filter worked for me.
Comment 37 David Cook 2023-11-02 00:19:30 UTC
It looks like the same problem exists on http://localhost:8081/cgi-bin/koha/reports/itemslost.pl which uses koha-tmpl/intranet-tmpl/prog/js/table_filters.js as well

Actually, it's a bit different. Initially, it shows the filters and placeholder text in the right place.

But if you click "Deactivate filters" and then "Activate filters" again the placeholder text is gone and the filters are above the table headers. 

The "Search:" and column filters work data-wise though.
Comment 38 David Cook 2023-11-02 00:40:55 UTC
(In reply to Jonathan Druart from comment #35)
> Is it working on top of bug 33568? If so there is no need to fix it here.

Looks like table_filters.js is still using the legacy syntax: 
https://gitlab.com/joubu/Koha/-/blob/bug_33568/koha-tmpl/intranet-tmpl/prog/js/table_filters.js

Something relevant is that the new "./koha-tmpl/intranet-tmpl/lib/jquery/plugins/jquery.dataTables.columnFilter.js" does not contain "bFiltersAlreadyActivated"

Looks like detail.pl has some probably unnecessary code involving the header rows...
Comment 39 David Cook 2023-11-02 01:14:15 UTC
(In reply to David Cook from comment #38)
> Looks like detail.pl has some probably unnecessary code involving the header
> rows...

Nope I'm so confused. 

/cgi-bin/koha/catalogue/detail.pl?biblionumber=29 won't show the placeholder text but /cgi-bin/koha/reports/itemslost.pl does even though they're running the same code...

There's something funny with this line:
clone = thead_row.clone().addClass('filters_row');

The action of adding that class is changing the row and that's what seems to be causing the placeholder problem...
Comment 40 David Cook 2023-11-02 01:15:53 UTC
No that's not it... the console.log() isn't showing me the data structure at the point in time. It's just going to returning the object and then I'm seeing it after it's already been transformed later...
Comment 41 David Cook 2023-11-02 01:38:40 UTC
Created attachment 158229 [details] [review]
Bug 34913: Fix Activate/Deactive filters with table_filters.js

This change prevents columnFilter from being called twice. If it's
called twice, it compromises the formatting of the filters.
Comment 42 David Cook 2023-11-02 01:39:44 UTC
Jonathan, can you add the above patch to your branch?

That problem was sure annoying but removing the second totally unnecessary call to columnFilter is what fixes the problem on the following pages:

http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
http://localhost:8081/cgi-bin/koha/reports/itemslost.pl
Comment 43 Jonathan Druart 2023-11-02 06:32:06 UTC
Done, patch added to bug_34913 and bug_33568.
Comment 44 Jonathan Druart 2023-11-14 15:54:56 UTC
Why is this stuck?
Comment 45 David Cook 2023-11-23 23:32:40 UTC
(In reply to Jonathan Druart from comment #44)
> Why is this stuck?

Good question. It would be good for this one to make it into the QA Team emails. 

I'd hoped this would make it into 23.11 but I think it's too late now :(
Comment 46 Jonathan Druart 2023-12-13 15:20:56 UTC
Remote branch rebased against master.
Comment 47 Jonathan Druart 2024-01-04 13:45:30 UTC
Who has tested and approved the remote branch?

Me, then? Owen and David?
Comment 48 Jonathan Druart 2024-01-09 15:17:35 UTC
Remote branch rebased against master.
Comment 49 Martin Renvoize 2024-01-09 20:26:28 UTC
Thanks for the rebase. I'm scheduling a bit of time to look at this first thing tomorrow.
Comment 50 Martin Renvoize 2024-01-10 14:51:23 UTC
catalogue/itemsearch.pl
+    use Data::Printer colored => 1; warn p $search_params;
Comment 51 Martin Renvoize 2024-01-10 15:10:40 UTC
OK.. all I've tested appears to be working.. that was a workout, we have a metric tonne of tables!

Could you clean out that last Data::Printer I spotted and mentioned above then add my SO lines on the remainder of patches on the branch please?

I'm happy to call this PQA, the scripts are happy and so far I can't spot any regressions now that the items table issues have been resolved.

It does feel slightly odd still having that last bFiltersAlreadyActivated still in the code, but things do all appear to be working now.. it's great to see how much cleanup this resulted in.

I'd advocate pushing to master and maybe waiting a week before backport in the hopes that any last strugglers we missed might be flushed out.  As such.. I'm going to update the status to PQA :)
Comment 52 Jonathan Druart 2024-01-10 15:25:02 UTC
(In reply to Martin Renvoize from comment #51)
> Could you clean out that last Data::Printer I spotted and mentioned above
> then add my SO lines on the remainder of patches on the branch please?

Done.
Comment 53 Katrin Fischer 2024-01-17 12:14:06 UTC
Can you please rebase on latest master? 

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/edifactmsgs.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc
error: could not apply 6bdfa73a80... Bug 34913: DataTables upgrade: Update CSS and option names
Comment 54 Jonathan Druart 2024-01-17 17:40:58 UTC
I am afk until next Tuesday.
Comment 55 Owen Leonard 2024-01-17 17:52:54 UTC
My branch has been synced with Jonathan's and rebased against master:

https://gitlab.com/koha-dev/koha-dev/-/commits/bug_34913
Comment 56 wainuiwitikapark 2024-01-24 03:26:43 UTC
Unfortunately I don't have access to rmaint comms but I am the only one around at the minute to do things for the upcoming security release, but I am unsure what I need to backport
Comment 62 Katrin Fischer 2024-01-26 14:17:30 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 63 Katrin Fischer 2024-01-26 16:04:47 UTC
Please fix!

koha_1       | #   Failed test 'Test::Perl::Critic for "C4/Utils/DataTables/VirtualShelves.pm"'
koha_1       | #   at /usr/share/perl5/Test/Perl/Critic.pm line 121.
koha_1       | # 
koha_1       | #   "@order_by" is declared but not used at line 86, column 9.  Unused variables clutter code and make it harder to read.  (Severity: 3)
koha_1       | Use of uninitialized value $_ in pattern match (m//) at /usr/share/perl5/PPIx/Regexp/Structure/Quantifier.pm line 117.
Comment 64 Jonathan Druart 2024-01-29 09:42:43 UTC
Created attachment 161575 [details] [review]
Bug 34913: Fix perlcritic for VirtualShelves.pm

C4/Utils/DataTables/VirtualShelves.pm: "@order_by" is declared but not used at line 86, column 9.  Unused variables clutter code and make it harder to read.  (Severity: 3)
Comment 65 Jonathan Druart 2024-01-30 07:15:37 UTC
(In reply to Jonathan Druart from comment #64)
> Created attachment 161575 [details] [review] [review]
> Bug 34913: Fix perlcritic for VirtualShelves.pm
> 
> C4/Utils/DataTables/VirtualShelves.pm: "@order_by" is declared but not used
> at line 86, column 9.  Unused variables clutter code and make it harder to
> read.  (Severity: 3)

Patch has been pushed.
Comment 66 Fridolin Somers 2024-01-31 12:50:22 UTC
Last patch (Fix perlcritic for VirtualShelves.pm) is minor, it will be in 23.11.03
Comment 67 Fridolin Somers 2024-02-01 16:15:33 UTC
Pushed to 23.11.x for 23.11.02
Comment 68 Lucas Gass 2024-03-19 17:34:51 UTC
We need to reopen this and rebase this patchset for 23.05.x.
Comment 70 Lucas Gass 2024-03-19 19:22:04 UTC
Backported to 23.05.x for upcoming 23.05.10
Comment 71 wainuiwitikapark 2024-04-12 01:36:40 UTC
(In reply to Lucas Gass from comment #70)
> Backported to 23.05.x for upcoming 23.05.10

Hey, I have found an issue which I believe comes from: "Bug 34913: Upgrade staff interface DataTables from 1.10.18 to 1.13.6"

When adding orders from a marc file to a basket, the text in the form is showing up as white.

Applying the following to intranetusercss temporarily fixes the issue:

table.dataTable tbody tr.selected > * {
  color: black;
}

Is it possible for this issue to please be fixed?

Thanks,

Wainui
Comment 72 Owen Leonard 2024-04-18 11:29:46 UTC
(In reply to wainuiwitikapark from comment #71)

> When adding orders from a marc file to a basket, the text in the form is
> showing up as white.

I believe this refers to problem fixed by Bug 36035: Wrong text colour in addorderiso2709.pl.