Bug 24279 - Claims Returned does not work when set from moredetail.pl or additem.pl
Summary: Claims Returned does not work when set from moredetail.pl or additem.pl
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 14697
Blocks: 26083 26093
  Show dependency treegraph
 
Reported: 2019-12-19 16:30 UTC by Andrew Fuerste-Henry
Modified: 2020-11-17 20:06 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 24279 - Claims Returned does not work when set from moredetail.pl (2.79 KB, patch)
2019-12-30 13:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279 - Claims Returned does not work when set from additem.pl (1.83 KB, patch)
2019-12-30 13:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279 - Claims Returned does not work when set from moredetail.pl (2.83 KB, patch)
2020-01-02 06:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24279 - Claims Returned does not work when set from additem.pl (1.87 KB, patch)
2020-01-02 06:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24279: (QA follow-up) Show values disabled with tooltips (7.71 KB, patch)
2020-04-23 13:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279: (QA follow-up) Show values disabled with tooltips (7.62 KB, patch)
2020-04-23 13:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279: Display the tip in the title of the option (5.92 KB, patch)
2020-05-07 10:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24279: Disable lost status select if item has a return claim (1.67 KB, patch)
2020-05-26 11:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279: Claims Returned does not work when set from moredetail.pl (2.87 KB, patch)
2020-07-24 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Claims Returned does not work when set from additem.pl (1.90 KB, patch)
2020-07-24 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: (QA follow-up) Show values disabled with tooltips (7.66 KB, patch)
2020-07-24 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Display the tip in the title of the option (5.98 KB, patch)
2020-07-24 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Disable lost status select if item has a return claim (1.69 KB, patch)
2020-07-24 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Disable editing lost status in items editor if item is a return claim (5.38 KB, patch)
2020-07-24 11:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24279: Claims Returned does not work when set from moredetail.pl (2.93 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Claims Returned does not work when set from additem.pl (1.96 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: (QA follow-up) Show values disabled with tooltips (7.72 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Display the tip in the title of the option (6.04 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Disable lost status select if item has a return claim (1.75 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24279: Disable editing lost status in items editor if item is a return claim (5.48 KB, patch)
2020-07-27 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-12-19 16:30:12 UTC
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
Comment 1 Kyle M Hall 2019-12-30 13:05:27 UTC
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
Comment 2 Kyle M Hall 2019-12-30 13:05:36 UTC
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
Comment 3 Katrin Fischer 2020-01-02 06:48:14 UTC
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>
Comment 4 Katrin Fischer 2020-01-02 06:48:41 UTC
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>
Comment 5 Katrin Fischer 2020-01-02 06:51:13 UTC
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.
Comment 6 Andrew Fuerste-Henry 2020-01-29 14:25:57 UTC
Katrin, did you mean to mark this signed off?
Comment 7 Katrin Fischer 2020-01-29 14:27:59 UTC
Thx, Andrew! Changed status.
Comment 8 Jonathan Druart 2020-02-05 11:17:17 UTC
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.
Comment 9 Kyle M Hall 2020-02-05 11:31:32 UTC
(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.
Comment 10 Jonathan Druart 2020-02-05 11:44:23 UTC
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>
Comment 11 Martin Renvoize 2020-02-24 12:27:02 UTC
Are you intending to come back to this one Kyle?
Comment 12 Kyle M Hall 2020-04-23 13:06:50 UTC
Created attachment 103597 [details] [review]
Bug 24279: (QA follow-up) Show values disabled with tooltips
Comment 13 Kyle M Hall 2020-04-23 13:08:49 UTC
Created attachment 103598 [details] [review]
Bug 24279: (QA follow-up) Show values disabled with tooltips
Comment 14 Kyle M Hall 2020-04-23 13:10:47 UTC
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.
Comment 15 Jonathan Druart 2020-05-07 10:19:13 UTC
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?
Comment 16 Jonathan Druart 2020-05-07 10:34:05 UTC
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.
Comment 17 Kyle M Hall 2020-05-13 13:35:02 UTC
(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.
Comment 18 Katrin Fischer 2020-05-22 12:05:36 UTC
(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?
Comment 19 Jonathan Druart 2020-05-26 08:40:32 UTC
Yes I think we need a follow-up. Failing QA.
Comment 20 Kyle M Hall 2020-05-26 11:27:45 UTC
Created attachment 105355 [details] [review]
Bug 24279: Disable lost status select if item has a return claim
Comment 21 Marcel de Rooy 2020-07-24 07:26:42 UTC
        * Commit title does not start with 'Bug XXXXX: ' - 1818ebc014
        * Commit title does not start with 'Bug XXXXX: ' - d677a15dd1
Comment 22 Marcel de Rooy 2020-07-24 08:59:55 UTC
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>
Comment 23 Marcel de Rooy 2020-07-24 08:59:59 UTC
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>
Comment 24 Marcel de Rooy 2020-07-24 09:00:04 UTC
Created attachment 107288 [details] [review]
Bug 24279: (QA follow-up) Show values disabled with tooltips
Comment 25 Marcel de Rooy 2020-07-24 09:00:08 UTC
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.
Comment 26 Marcel de Rooy 2020-07-24 09:00:14 UTC
Created attachment 107290 [details] [review]
Bug 24279: Disable lost status select if item has a return claim
Comment 27 Marcel de Rooy 2020-07-24 09:02:18 UTC
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.
Comment 28 Kyle M Hall 2020-07-24 10:48:30 UTC
(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.
Comment 29 Kyle M Hall 2020-07-24 11:26:55 UTC
Created attachment 107311 [details] [review]
Bug 24279: Disable editing lost status in items editor if item is a return claim
Comment 30 Marcel de Rooy 2020-07-27 06:54:30 UTC
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>
Comment 31 Marcel de Rooy 2020-07-27 06:54:35 UTC
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>
Comment 32 Marcel de Rooy 2020-07-27 06:54:39 UTC
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>
Comment 33 Marcel de Rooy 2020-07-27 06:54:44 UTC
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>
Comment 34 Marcel de Rooy 2020-07-27 06:54:49 UTC
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>
Comment 35 Marcel de Rooy 2020-07-27 06:54:54 UTC
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>
Comment 36 Jonathan Druart 2020-07-27 15:25:55 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 37 Lucas Gass 2020-07-31 20:45:33 UTC
Doesn't apply clean to 20.05.x branch, if needed please rebase :)