Bug 32775

Summary: Ordering when there are multiple languages within a language group is wrong
Product: Koha Reporter: Alex Buckley <alexbuckley>
Component: System AdministrationAssignee: Alex Buckley <alexbuckley>
Status: RESOLVED FIXED QA Contact: Fridolin Somers <fridolin.somers>
Severity: minor    
Priority: P5 - low CC: aleisha, fridolin.somers, gmcharlt, lucas, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07
Attachments: Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: (QA follow-up) Fix tab and indenting issues
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: (QA follow-up) Fix tab and indenting issues
Bug 32775: (follow-up) Unit tests
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: (QA follow-up) Fix tab and indenting issues
Bug 32775: (follow-up) Unit tests
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: (QA follow-up) Fix tab and indenting issues
Bug 32775: (follow-up) Unit tests
Bug 32775: (follow-up) Unit tests
Bug 32775: Do not order language directories based on when they were created
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group
Bug 32775: (QA follow-up) Fix tab and indenting issues
Bug 32775: (QA follow-up) remove useless empty line

Description Alex Buckley 2023-02-01 01:45:29 UTC
When you have en-NZ (ordered first), and a different non-en language (ordered second) enabled on your Koha instance then you can encounter the non-en language displaying first in the 'language' and 'OPACLanguages' system preferences.
Comment 1 Alex Buckley 2023-02-01 02:02:56 UTC
Created attachment 145918 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Catalyst IT
Comment 2 Alex Buckley 2023-02-01 02:03:24 UTC
Created attachment 145919 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Catalyst IT
Comment 3 Alex Buckley 2023-02-02 01:52:37 UTC
Created attachment 145983 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Comment 4 Alex Buckley 2023-02-02 01:54:38 UTC
Created attachment 145984 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Comment 5 Martin Renvoize 2023-02-02 12:50:18 UTC
Created attachment 145996 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-02-02 12:50:23 UTC
Created attachment 145997 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-02-02 12:50:27 UTC
Created attachment 145998 [details] [review]
Bug 32775: (QA follow-up) Fix tab and indenting issues

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2023-02-02 12:51:11 UTC
Working as described, signing off.

I added a QA follow-up to fix the use of tabs and ran a perltidy over the changed code block for clarity.
Comment 9 Katrin Fischer 2023-02-05 12:13:44 UTC
Created attachment 146194 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2023-02-05 12:13:48 UTC
Created attachment 146195 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2023-02-05 12:13:54 UTC
Created attachment 146196 [details] [review]
Bug 32775: (QA follow-up) Fix tab and indenting issues

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2023-02-05 12:15:34 UTC
There are some bigger changes to a method in C4 here - we should have some unit tests.
Comment 13 Alex Buckley 2023-02-10 00:50:46 UTC
Working on unit tests
Comment 14 Alex Buckley 2023-02-13 02:19:03 UTC
Created attachment 146569 [details] [review]
Bug 32775: (follow-up) Unit tests

Test plan:
1. Apply this patch and restart services
2. Run t/db_dependent/Languages.t
3. Notice tests 16 and 17 sometimes fail
4. Apply all the other patches and restart services
5. Run t/db_dependent/Languages.t
6. Notice all tests consistently pass

Sponsored-by: Kinder library, New Zealand
Comment 15 Alex Buckley 2023-02-13 02:19:25 UTC
Created attachment 146570 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Alex Buckley 2023-02-13 02:19:40 UTC
Created attachment 146571 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Alex Buckley 2023-02-13 02:19:55 UTC
Created attachment 146572 [details] [review]
Bug 32775: (QA follow-up) Fix tab and indenting issues

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Alex Buckley 2023-02-13 03:08:45 UTC
Created attachment 146573 [details] [review]
Bug 32775: (follow-up) Unit tests

Test plan:
1. Apply this patch and restart services
2. Run t/db_dependent/Languages.t
3. Notice tests 16 and 17 sometimes fail
4. Apply all the other patches and restart services
5. Run t/db_dependent/Languages.t
6. Notice all tests consistently pass

