Bug 34269

Summary: Regression in circulation rules for 'similar' patron categories
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: System AdministrationAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jake.deery, jonathan.druart, m.de.rooy, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.03,22.11.09
Circulation function:
Bug Depends on: 30517    
Bug Blocks:    
Attachments: Bug 34269: Convert to using codes in select compares
Bug 34269: Convert to using codes in select compares
Bug 34269: Convert to using codes in select compares
Bug 34269: (QA follow-up) Add missing TT filters
Bug 34269: Convert to using codes in select compares
Bug 34269: (QA follow-up) Add missing TT filters
Bug 34269: Convert to using codes in select compares
Bug 34269: (QA follow-up) Add missing TT filters
Bug 34269: Convert to using codes in select compares
Bug 34269: (QA follow-up) Add missing TT filters
Bug 34269: Convert to using codes in select compares
Bug 34269: (QA follow-up) Add missing TT filters

Description Martin Renvoize (ashimema) 2023-07-13 12:57:41 UTC
Bug 30517 loosens the regular expression used in the edit functionality of the circulation rules matrix in an attempt to make translations work.. However it has caused a regression for the case where a library may have similarly named patron categories (and likely also for item types).

Our partner has the following valid patron categories:

"Staff", "Staff (ISA)", "Staff (temporary)" and "Staff (zero entitlements)"

With the regular expression change introduced in bug 30517, the first three categories all get reduced to just `staff` and as such you can no longer edit rules that contain those categories.

We really shouldn't be relying on a regular expression here at all.. we should be able to pass in the codes rather than descriptions and utilise those for matching against.
Comment 1 Martin Renvoize (ashimema) 2023-07-13 16:02:44 UTC Comment hidden (obsolete)
Comment 2 Martin Renvoize (ashimema) 2023-07-13 16:03:29 UTC
Created attachment 153408 [details] [review]
Bug 34269: Convert to using codes in select compares

This patch updates the logic in smart rules to compare option values to
codes as apposed to option texts to value descriptions.
Comment 3 Katrin Fischer 2023-07-14 09:14:35 UTC
Testing...
Comment 4 Katrin Fischer 2023-07-14 10:06:22 UTC
Created attachment 153456 [details] [review]
Bug 34269: Convert to using codes in select compares

This patch updates the logic in smart rules to compare option values to
codes as apposed to option texts to value descriptions.

0. Apply patch
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type
      Example: Create a 'Children's books' itemtypetype
      and assig 'Books' as its parent
   3. Create a third item type with the same description but something added in ():
      Example: 'Children's books (3-5)'
3. Create a circulation rule for the parent type
   Example: All/Books, with 2 checkouts allowed
4. Create a circulation rule for:
   All/All with 3 checkouts allowed
5. In English, click on "Edit" next to the parent type rule (All/Books)
   --> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
   --> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
   --> The All/Books rule is modified
9. Add rules for Children's books and Children's books (3-5)
10. Click on "Edit" next to each rule and change a value
   --> Verify that the changed values are always saved for the correct rule
Comment 5 Katrin Fischer 2023-07-14 10:06:25 UTC
Created attachment 153457 [details] [review]
Bug 34269: (QA follow-up) Add missing TT filters
Comment 6 Jonathan Druart 2023-07-27 12:10:51 UTC
Conflict with bug 33028.
Comment 7 Martin Renvoize (ashimema) 2023-07-27 13:59:48 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize (ashimema) 2023-07-27 13:59:51 UTC Comment hidden (obsolete)
Comment 9 Martin Renvoize (ashimema) 2023-07-27 14:02:07 UTC
Created attachment 153987 [details] [review]
Bug 34269: Convert to using codes in select compares

This patch updates the logic in smart rules to compare option values to
codes as apposed to option texts to value descriptions.

0. Apply patch
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type
      Example: Create a 'Children's books' itemtypetype
      and assig 'Books' as its parent
   3. Create a third item type with the same description but something added in ():
      Example: 'Children's books (3-5)'
3. Create a circulation rule for the parent type
   Example: All/Books, with 2 checkouts allowed
4. Create a circulation rule for:
   All/All with 3 checkouts allowed
