Bug 28986

Summary:	Parent itemtype not selected when editing circ rules
Product:	Koha	Reporter:	Nick Clemens (kidclamp) <nick>
Component:	Staff interface	Assignee:	Nick Clemens (kidclamp) <nick>
Status:	CLOSED FIXED	QA Contact:	Joonas Kylmälä <joonas.kylmala>
Severity:	major
Priority:	P5 - low	CC:	fridolin.somers, gmcharlt, joonas.kylmala, kyle, victor
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	21.11.00,21.05.05,20.11.11
Circulation function:
Attachments:	Bug 28986: Remove (All) notation to correctly select rule for editing Bug 28986: Remove (All) notation to correctly select rule for editing Bug 28986: Remove (All) notation to correctly select rule for editing

Description Nick Clemens (kidclamp) 2021-09-09 15:20:58 UTC

To recreate:
1 - Go to Administration -> Item types
2 - Edit a type to have a parent of another type
3 - Go to Administration -> Circulation and fines rules
4 - Add a rule for the child type
5 - Add a rule for the parent type
6 - Click edit on child type, note dropdown in editor correctly selected
7 - Click edit on parent type - note dropdown menu refers to 'All' itemtypes

Comment 1 Nick Clemens (kidclamp) 2021-09-09 15:25:12 UTC

Created attachment 124717 [details] [review]
Bug 28986: Remove (All) notation to correctly select rule for editing

This patch updates the JS comparison code to remove the '(All)' hint before comparing
to the current value

To test:
1 - Go to Administration -> Item types
2 - Edit a type to have a parent of another type
3 - Go to Administration -> Circulation and fines rules
4 - Add a rule for all categories for the child itemtype
5 - Add a rule for all categories for the parent itemtype
6 - Click edit on child type, note itemtype dropdown in editor correctly selected
7 - Click edit on parent type - note itemtype dropdown menu refers to 'All' itemtypes
8 - Apply patch
9 - Reload page and edit parent type rule
10 - Itemtype dropdown is correctly populated

Comment 2 Andrew Fuerste-Henry 2021-09-09 16:59:58 UTC

Created attachment 124718 [details] [review]
Bug 28986: Remove (All) notation to correctly select rule for editing

This patch updates the JS comparison code to remove the '(All)' hint before comparing
to the current value

To test:
1 - Go to Administration -> Item types
2 - Edit a type to have a parent of another type
3 - Go to Administration -> Circulation and fines rules
4 - Add a rule for all categories for the child itemtype
5 - Add a rule for all categories for the parent itemtype
6 - Click edit on child type, note itemtype dropdown in editor correctly selected
7 - Click edit on parent type - note itemtype dropdown menu refers to 'All' itemtypes
8 - Apply patch
9 - Reload page and edit parent type rule
10 - Itemtype dropdown is correctly populated

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Comment 3 Joonas Kylmälä 2021-09-11 15:07:48 UTC

Created attachment 124796 [details] [review]
Bug 28986: Remove (All) notation to correctly select rule for editing

This patch updates the JS comparison code to remove the '(All)' hint before comparing
to the current value

To test:
1 - Go to Administration -> Item types
2 - Edit a type to have a parent of another type
3 - Go to Administration -> Circulation and fines rules
4 - Add a rule for all categories for the child itemtype
5 - Add a rule for all categories for the parent itemtype
6 - Click edit on child type, note itemtype dropdown in editor correctly selected
7 - Click edit on parent type - note itemtype dropdown menu refers to 'All' itemtypes
8 - Apply patch
9 - Reload page and edit parent type rule
10 - Itemtype dropdown is correctly populated

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Comment 4 Joonas Kylmälä 2021-09-11 15:12:24 UTC

Without refactoring the table logic here totally this looks like it is the best least-effort-fix we can do right now. I wonder at what state the new GUI for managing these circ rules is, the code here is very hacky and we cannot forever do this sort of one off fixes. Anyway passing QA since as I said, this is seemingly the best we can do right now without having the new editing page for circ rules or doing total refactoring.

Comment 5 Jonathan Druart 2021-09-20 12:48:21 UTC

Pushed to master for 21.11, thanks to everybody involved!

Comment 6 Kyle M Hall (khall) 2021-10-01 17:33:15 UTC

Pushed to 21.05.x for 21.05.05

Comment 7 Fridolin Somers 2021-10-06 20:10:32 UTC

Pushed to 20.11.x for 20.11.11

Comment 8 Victor Grousset/tuxayo 2021-10-16 01:32:06 UTC

Missing dependencies for 20.05.x, it shouldn't be affected, no backport.

Item type parent is only since 20.11.x