Bug 28762 - Item status shows incorrectly on course-details.pl
Summary: Item status shows incorrectly on course-details.pl
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords: rel_24_05_candidate
: 29095 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-07-26 20:01 UTC by Andrew Fuerste-Henry
Modified: 2024-05-07 09:40 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28762: Handle different item statuses in course details (2.84 KB, patch)
2021-07-27 20:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.18 KB, patch)
2021-07-27 21:21 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (1.96 KB, patch)
2021-07-27 21:46 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (1.96 KB, patch)
2021-07-27 21:58 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (835 bytes, patch)
2021-09-08 17:02 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.26 KB, patch)
2021-09-09 01:53 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.02 KB, patch)
2021-09-09 01:54 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (876 bytes, patch)
2021-09-09 01:55 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 28762: (QA follow-up) A first attempt to sync the includes (5.42 KB, patch)
2021-09-10 09:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.82 KB, patch)
2021-09-23 18:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.29 KB, patch)
2021-09-24 07:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.41 KB, patch)
2021-09-24 07:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.91 KB, patch)
2021-09-24 09:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.37 KB, patch)
2024-01-27 19:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.02 KB, patch)
2024-01-27 19:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (878 bytes, patch)
2024-01-27 19:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.82 KB, patch)
2024-01-27 19:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.40 KB, patch)
2024-01-27 19:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.91 KB, patch)
2024-01-27 19:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.43 KB, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.08 KB, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (943 bytes, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.89 KB, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.46 KB, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.97 KB, patch)
2024-01-27 19:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.43 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.08 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (943 bytes, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.89 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.46 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.97 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Add test for new 'instructors' accessor (2.61 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Unit tests for not_for_loan accessor (2.34 KB, patch)
2024-01-29 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.43 KB, patch)
2024-03-18 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.08 KB, patch)
2024-03-18 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (943 bytes, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.90 KB, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.46 KB, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.97 KB, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Add test for new 'instructors' accessor (2.61 KB, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Unit tests for not_for_loan accessor (2.43 KB, patch)
2024-03-18 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.43 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.08 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (943 bytes, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.90 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.46 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.94 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Add test for new 'instructors' accessor (2.61 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Unit tests for not_for_loan accessor (2.43 KB, patch)
2024-04-30 11:14 UTC, Martin Renvoize
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 2021-07-26 20:01:25 UTC
To recreate:
- create a course in course reserves, add an item to it.
- confirm your item shows Available for its status on course-details.pl
- edit your item to be withdrawn, lost, damaged, notforloan, and restricted use
- reload course-details.pl, confirm it still shows available

filed bug 28761 for this on the OPAC. On the OPAC, withdrawn, lost, and damaged *do* update the status.
Comment 1 Lucas Gass 2021-07-27 20:38:16 UTC
Created attachment 123233 [details] [review]
Bug 28762: Handle different item statuses in course details

To test:
1. create a course in course reserves, add an item to it.
2. confirm your item shows Available for its status on course-details.pl
3. edit your item to be withdrawn, lost, damaged, notforloan, and restricted use
4. reload course-details.pl, confirm it still shows available
5. apply patch
6. repeat step 3 with each of the statuses and make sure it correctly shows on course-details.pl

https://bugs.koha-community.org/show_bug.cgi?id=28761
Comment 2 Lucas Gass 2021-07-27 21:21:50 UTC
Created attachment 123234 [details] [review]
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt

This patch adds an item-status.inc to the staff side much like what is already in place on the OPAC.

To test:
1. create a course in course reserves, add an item to it.
2. confirm your item shows Available for its status on course-details.pl
3. edit your item to be withdrawn, lost, damaged, notforloan, and restricted use
4. reload course-details.pl, confirm it still shows available
5. apply patch
6. repeat step 3 with each of the statuses and make sure it correctly shows on course-details.pl
Comment 3 Lucas Gass 2021-07-27 21:46:59 UTC
Created attachment 123236 [details] [review]
Bug 28762: handle notforloan better
Comment 4 Lucas Gass 2021-07-27 21:58:10 UTC
Created attachment 123237 [details] [review]
Bug 28762: handle notforloan better
Comment 5 Hayley Pelham 2021-09-03 02:25:16 UTC
Hi Lucas,

I can confirm that the patch fixes the Status of the course reserve item to show whether it is withdrawn, damaged, etc.

However, if it is none of those, the status column no longer shows Available, but instead is blank.

I am assuming it should say available if none of those values you listed are true?

Cheers,
Hayley
Comment 6 Lucas Gass 2021-09-08 17:02:54 UTC
Created attachment 124661 [details] [review]
Bug 28762: (follow-up) fix template logic with Available
Comment 7 Lucas Gass 2021-09-08 17:03:50 UTC
(In reply to Hayley Pelham from comment #5)
> Hi Lucas,
> 
> I can confirm that the patch fixes the Status of the course reserve item to
> show whether it is withdrawn, damaged, etc.
> 
> However, if it is none of those, the status column no longer shows
> Available, but instead is blank.
> 
> I am assuming it should say available if none of those values you listed are
> true?
> 
> Cheers,
> Hayley

Thanks Haley! I am have made a correction and I am reseting the status of this one to 'Needs sign-off'
Comment 8 Hayley Pelham 2021-09-09 01:53:51 UTC
Created attachment 124666 [details] [review]
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt

This patch adds an item-status.inc to the staff side much like what is already in place on the OPAC.

To test:
1. create a course in course reserves, add an item to it.
2. confirm your item shows Available for its status on course-details.pl
3. edit your item to be withdrawn, lost, damaged, notforloan, and restricted use
4. reload course-details.pl, confirm it still shows available
5. apply patch
6. repeat step 3 with each of the statuses and make sure it correctly shows on course-details.pl

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Comment 9 Hayley Pelham 2021-09-09 01:54:19 UTC
Created attachment 124667 [details] [review]
Bug 28762: handle notforloan better

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Comment 10 Hayley Pelham 2021-09-09 01:55:08 UTC
Created attachment 124668 [details] [review]
Bug 28762: (follow-up) fix template logic with Available

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Comment 11 Marcel de Rooy 2021-09-10 08:47:26 UTC
QA: Looking here
Comment 12 Marcel de Rooy 2021-09-10 08:58:52 UTC
My concern here is introducing a item-status.inc that will be deviating from the opac version quite a bit already.
Thinking about how we could keep them together. Maybe a few more lines here or there, but if they could be the same?
Comment 13 Marcel de Rooy 2021-09-10 09:28:41 UTC
Created attachment 124744 [details] [review]
Bug 28762: (QA follow-up) A first attempt to sync the includes

This is just a start.
Should we resolve all differences?
Comment 14 Marcel de Rooy 2021-09-10 09:31:13 UTC
Jonathan: What do you think? Get the includes correct here and enforce synced changes or just push them while different, and hope to keep them closer? ;)
Comment 15 Marcel de Rooy 2021-09-10 09:32:37 UTC
Obviously you would need to pass staff => 1 now too in the course reserves page in staff client..
If we go this direction?
Comment 16 Jonathan Druart 2021-09-10 09:52:12 UTC
(In reply to Marcel de Rooy from comment #14)
> Jonathan: What do you think? Get the includes correct here and enforce
> synced changes or just push them while different, and hope to keep them
> closer? ;)

Hard call, I don't think we should have them all in sync.
Maybe we could have a 'common' directory with some include files that would be identically both sides? We even could have a test to ensure it.
Comment 17 Jonathan Druart 2021-09-10 09:55:32 UTC
+        [% IF ( OPACShowCheckoutName ) %]

this is wrong for staff, there is no test on "opac".
Comment 18 Marcel de Rooy 2021-09-10 10:00:01 UTC
(In reply to Jonathan Druart from comment #17)
> +        [% IF ( OPACShowCheckoutName ) %]
> 
> this is wrong for staff, there is no test on "opac".

Yeah. I did not finish this sync operation. But wanted to get some discussion first. It should be possible to add some opac/staff checks and have them exactly identical.
But it might be to hard to require that here.
Comment 19 Marcel de Rooy 2021-09-13 08:45:23 UTC
Comment on attachment 124744 [details] [review]
Bug 28762: (QA follow-up) A first attempt to sync the includes

Coming here again, I think we are asking too much when insisting on a synced item status include. So I am obsoleting this follow-up.
Comment 20 Marcel de Rooy 2021-09-13 08:55:18 UTC
So from this perspective looking at a new include, I see things like:

[% IF item.isa('Koha::Item') %]
..
[% IF NOT ( item.isa('Koha::Item') ) AND item.transfertwhen %]

Which is awful. Yes, I know that it comes from the opac include, but we should really solve its twisted usage.

When we are starting at staff here, please use one consistent approach. Either always pass a Koha object, or do something else. But keep it consistent.
Comment 21 Marcel de Rooy 2021-09-13 08:55:50 UTC
Changing status for comment17 and comment20
Comment 22 Martin Renvoize 2021-09-23 17:09:06 UTC
*** Bug 29095 has been marked as a duplicate of this bug. ***
Comment 23 Martin Renvoize 2021-09-23 18:03:55 UTC
OK, this certainly needs some work.. if we're going to introduce an 'item-status' include here I feel it should really be re-usable and simple/clear.

For that to be the case we should really be expecting a simple 'item' object to be passed and not deal with special cases for weird structures.

I propose updating the minimal controller to use Koha:: Objects instead of C4:: classes and methods for these item details.
Comment 24 Martin Renvoize 2021-09-23 18:08:26 UTC
Created attachment 125220 [details] [review]
Bug 28762: Use Koha::Course in course-details controller

This patch updates Koha::Course to include the 'instructors' relation
accessor and then update the course-details controller to use the
Koha::Course object and pass it to the template instead of building a
hash using GetCourse.
Comment 25 Martin Renvoize 2021-09-23 18:10:06 UTC
My follow-up should allow us to switch out the `isa` in the template include now I believe.
Comment 26 Martin Renvoize 2021-09-24 07:42:16 UTC
Created attachment 125228 [details] [review]
Bug 28762: Update item-status include

This patch updates the item-status include so that it expects just an
item object making if simpler and more widely re-usable.
Comment 27 Martin Renvoize 2021-09-24 07:57:42 UTC
Created attachment 125229 [details] [review]
Bug 28762: Update item-status include

This patch updates the item-status include so that it expects just an
item object making if simpler and more widely re-usable.
Comment 28 Martin Renvoize 2021-09-24 08:02:10 UTC
The include here was heavily inspired by the opac equivalent, the status cell of the holdings table in catalogue/detail.tt and the equivalent include included in bug 28854/bug 24023.

What I'm least familiar with here is holds and independent branches, so please test that area carefully.

The challenge is holds in the holds table vs holds in the tmp_holdsqueue.. they have different cases but I think I've handled both.

As for IndependantBranches.. there's some really funky code in catalogue/details and GetItemInfo that deals with that.. we should be moving to Koha::Item objects in catalogue/details as soon as possible with this patch in my opinion.. but I feel that area may need some work.
Comment 29 Martin Renvoize 2021-09-24 09:26:30 UTC
Created attachment 125233 [details] [review]
Bug 28762: Move notforloan fallback into object
Comment 30 Kelly McElligott 2022-02-04 19:00:23 UTC
Upon testing this, I found that the Not for Loan status was not showing accurately in the Course Reserve table on the OPAC.  Testing for the status availability is correct on Staff Interface.  

A not-for-loan status shows available regardless of what is chosen (Ordered, Staff Collection, Not for Loan) in the Course Reserve table on the OPAC.  Looking at the specific item within the OPAC, the not-for-loan status is showing correctly- specifically, it is using what is on the Authorized Value OPAC description.
Comment 31 Dani Elder 2024-01-26 22:15:40 UTC
It would be nice if the course reserves status would show in course reserves. When items are long long over due this keeps staff from hunting down a book that may look available in the staff interface. They can toggle to the opac to check, but they shouldn't have to.
Comment 32 Martin Renvoize 2024-01-27 19:10:58 UTC Comment hidden (obsolete)
Comment 33 Martin Renvoize 2024-01-27 19:11:02 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2024-01-27 19:11:05 UTC Comment hidden (obsolete)
Comment 35 Martin Renvoize 2024-01-27 19:11:08 UTC Comment hidden (obsolete)
Comment 36 Martin Renvoize 2024-01-27 19:11:12 UTC Comment hidden (obsolete)
Comment 37 Martin Renvoize 2024-01-27 19:11:15 UTC Comment hidden (obsolete)
Comment 38 Martin Renvoize 2024-01-27 19:50:35 UTC Comment hidden (obsolete)
Comment 39 Martin Renvoize 2024-01-27 19:50:38 UTC Comment hidden (obsolete)
Comment 40 Martin Renvoize 2024-01-27 19:50:42 UTC Comment hidden (obsolete)
Comment 41 Martin Renvoize 2024-01-27 19:50:45 UTC Comment hidden (obsolete)
Comment 42 Martin Renvoize 2024-01-27 19:50:49 UTC Comment hidden (obsolete)
Comment 43 Martin Renvoize 2024-01-27 19:50:52 UTC Comment hidden (obsolete)
Comment 44 Martin Renvoize 2024-01-29 10:47:06 UTC Comment hidden (obsolete)
Comment 45 Martin Renvoize 2024-01-29 10:47:10 UTC Comment hidden (obsolete)
Comment 46 Martin Renvoize 2024-01-29 10:47:13 UTC Comment hidden (obsolete)
Comment 47 Martin Renvoize 2024-01-29 10:47:16 UTC Comment hidden (obsolete)
Comment 48 Martin Renvoize 2024-01-29 10:47:20 UTC Comment hidden (obsolete)
Comment 49 Martin Renvoize 2024-01-29 10:47:23 UTC Comment hidden (obsolete)
Comment 50 Martin Renvoize 2024-01-29 10:47:26 UTC Comment hidden (obsolete)
Comment 51 Martin Renvoize 2024-01-29 10:47:30 UTC Comment hidden (obsolete)
Comment 52 Martin Renvoize 2024-03-18 12:10:55 UTC Comment hidden (obsolete)
Comment 53 Martin Renvoize 2024-03-18 12:10:58 UTC Comment hidden (obsolete)
Comment 54 Martin Renvoize 2024-03-18 12:11:02 UTC Comment hidden (obsolete)
Comment 55 Martin Renvoize 2024-03-18 12:11:05 UTC Comment hidden (obsolete)
Comment 56 Martin Renvoize 2024-03-18 12:11:08 UTC Comment hidden (obsolete)
Comment 57 Martin Renvoize 2024-03-18 12:11:11 UTC Comment hidden (obsolete)
Comment 58 Martin Renvoize 2024-03-18 12:11:14 UTC Comment hidden (obsolete)
Comment 59 Martin Renvoize 2024-03-18 12:11:18 UTC Comment hidden (obsolete)
Comment 60 Martin Renvoize 2024-03-18 12:14:57 UTC
Another rebase done.
Comment 61 Victor Grousset/tuxayo 2024-04-30 01:28:06 UTC
there might be an issue found in comment 30
Comment 62 Martin Renvoize 2024-04-30 11:13:27 UTC
(In reply to Victor Grousset/tuxayo from comment #61)
> there might be an issue found in comment 30

This patchset only touches the staff client and the comment appears to say the results here are correct.

I think if we want to sync the OPAC back to follow the logic here we should do that in a follow-up bug rather than hold this patch back any longer.
Comment 63 Martin Renvoize 2024-04-30 11:14:03 UTC Comment hidden (obsolete)
Comment 64 Martin Renvoize 2024-04-30 11:14:07 UTC Comment hidden (obsolete)
Comment 65 Martin Renvoize 2024-04-30 11:14:11 UTC Comment hidden (obsolete)
Comment 66 Martin Renvoize 2024-04-30 11:14:15 UTC Comment hidden (obsolete)
Comment 67 Martin Renvoize 2024-04-30 11:14:18 UTC Comment hidden (obsolete)
Comment 68 Martin Renvoize 2024-04-30 11:14:22 UTC Comment hidden (obsolete)
Comment 69 Martin Renvoize 2024-04-30 11:14:25 UTC Comment hidden (obsolete)
Comment 70 Martin Renvoize 2024-04-30 11:14:28 UTC Comment hidden (obsolete)
Comment 71 Martin Renvoize 2024-04-30 11:15:31 UTC
Just another quick rebase