Sponsored-by: Kinder library, New Zealand
Comment 19 Alex Buckley 2023-02-13 03:09:03 UTC
Created attachment 146574 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Alex Buckley 2023-02-13 03:09:26 UTC
Created attachment 146575 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Alex Buckley 2023-02-13 03:09:42 UTC
Created attachment 146576 [details] [review]
Bug 32775: (QA follow-up) Fix tab and indenting issues

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Alex Buckley 2023-02-13 03:12:05 UTC
(In reply to Katrin Fischer from comment #12)
> There are some bigger changes to a method in C4 here - we should have some
> unit tests.

Hi Katrin, 

I have attached a unit test follow-up patch. I have ordered it as the first patch so people can run it before applying our patchfix and see that it doesn't pass, then apply our other patches and re-run the unit test and see the tests do pass.

Could you please let us know if we need to provide more extensive unit tests?

Alex
Comment 23 Katrin Fischer 2023-02-13 08:40:03 UTC
If the only change is providing unit tests, it's ok to keep the SO status. No need for retesting. I think another QA'er can better reply to the question of the amount/quality of tests.
Comment 24 Alex Buckley 2023-02-13 20:29:26 UTC
Thank you Katrin
Comment 25 Fridolin Somers 2023-03-18 02:57:05 UTC
Hi,

Small QA fail :
 FAIL	C4/Languages.pm
   FAIL	  spelling
		 acount ==> account
		 acount ==> account
		 acount ==> account

False alert, it is the var :
  my $acount = @{ $language_groups->{$a} };

I pass QA
Comment 26 Fridolin Somers 2023-03-18 02:59:54 UTC
Created attachment 148392 [details] [review]
Bug 32775: (follow-up) Unit tests

Test plan:
1. Apply this patch and restart services
2. Run t/db_dependent/Languages.t
3. Notice tests 16 and 17 sometimes fail
4. Apply all the other patches and restart services
5. Run t/db_dependent/Languages.t
6. Notice all tests consistently pass

Sponsored-by: Kinder library, New Zealand
Comment 27 Fridolin Somers 2023-03-18 03:00:47 UTC
Created attachment 148393 [details] [review]
Bug 32775: (follow-up) Unit tests

Test plan:
1. Apply this patch and restart services
2. Run t/db_dependent/Languages.t
3. Notice tests 16 and 17 sometimes fail
4. Apply all the other patches and restart services
5. Run t/db_dependent/Languages.t
6. Notice all tests consistently pass

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 28 Fridolin Somers 2023-03-18 03:00:50 UTC
Created attachment 148394 [details] [review]
Bug 32775: Do not order language directories based on when they were created

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 29 Fridolin Somers 2023-03-18 03:00:53 UTC
Created attachment 148395 [details] [review]
Bug 32775: Correctly display language order in sysprefs when you have multiple languages in a group

Test plan:
1. Apply first patch
2. Install en-NZ and another language - e.g. mi-NZ - so you have three
languages showing in your sysprefs en-NZ (selected), en (unselected) and
mi-NZ (selected)
3. Tick en-NZ and mi-NZ in language and OPACLanguages sysprefs. Order
en-NZ above mi-NZ and save
4. Refresh the syspref page and confirm mi-NZ is displaying above en-NZ,
even though you ordered en-NZ first
5. Refresh the syspref page multiple times and observe the order of the
languages changes
6. Apply this (second) patch
7. Restart plack
8. Refresh syspref page several times confirming that en-NZ is always
ordered first - as expected.

Sponsored-by: Kinder library, New Zealand
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 30 Fridolin Somers 2023-03-18 03:00:55 UTC
Created attachment 148396 [details] [review]
Bug 32775: (QA follow-up) Fix tab and indenting issues

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 31 Fridolin Somers 2023-03-18 03:00:58 UTC
Created attachment 148397 [details] [review]
Bug 32775: (QA follow-up) remove useless empty line

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 32 Fridolin Somers 2023-03-18 03:03:50 UTC
> Could you please let us know if we need to provide more extensive unit tests?

In my opinion this is already good.
It is a small change ;)
Comment 33 Alex Buckley 2023-03-20 04:26:12 UTC
(In reply to Fridolin Somers from comment #32)
> > Could you please let us know if we need to provide more extensive unit tests?
> 
> In my opinion this is already good.
> It is a small change ;)

Thanks very much Fridolin
Comment 34 Tomás Cohen Arazi 2023-05-15 18:51:03 UTC
Please rebase.
Comment 35 Tomás Cohen Arazi 2023-05-15 18:54:30 UTC
Sorry, it was a very easy rebase. Fixed.
Comment 36 Tomás Cohen Arazi 2023-05-15 21:17:05 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 37 Alex Buckley 2023-05-15 21:25:57 UTC
Hello, 

Could we please request that this enhancement be backported to the 22.05.x branch. One of our clients sponsored this fix and would love to receive it before their upgrade to 23.05 later this year. 

Thanks,
Alex
Comment 38 Pedro Amorim 2023-06-05 16:59:37 UTC
Nice work everyone!

Pushed to 22.11.x for next release
Comment 39 Aleisha Amohia 2023-06-15 02:59:39 UTC
Thanks Pedro!

Can this please be backported to 22.05?

Aleisha