5. In English, click on "Edit" next to the parent type rule (All/Books)
   --> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
   --> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
   --> The All/Books rule is modified
9. Add rules for Children's books and Children's books (3-5)
10. Click on "Edit" next to each rule and change a value
   --> Verify that the changed values are always saved for the correct rule
Comment 10 Martin Renvoize (ashimema) 2023-07-27 14:02:10 UTC
Created attachment 153988 [details] [review]
Bug 34269: (QA follow-up) Add missing TT filters
Comment 11 Katrin Fischer 2023-07-29 21:50:12 UTC
Looks like my SO lines here are missing - I'll fix.
Comment 12 Katrin Fischer 2023-07-29 21:51:17 UTC
Created attachment 154074 [details] [review]
Bug 34269: Convert to using codes in select compares

This patch updates the logic in smart rules to compare option values to
codes as apposed to option texts to value descriptions.

0. Apply patch
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type
      Example: Create a 'Children's books' itemtypetype
      and assig 'Books' as its parent
   3. Create a third item type with the same description but something added in ():
      Example: 'Children's books (3-5)'
3. Create a circulation rule for the parent type
   Example: All/Books, with 2 checkouts allowed
4. Create a circulation rule for:
   All/All with 3 checkouts allowed
5. In English, click on "Edit" next to the parent type rule (All/Books)
   --> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
   --> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
   --> The All/Books rule is modified
9. Add rules for Children's books and Children's books (3-5)
10. Click on "Edit" next to each rule and change a value
   --> Verify that the changed values are always saved for the correct rule

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2023-07-29 21:51:20 UTC
Created attachment 154075 [details] [review]
Bug 34269: (QA follow-up) Add missing TT filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Marcel de Rooy 2023-08-04 09:28:30 UTC
Processing files before patches
File not found : css/staff-global.css at /usr/share/koha/Koha/Template/Plugin/Asset.pm line 107.

Processing files after patches
File not found : css/staff-global.css at /usr/share/koha/Koha/Template/Plugin/Asset.pm line 107.


Please explain.
Comment 15 Marcel de Rooy 2023-08-04 09:31:32 UTC
(In reply to Marcel de Rooy from comment #14)
> Processing files before patches
> File not found : css/staff-global.css at
> /usr/share/koha/Koha/Template/Plugin/Asset.pm line 107.
> 
> Processing files after patches
> File not found : css/staff-global.css at
> /usr/share/koha/Koha/Template/Plugin/Asset.pm line 107.
> 
> 
> Please explain.

Hmm. Dont bother. I ran build resources again ;)
Comment 16 Marcel de Rooy 2023-08-04 09:52:26 UTC
Created attachment 154255 [details] [review]
Bug 34269: Convert to using codes in select compares

This patch updates the logic in smart rules to compare option values to
codes as apposed to option texts to value descriptions.

0. Apply patch
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type
      Example: Create a 'Children's books' itemtypetype
      and assig 'Books' as its parent
   3. Create a third item type with the same description but something added in ():
      Example: 'Children's books (3-5)'
3. Create a circulation rule for the parent type
   Example: All/Books, with 2 checkouts allowed
4. Create a circulation rule for:
   All/All with 3 checkouts allowed
5. In English, click on "Edit" next to the parent type rule (All/Books)
   --> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
   --> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
   --> The All/Books rule is modified
9. Add rules for Children's books and Children's books (3-5)
10. Click on "Edit" next to each rule and change a value
   --> Verify that the changed values are always saved for the correct rule

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2023-08-04 09:52:29 UTC
Created attachment 154256 [details] [review]
Bug 34269: (QA follow-up) Add missing TT filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Tomás Cohen Arazi (tcohen) 2023-08-15 08:24:47 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 19 Fridolin Somers 2023-08-17 18:56:32 UTC
Pushed to 23.05.x for 23.05.03
Comment 20 Fridolin Somers 2023-08-17 18:57:22 UTC
<td data-code="[%- IF auto_renew -%]yes[%- ELSE -%]no[%- END -%]">

Lets hope this does not appear in translation files.
Comment 21 Pedro Amorim 2023-08-18 13:46:35 UTC
Nice work everyone!

Pushed to 22.11.x for next release