https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25534 --- Comment #52 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #39)
Doing a first review here:
1) QA script:
Fixed!
2) Unit tests
Fixed!
3) Docs
Maybe it would be worth mentioning that this allows to store a cancellation reason in the documentation? (Bug title + description for release notes). Currently only the email functionality is mentioned, but we also have an interesting db change.
Added! Feel free to change them, I wasn't sure what I should add really.
4) Constants for translation?
+ var REASON_LABEL = _("Reason: ");
I believe this is not really needed here as the Javascript code is in a tt file and not js. Not a blocker, but was wondering.
I see that model being followed on many templates, added for consistency.
5) Sample notice
Sample notice added!
6) Authorised value
I've added the AV category, but no values. I suppose we could add values for new installations, but I don't think we'd want them for existing installs being upgraded. Let me know what you think.
7) GUI
reserve.pl I think it would be nice if the cancellation reason only activated once a "del" was selected. Otherwise people might expect it to do something without that or expect it to also work when they cancel using "X".
It *does* work when canceling using the "X"! For that reason it is not hidden. If you think it's simpler to not allow a reason when using the X and have it hidden unless one of the pulldowns is marked "del" I can make that change too.
moremember.pl/circulation.pl I think for context we should add "Cancellation reason:" here before the pull down too.
Done!
Why pendingreserves, but not holds queue?
Pending reserves has a cancellation function already while the holds queue does not.
pendingreserves: I'd remove the bold and add : (only one string to translate for translators :) )
Done! -- You are receiving this mail because: You are watching all bug changes.