|
Description
Alex Buckley
2023-02-01 01:45:29 UTC
Created attachment 145918 [details] [review] Bug 32775: Do not order language directories based on when they were created Sponsored-by: Catalyst IT 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 Created attachment 145983 [details] [review] Bug 32775: Do not order language directories based on when they were created Sponsored-by: Kinder library, New Zealand 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 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> 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> 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> 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. 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> 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> 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> There are some bigger changes to a method in C4 here - we should have some unit tests. Working on unit tests 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 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> 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> 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> 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 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> 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> 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> (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 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. Thank you Katrin 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
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 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> 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> 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> 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> Created attachment 148397 [details] [review] Bug 32775: (QA follow-up) remove useless empty line Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> > 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 ;)
(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 Please rebase. Sorry, it was a very easy rebase. Fixed. Pushed to master for 23.05. Nice work everyone, thanks! 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 Nice work everyone! Pushed to 22.11.x for next release Thanks Pedro! Can this please be backported to 22.05? Aleisha |