Bug 36235 - System preferences chopping everything after a semicolon.
Summary: System preferences chopping everything after a semicolon.
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords: regression
Depends on: 36084
Blocks:
  Show dependency treegraph
 
Reported: 2024-03-05 15:21 UTC by Lucas Gass
Modified: 2024-03-20 08:45 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.05.00


Attachments
Bug 36235: Encode system preference data with encodeURIComponent (1.50 KB, patch)
2024-03-05 16:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 36235: Encode system preference data with encodeURIComponent (1.56 KB, patch)
2024-03-08 16:16 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36235: Encode system preference data with encodeURIComponent (1.60 KB, patch)
2024-03-09 20:16 UTC, David Nind
Details | Diff | Splinter Review
Bug 36235: Encode system preference data with encodeURIComponent (1.68 KB, patch)
2024-03-11 12:25 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 Lucas Gass 2024-03-05 15:21:40 UTC
I noticed this with IntranetUserJS. To recreate:

1. Add some JS that includes a semicolon to IntranetUserJS:

$(document).ready(function() {
    let my_old_cat = $('#categorycode_entry').val();
    console.log(my_old_cat);
    $('#categorycode_entry').on('change' , function() {
        let my_new_cat = $('#categorycode_entry').val();
        console.log( my_old_cat );
        console.log( my_new_cat );
    });
});

2. Save and reload, or check in DB and you'll see this value:

$(document).ready(function() {
    let my_old_cat = $('#categorycode_entry').val()


To see in DB you can use:

select value from systempreferences where variable = 'IntranetUserJS';
Comment 1 Lucas Gass 2024-03-05 15:28:31 UTC
This doesn't effect only CodeMirror system preference but all that allow free text:

I tested with AdditionalFieldsInZ3950ResultAuthSearch
Comment 2 Jonathan Druart 2024-03-05 15:56:29 UTC
diff --git a/koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js b/koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js
index 1b8634951b6..e2b9bcbac8a 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js
@@ -32,7 +32,7 @@ export class SysprefAPIClient extends HttpClient {
                         .map(variable =>
                             sysprefs[variable].length
                                 ? sysprefs[variable].map(value =>
-                                      "%s=%s".format(variable, value)
+                                      "%s=%s".format(variable, encodeURIComponent(value))
                                   )
                                 : "%s=".format(variable)
                         )
Comment 3 Lucas Gass 2024-03-05 16:28:15 UTC
Created attachment 162778 [details] [review]
Bug 36235: Encode system preference data with encodeURIComponent

To test:
1. Add some data to a freetext system preference like IntranetUserJS or AdditionalFieldsInZ3950ResultAuthSearch that includes a semicolon;
2. Save the pref and reload the page or check the DB. Everything after the semicolon is removed.
3. Apply patch, try again. Data should remain as is.
Comment 4 Lucas Gass 2024-03-05 16:28:48 UTC
I just converted Jonathan's diff into a patch.
Comment 5 Jonathan Druart 2024-03-08 13:17:26 UTC
(In reply to Lucas Gass from comment #4)
> I just converted Jonathan's diff into a patch.

Did it fix your problem?
Comment 6 Lucas Gass 2024-03-08 14:37:59 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to Lucas Gass from comment #4)
> > I just converted Jonathan's diff into a patch.
> 
> Did it fix your problem?

Yes.
Comment 7 Pedro Amorim 2024-03-08 16:16:49 UTC
Created attachment 162975 [details] [review]
Bug 36235: Encode system preference data with encodeURIComponent

To test:
1. Add some data to a freetext system preference like IntranetUserJS or AdditionalFieldsInZ3950ResultAuthSearch that includes a semicolon;
2. Save the pref and reload the page or check the DB. Everything after the semicolon is removed.
3. Apply patch, try again. Data should remain as is.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 8 Pedro Amorim 2024-03-08 16:17:50 UTC
I stumbled upon this by chance while copying intranetuserjs from a live environment to my own local test environment. This should be QAd and/or pushed asap.
Comment 9 David Nind 2024-03-09 20:16:52 UTC
Created attachment 163011 [details] [review]
Bug 36235: Encode system preference data with encodeURIComponent

To test:
1. Add some data to a freetext system preference like IntranetUserJS or AdditionalFieldsInZ3950ResultAuthSearch that includes a semicolon;
2. Save the pref and reload the page or check the DB. Everything after the semicolon is removed.
3. Apply patch, try again. Data should remain as is.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2024-03-09 20:19:55 UTC
Adding my sign off as well, in case that will help with QA.
Comment 11 Jonathan Druart 2024-03-11 12:25:13 UTC
Created attachment 163026 [details] [review]
Bug 36235: Encode system preference data with encodeURIComponent

To test:
1. Add some data to a freetext system preference like IntranetUserJS or AdditionalFieldsInZ3950ResultAuthSearch that includes a semicolon;
2. Save the pref and reload the page or check the DB. Everything after the semicolon is removed.
3. Apply patch, try again. Data should remain as is.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Katrin Fischer 2024-03-12 16:49:43 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 13 Fridolin Somers 2024-03-13 08:03:39 UTC
Bug 36084 is not in 23.11.x but looks like this can be backported ?
Comment 14 Pedro Amorim 2024-03-13 09:51:16 UTC
(In reply to Fridolin Somers from comment #13)
> Bug 36084 is not in 23.11.x but looks like this can be backported ?

I don't think so.
koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js does not exist in 23,11,x
Comment 15 Fridolin Somers 2024-03-19 07:44:40 UTC
(In reply to Pedro Amorim from comment #14)
> (In reply to Fridolin Somers from comment #13)
> > Bug 36084 is not in 23.11.x but looks like this can be backported ?
> 
> I don't think so.
> koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js
> does not exist in 23,11,x

Yes it exists, since Bug 33289

https://git.koha-community.org/Koha-community/Koha/src/branch/23.11.x/koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js
Comment 16 Jonathan Druart 2024-03-19 08:39:35 UTC
The *vue* file exists, not koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js !
Comment 17 Pedro Amorim 2024-03-19 23:30:01 UTC
Correct, we now have:
koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js
koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js

This is not great, for many reasons, one of which highlighted just now (it's confusing and redundant).
Comment 18 Jonathan Druart 2024-03-20 05:12:17 UTC
(In reply to Pedro Amorim from comment #17)
> Correct, we now have:
> koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js
> koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js
> 
> This is not great, for many reasons, one of which highlighted just now (it's
> confusing and redundant).

As said in other places, I have ofc tried to avoid that but failed. Suggestions are more than welcome!
Comment 19 Fridolin Somers 2024-03-20 08:45:44 UTC
(In reply to Pedro Amorim from comment #17)
> Correct, we now have:
> koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js
> koha-tmpl/intranet-tmpl/prog/js/fetch/system-preferences-api-client.js
> 
> This is not great, for many reasons, one of which highlighted just now (it's
> confusing and redundant).

Ooohhhh.
I need coffee ;)