Bug 36196 - Handling NULL data in ajax calls for cities
Summary: Handling NULL data in ajax calls for cities
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Thibaud Guillot (thibaud_g)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 37492
  Show dependency treegraph
 
Reported: 2024-02-28 15:18 UTC by Thibaud Guillot (thibaud_g)
Modified: 2024-09-18 06:37 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04,23.11.09
Circulation function:


Attachments
Bug 36196 : Handling NULL data on ajax call (1.50 KB, patch)
2024-02-28 15:25 UTC, Thibaud Guillot (thibaud_g)
Details | Diff | Splinter Review
Bug 36196: Handling NULL data on ajax call (1.49 KB, patch)
2024-02-28 23:25 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36196: Handling NULL data on ajax call (1.55 KB, patch)
2024-02-28 23:25 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36196: Handling NULL data on ajax call (1.61 KB, patch)
2024-07-26 13:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thibaud Guillot (thibaud_g) 2024-02-28 15:18:30 UTC
Hello all,

Just a simple patch here, I had the case recently, if a data is null in the cities table, the datatable won't handle the data well and will load indefinitely. I've added a condition so that it can be displayed despite NULL data.
Comment 1 Thibaud Guillot (thibaud_g) 2024-02-28 15:25:43 UTC
Created attachment 162535 [details] [review]
Bug 36196 : Handling NULL data on ajax call

Test plan:

1) Update some data in your cities table, sample for one send:
"UPDATE cities SET city_state=NULL WHERE cityid=<id>"
2) Go on "/cgi-bin/koha/admin/cities.pl" and wait a entire life :)
3) Apply this patch
4) Rebuild your po files if needed
5) Reload the same page and now you get normally the datatable

Sponsored by : BibLibre
Comment 2 Lucas Gass (lukeg) 2024-02-28 23:25:01 UTC
Created attachment 162551 [details] [review]
Bug 36196: Handling NULL data on ajax call

Test plan:

1) Update some data in your cities table, sample for one send:
"UPDATE cities SET city_state=NULL WHERE cityid=<id>"
2) Go on "/cgi-bin/koha/admin/cities.pl" and wait a entire life :)
3) Apply this patch
4) Rebuild your po files if needed
5) Reload the same page and now you get normally the datatable

Sponsored by : BibLibre
Comment 3 Lucas Gass (lukeg) 2024-02-28 23:25:33 UTC
Created attachment 162552 [details] [review]
Bug 36196: Handling NULL data on ajax call

Test plan:

1) Update some data in your cities table, sample for one send:
"UPDATE cities SET city_state=NULL WHERE cityid=<id>"
2) Go on "/cgi-bin/koha/admin/cities.pl" and wait a entire life :)
3) Apply this patch
4) Rebuild your po files if needed
5) Reload the same page and now you get normally the datatable

Sponsored by : BibLibre

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Victor Grousset/tuxayo 2024-03-06 21:20:49 UTC
This seem to be the proper way to handle null values:
https://stackoverflow.com/questions/48917064/how-do-you-properly-handle-null-values-in-datatables/48920248#48920248
Hopefully that works in our case.

From reading quickly:
Adding «"defaultContent": ""»
And maybe having to change our "targets" value if for some reason it only work with '_all' (and that we somehow can get away without [0,1,2,3,4])
Comment 5 Victor Grousset/tuxayo 2024-03-06 21:28:45 UTC
Another source:
https://stackoverflow.com/questions/57711349/how-to-deal-with-null-values-in-jquery-datatable-when-using-ajax/57714479#57714479

---

And another.
According to the usage of defaultContent there, we can even target individual columns for default values:
https://datatables.net/forums/discussion/50823/handling-null-values

So it seems pretty sure we can use that.
Comment 6 Thibaud Guillot (thibaud_g) 2024-03-07 16:15:45 UTC
Hi,

Thanks Victor for your feedback, I think that will be better too... to be honest I look into the code and I found the same behavior in identity_providers.tt#L419.. 

I will modify this
Comment 7 Baptiste Wojtkowski (bwoj) 2024-07-25 14:25:32 UTC
Hello,

The way this fixes the behaviour for default seems standard in koha :

koha@koha-dev:~/src$ git grep -C1 "return data.escapeHtml();"
koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt-                        if ( type == 'display' ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt:                            return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/cities.tt-                        }
--
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smtp_servers.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smtp_servers.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smtp_servers.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt-                                if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt:                                    return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt-                                }
--
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt-                            if ( data && type == 'display' ) {
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt-                            }
--
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/concerns.tt-                            if (data != null) {
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/concerns.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/concerns.tt-                            } else {
--
koha-tmpl/intranet-tmpl/prog/en/modules/tools/quotes.tt-                            if ( data != null ) {
koha-tmpl/intranet-tmpl/prog/en/modules/tools/quotes.tt:                                return data.escapeHtml();
koha-tmpl/intranet-tmpl/prog/en/modules/tools/quotes.tt-                            }


Since this ticket aims at fixing a broken functionnality, couldn't we just pass QA for this one and consider changing the way it works in general in another ticket ?
Comment 8 Martin Renvoize (ashimema) 2024-07-26 13:16:05 UTC
Created attachment 169699 [details] [review]
Bug 36196: Handling NULL data on ajax call

Test plan:

1) Update some data in your cities table, sample for one send:
"UPDATE cities SET city_state=NULL WHERE cityid=<id>"
2) Go on "/cgi-bin/koha/admin/cities.pl" and wait a entire life :)
3) Apply this patch
4) Rebuild your po files if needed
5) Reload the same page and now you get normally the datatable

Sponsored by: BibLibre
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize (ashimema) 2024-07-26 13:19:02 UTC
Passing QA here and moved the discussion around defaultContent to a new wider bug.. we should certainly consider it.
Comment 10 Martin Renvoize (ashimema) 2024-07-26 13:51:03 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 11 Victor Grousset/tuxayo 2024-07-26 19:35:50 UTC
Thanks Martin, I was considering not blocking on that issue and letting other QA people chip in, you rear my mind :)

And thanks for the new ticket.

> The way this fixes the behaviour for default seems standard in koha :


(In reply to Baptiste Wojtkowski (bwoj) from comment #7)
We have a lot of unofficial standards and sometimes it's a trap ^^"
Comment 12 Lucas Gass (lukeg) 2024-08-29 16:57:53 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 13 Fridolin Somers 2024-09-16 13:55:08 UTC
Pushed to 23.11.x for 23.11.09
Comment 14 Wainui Witika-Park 2024-09-18 06:37:03 UTC
Not backporting to 23.05.x unless requested