[Koha-bugs] [Bug 25534] Add ability to send an email specifying a reason and store the reason when canceling a hold

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25534

--- Comment #52 from Kyle M Hall <kyle at 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.