To test: - configure claims returned per bug 14697 - Check an item out to a patron - Go to either moredetail.pl or additem.pl and set the item to your claims returned lost value - observe that the item has followed normal lost behavior rather than claims returned behavior We see two options to fix this, either: - Hide the claims returned lost value from moredetail.pl and additem.pl OR - Make Koha follows claims returned process regardless of how the item is set to that claims returned lost value
Created attachment 96683 [details] [review] Bug 24279 - Claims Returned does not work when set from moredetail.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to moredetail.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown
Created attachment 96684 [details] [review] Bug 24279 - Claims Returned does not work when set from additem.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to additem.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown
Created attachment 96724 [details] [review] Bug 24279 - Claims Returned does not work when set from moredetail.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to moredetail.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 96725 [details] [review] Bug 24279 - Claims Returned does not work when set from additem.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to additem.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This works as a quick fix and I've signed it off. I wonder if it's transparent to the users why the value will be missing on the pull downs. Also: are there other possible loop holes if we only fix this on template level? Thinking of the REST API for one.
Katrin, did you mean to mark this signed off?
Thx, Andrew! Changed status.
Should not the option be displayed but greyed instead? With a popup telling the user why it's not selectable? Is the idea to remove ClaimReturnedLostValue from LOST wherever this dropdown list is displayed? I guess we will need to adjust the acquisition (new order, serial receiving) views as well in that case.
(In reply to Jonathan Druart from comment #8) > Should not the option be displayed but greyed instead? With a popup telling > the user why it's not selectable? > > Is the idea to remove ClaimReturnedLostValue from LOST wherever this > dropdown list is displayed? I guess we will need to adjust the acquisition > (new order, serial receiving) views as well in that case. That sounds a bit challenging to implement. Would adding some "help" text indicating something along those lines be acceptable. If you know how to implement a tooltip on a disable select value, that does seem like the nicest way to do this.
disabled then title attribute on the option should work. It is used already in opac-reserve.tt: 244 <option value="[% library.branchcode | html %]" disabled="disabled" title="At least one item is available at this library">[% library.b ranchname | html %]</option>
Are you intending to come back to this one Kyle?
Created attachment 103597 [details] [review] Bug 24279: (QA follow-up) Show values disabled with tooltips
Created attachment 103598 [details] [review] Bug 24279: (QA follow-up) Show values disabled with tooltips
I was able to add bootstrap tooltips, but they cannot be attached directly to disabled select options. I opted to add a question mark icon with the tooltip instead.
Kyle, if there is a claim, then we see that when editing the item: https://snipboard.io/DSpFy7.jpg Should we disable the dropdown as well?
Created attachment 104487 [details] [review] Bug 24279: Display the tip in the title of the option This patch tells select2 to retrieve the title attribute from the original option node.
(In reply to Jonathan Druart from comment #15) > Kyle, if there is a claim, then we see that when editing the item: > https://snipboard.io/DSpFy7.jpg > > Should we disable the dropdown as well? Yes, that seems like a good idea.
(In reply to Kyle M Hall from comment #17) > (In reply to Jonathan Druart from comment #15) > > Kyle, if there is a claim, then we see that when editing the item: > > https://snipboard.io/DSpFy7.jpg > > > > Should we disable the dropdown as well? > > Yes, that seems like a good idea. Do we wait for another follow-up here or is this ready for QA?
Yes I think we need a follow-up. Failing QA.
Created attachment 105355 [details] [review] Bug 24279: Disable lost status select if item has a return claim
* Commit title does not start with 'Bug XXXXX: ' - 1818ebc014 * Commit title does not start with 'Bug XXXXX: ' - d677a15dd1
Created attachment 107286 [details] [review] Bug 24279: Claims Returned does not work when set from moredetail.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to moredetail.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 107287 [details] [review] Bug 24279: Claims Returned does not work when set from additem.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to additem.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 107288 [details] [review] Bug 24279: (QA follow-up) Show values disabled with tooltips
Created attachment 107289 [details] [review] Bug 24279: Display the tip in the title of the option This patch tells select2 to retrieve the title attribute from the original option node.
Created attachment 107290 [details] [review] Bug 24279: Disable lost status select if item has a return claim
Adjusted the two commit titles. If I claim a return, I can still remove the lost status of that item. The item is no longer lost but issued with a return claim. Seems to be an inconsistent situation. Probably should not be allowed either? Probably out of scope: If I check in the item that has been claimed as returned, I still need to resolve the claim. Which is extra work and could be forgotten. And even if I say that is found in the library (which speaks in favor of the patron), the patron is still marked with the returned claim. Changing status.
(In reply to Marcel de Rooy from comment #27) > Adjusted the two commit titles. > > If I claim a return, I can still remove the lost status of that item. The > item is no longer lost but issued with a return claim. Seems to be an > inconsistent situation. Probably should not be allowed either? I'll take a look! > Probably out of scope: > If I check in the item that has been claimed as returned, I still need to > resolve the claim. Which is extra work and could be forgotten. And even if I > say that is found in the library (which speaks in favor of the patron), the > patron is still marked with the returned claim. I would agree that is out of scope. This was actually specified in the development. However, now that I'm thinking about it, perhaps we could file a bug to add a syspref to prevent returns on claimed items in the same way we can prevent returns on withdrawn items and such.
Created attachment 107311 [details] [review] Bug 24279: Disable editing lost status in items editor if item is a return claim
Created attachment 107376 [details] [review] Bug 24279: Claims Returned does not work when set from moredetail.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to moredetail.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 107377 [details] [review] Bug 24279: Claims Returned does not work when set from additem.pl Test Plan: 1) Configure Claimes Returned 2) Apply this patch 3) Go to additem.pl for that item 4) Note the claims returned lost status does not show in the lost status pulldown Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 107378 [details] [review] Bug 24279: (QA follow-up) Show values disabled with tooltips Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 107379 [details] [review] Bug 24279: Display the tip in the title of the option This patch tells select2 to retrieve the title attribute from the original option node. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 107380 [details] [review] Bug 24279: Disable lost status select if item has a return claim Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 107381 [details] [review] Bug 24279: Disable editing lost status in items editor if item is a return claim Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 20.11, thanks to everybody involved!
Doesn't apply clean to 20.05.x branch, if needed please rebase :)