If you modify the patron category and the item type when you edit a circulation rule, this will create a new circulation rule
Created attachment 60325 [details] [review] Bug 17927: (QA followup) Fix timestamp nullable in hold.json Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60329 [details] [review] Disabled the select for patron Category and Item Type Test plan 1 - Edit a circulation rule and check you can modify the patron Category and item type 2 - Apply patch 3 - Edit a circulation rule : you can't change anymore
Patch works as intended, but I'm not sure if this is the best solution? Allowing for quick copying of a circulation rule (simply, change patron category and/or item type) seems to me to be useful - although a bit illogical (why create a new rule, and not just change the edited one?). Is it a bug or a feature? Even if this is the preferred method, I'm not to keen on the fact that patron category and itemtype still look like dropdowns, but can't be clicked. Had i not known about the issue and stumbled upon it, I would have thought that to be a bug! Either they should not look like dropdowns and/or be grayed out, or similar.
Hi ! I don't think there is a quite better solution without changing everything in the code, but if you have any better idea I can try it ;) I mean, this situation is better than the old one, because there is no unexpected behaviour of the application, in the old one, you can click on edit, save, see the new rule as expected, not notice there is a new one and work with a new circulation rule. The application works so : - when you click on edit, every fields of the current row get copied into the last row. - when you submit the edited lines (which has no other status than the other lines), it checks if there is a row in the base with the same key (library + patron category + Item type). Case not it creates a new one, otherwise it modifies the old one : this means if you accidentally create a new rule, you will destroy the existing one. I think to solve completely the problem, the interface should be transformed at all, because with this working, we can't : - write a function which copies without editing if the keys are the same - write a function which edits without creating a new one if the keys are differents. Moreover the comments in the code were expecting this behaviour, but I think somewhere in the versions the behaviour got lost. I don't think an alert would be a good idea because : - if you put it on submit button, you will get it whenever you create a new rule - if you put it on the edit button, you will get it whenever you edit a rule and it is quite annoying A half solution could be to alert if any line is about to get replaced, but it is the same problem : the user will be alerted only if the button does his job.
I see your point Baptiste! This could be an interim solution, rather than rewriting the whole circulation rules. But is it perhaps possible to gray out the two dropdowns (patron category and item type) or similar, so that it will be apparent to the users that these are actually not clickable when editing a rule?
Hi ! You mean gray out the field or the menus ? Because graying out the scrollable menus is the expected behavior of this patch..
Yes, I mean graying out the scrollable menus. I can't click them, but I don't see anything visually that these are not editable. Or am I missing something?
Hi, What browser do you use ? On firefox and chromium it works as expected on my computer (I can clearly see a difference with other buttons).
Hi Baptiste! I've been testing with Google Chrome. I just double-checked with Firefox, and there it looks good (the blue turns grey on the scrolldown menu)! In Chrome it's gray to start with though.
May I change the status for needs signoff ?
I'm changing the status to Needs signoff, since my previous issue was browser-related.
Created attachment 63288 [details] [review] [SIGNED-OFF] Disabled the select for patron Category and Item Type Test plan 1 - Edit a circulation rule and check you can modify the patron Category and item type 2 - Apply patch 3 - Edit a circulation rule : you can't change anymore https://bugs.koha-community.org/show_bug.cgi?id=18125 Signed-off-by: Owen Leonard <oleonard@myacpl.org>
I am not sure this is acceptable, it will become hard to copy rules. Maybe we need an additional "Copy" action button?
(In reply to Jonathan Druart from comment #13) > I am not sure this is acceptable, it will become hard to copy rules. > Maybe we need an additional "Copy" action button? Indeed. I am not so sure if the advantage of this oneliner outweights the disadvantage of no longer being able to copy a rule. It should not be that hard to add a copy button?
(In reply to Marcel de Rooy from comment #14) > (In reply to Jonathan Druart from comment #13) > > I am not sure this is acceptable, it will become hard to copy rules. > > Maybe we need an additional "Copy" action button? > > Indeed. I am not so sure if the advantage of this oneliner outweights the > disadvantage of no longer being able to copy a rule. > It should not be that hard to add a copy button? Hum.. the problem with a copy button it that the working of this button wouldn't be intuitive. I mean if we create an copy button (like the old edit), this will not copy a rule if you don't modify the 2 first fields.
(In reply to Baptiste from comment #15) > (In reply to Marcel de Rooy from comment #14) > > (In reply to Jonathan Druart from comment #13) > > > I am not sure this is acceptable, it will become hard to copy rules. > > > Maybe we need an additional "Copy" action button? > > > > Indeed. I am not so sure if the advantage of this oneliner outweights the > > disadvantage of no longer being able to copy a rule. > > It should not be that hard to add a copy button? > > Hum.. the problem with a copy button it that the working of this button > wouldn't be intuitive. > > I mean if we create an copy button (like the old edit), this will not copy a > rule if you don't modify the 2 first fields. Yeah, you may need to lift some check first and make sure it is applied later..
The problem with a copy button is following: if you don't modify the key, it will edit an existing one (which is the worst behaviour I think)
Baptiste doesn't work anymore at BibLibre. Unassigning to avoid signaling that this bug is still being supervized. Which could wrongly discourage someone to continue the work.
(In reply to Victor Grousset/tuxayo from comment #18) > Baptiste doesn't work anymore at BibLibre. > Unassigning to avoid signaling that this bug is still being supervized. > Which could wrongly discourage someone to continue the work. The behaviour of editing also acting as an option to copy rules is described in the manual and really very useful. I think we cannot remove it without introducing another option for copying that works better than what we have now. Closing WONTFIX.