Bug 28762 - Item status shows incorrectly on course-details.pl
Summary: Item status shows incorrectly on course-details.pl
Status: Pushed to main
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: Nick Clemens (kidclamp)
URL:
Keywords: rel_24_11_candidate, RM_priority
: 29095 (view as bug list)
Depends on:
Blocks: 37453
  Show dependency treegraph
 
Reported: 2021-07-26 20:01 UTC by Andrew Fuerste-Henry
Modified: 2024-07-24 15:30 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:
24.11.00


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
Bug 28762: (QA follow-up): fix number of tests (646 bytes, patch)
2024-07-19 01:38 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.43 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.08 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (943 bytes, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.90 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.46 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.94 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Add test for new 'instructors' accessor (2.61 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Unit tests for not_for_loan accessor (2.43 KB, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (follow-up) Remove warning in tests (840 bytes, patch)
2024-07-22 12:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Confirm return of not_for_loan in boolean context matches is_notforloan (2.18 KB, patch)
2024-07-22 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Replace is_notforloan with not_for_loan (2.64 KB, patch)
2024-07-22 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Rename not_for_loan as effective_not_for_loan_status (4.88 KB, patch)
2024-07-22 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: Rename not_for_loan as effective_not_for_loan_status (4.89 KB, patch)
2024-07-22 15:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28762: (QA follow-up): fix number of tests (704 bytes, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: add staff item-status.inc and better handle statues on course-details.tt (7.48 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: handle notforloan better (2.14 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: (follow-up) fix template logic with Available (999 bytes, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Use Koha::Course in course-details controller (1.95 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Update item-status include (8.52 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Move notforloan fallback into object (1.99 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Add test for new 'instructors' accessor (2.66 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Unit tests for not_for_loan accessor (2.32 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: (follow-up) Remove warning in tests (897 bytes, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Confirm return of not_for_loan in boolean context matches is_notforloan (2.24 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Replace is_notforloan with not_for_loan (2.70 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: Rename not_for_loan as effective_not_for_loan_status (4.95 KB, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: (QA follow-up) Restore protection against unknown itemtypes (917 bytes, patch)
2024-07-22 19:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28762: (follow-up) Fix missed not_for_loan_status rename (966 bytes, patch)
2024-07-23 07:45 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
Comment 72 Martin Renvoize 2024-07-03 11:55:27 UTC
Still looking here Marcel?
Comment 73 Marcel de Rooy 2024-07-03 12:14:39 UTC
(In reply to Martin Renvoize from comment #72)
> Still looking here Marcel?

Feel free to take over if I didnt touch it very recently.
Comment 74 Martin Renvoize 2024-07-08 08:59:10 UTC
OK, I've reset the assignee. Can't take it on for QA as it looks like I was heavily involved in it's development in the end. :(
Comment 75 Victor Grousset/tuxayo 2024-07-19 01:38:08 UTC
Created attachment 169172 [details] [review]
Bug 28762: (QA follow-up): fix number of tests
Comment 76 Victor Grousset/tuxayo 2024-07-19 03:42:09 UTC
Fixed number of tests mismatching and causing a failure.
(maybe we put the number of tests for a reason and not just to fix it blindly?)


---

Are the
diag("item-level_itypes: 0");
and
diag("item-level_itypes: 1");

still needed?


(removed from QA queue pending the two previous points)

----

string change looks good 👍

+msgid "Lost"
+msgid "Due: %s"
+msgid "Pending hold"
+msgid "Waiting at %s since %s."
+msgid "Item-level hold (placed %s) for delivery at %s."
+msgid "Restricted"
-msgid "Restricted"
-msgid "Lost"
Comment 77 Martin Renvoize 2024-07-22 10:11:18 UTC
Those 'diag' might be better as 'note' but I do like them in as I find they split up the output of a prove -vv  more clearly personally.. I'm not especially worried though if you want them removed.
Comment 78 Martin Renvoize 2024-07-22 12:30:31 UTC
Created attachment 169273 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 79 Martin Renvoize 2024-07-22 12:30:34 UTC
Created attachment 169274 [details] [review]
Bug 28762: handle notforloan better

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 80 Martin Renvoize 2024-07-22 12:30:38 UTC
Created attachment 169275 [details] [review]
Bug 28762: (follow-up) fix template logic with Available

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 81 Martin Renvoize 2024-07-22 12:30:41 UTC
Created attachment 169276 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 82 Martin Renvoize 2024-07-22 12:30:44 UTC
Created attachment 169277 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 83 Martin Renvoize 2024-07-22 12:30:48 UTC
Created attachment 169278 [details] [review]
Bug 28762: Move notforloan fallback into object

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 84 Martin Renvoize 2024-07-22 12:30:51 UTC
Created attachment 169279 [details] [review]
Bug 28762: Add test for new 'instructors' accessor
Comment 85 Martin Renvoize 2024-07-22 12:30:55 UTC
Created attachment 169280 [details] [review]
Bug 28762: Unit tests for not_for_loan accessor
Comment 86 Martin Renvoize 2024-07-22 12:30:58 UTC
Created attachment 169281 [details] [review]
Bug 28762: (follow-up) Remove warning in tests

Looks like a recent bug introduced a superflous warning, we just clean
that up here whilst we're in the code.
Comment 87 Martin Renvoize 2024-07-22 12:33:10 UTC
(In reply to Victor Grousset/tuxayo from comment #76)
> Fixed number of tests mismatching and causing a failure.
> (maybe we put the number of tests for a reason and not just to fix it
> blindly?)

Rebase error introduced the count discrepancy.. just fixing the number was the right thing to do.. I've rebased it now anyway.

> ---
> 
> Are the
> diag("item-level_itypes: 0");
> and
> diag("item-level_itypes: 1");

Replaced with 'note' which makes the tests quieter

Added a follow-up to clean up a superfluous warning introduced in another patch recently pushed.. no reason not to whilst were in the code.

Back to NQA
Comment 88 Martin Renvoize 2024-07-22 15:35:11 UTC
Created attachment 169311 [details] [review]
Bug 28762: Confirm return of not_for_loan in boolean context matches is_notforloan

This patch is simply here to prove that is_notforloan is just
not_for_loan but in a boolean context.. we'll remove it in the next
patch.
Comment 89 Martin Renvoize 2024-07-22 15:35:15 UTC
Created attachment 169312 [details] [review]
Bug 28762: Replace is_notforloan with not_for_loan

This patch replaces the use of is_notforloan with not_for_loan and
removes the older is_notforloan method and tests
Comment 90 Martin Renvoize 2024-07-22 15:35:18 UTC
Created attachment 169313 [details] [review]
Bug 28762: Rename not_for_loan as effective_not_for_loan_status

This patch updates the method name to follow current conventions.
Comment 91 Martin Renvoize 2024-07-22 15:43:49 UTC
Last three patches can happily be squashed.. they were submitted distinctly to help QA follow the flow of events.
Comment 92 Martin Renvoize 2024-07-22 15:45:24 UTC
Created attachment 169314 [details] [review]
Bug 28762: Rename not_for_loan as effective_not_for_loan_status

This patch updates the method name to follow current conventions.
Comment 93 Nick Clemens (kidclamp) 2024-07-22 19:10:22 UTC
Created attachment 169354 [details] [review]
Bug 28762: (QA follow-up): fix number of tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 94 Nick Clemens (kidclamp) 2024-07-22 19:10:25 UTC
Created attachment 169355 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 95 Nick Clemens (kidclamp) 2024-07-22 19:10:28 UTC
Created attachment 169356 [details] [review]
Bug 28762: handle notforloan better

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 96 Nick Clemens (kidclamp) 2024-07-22 19:10:31 UTC
Created attachment 169357 [details] [review]
Bug 28762: (follow-up) fix template logic with Available

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 97 Nick Clemens (kidclamp) 2024-07-22 19:10:33 UTC
Created attachment 169358 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 98 Nick Clemens (kidclamp) 2024-07-22 19:10:36 UTC
Created attachment 169359 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 99 Nick Clemens (kidclamp) 2024-07-22 19:10:39 UTC
Created attachment 169360 [details] [review]
Bug 28762: Move notforloan fallback into object

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 100 Nick Clemens (kidclamp) 2024-07-22 19:10:42 UTC
Created attachment 169361 [details] [review]
Bug 28762: Add test for new 'instructors' accessor

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 101 Nick Clemens (kidclamp) 2024-07-22 19:10:45 UTC
Created attachment 169362 [details] [review]
Bug 28762: Unit tests for not_for_loan accessor

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 102 Nick Clemens (kidclamp) 2024-07-22 19:10:48 UTC
Created attachment 169363 [details] [review]
Bug 28762: (follow-up) Remove warning in tests

Looks like a recent bug introduced a superflous warning, we just clean
that up here whilst we're in the code.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 103 Nick Clemens (kidclamp) 2024-07-22 19:10:51 UTC
Created attachment 169364 [details] [review]
Bug 28762: Confirm return of not_for_loan in boolean context matches is_notforloan

This patch is simply here to prove that is_notforloan is just
not_for_loan but in a boolean context.. we'll remove it in the next
patch.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 104 Nick Clemens (kidclamp) 2024-07-22 19:10:53 UTC
Created attachment 169365 [details] [review]
Bug 28762: Replace is_notforloan with not_for_loan

This patch replaces the use of is_notforloan with not_for_loan and
removes the older is_notforloan method and tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 105 Nick Clemens (kidclamp) 2024-07-22 19:10:56 UTC
Created attachment 169366 [details] [review]
Bug 28762: Rename not_for_loan as effective_not_for_loan_status

This patch updates the method name to follow current conventions.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 106 Nick Clemens (kidclamp) 2024-07-22 19:10:59 UTC
Created attachment 169367 [details] [review]
Bug 28762: (QA follow-up) Restore protection against unknown itemtypes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 107 Victor Grousset/tuxayo 2024-07-22 22:49:28 UTC
Wait! ILSDI_Services.t fails
(found while trying to check if the minor change in C4/ILSDI/Services.pm is covered (likely is))

kohadev-koha@kohadevbox:koha(testing-SO-QA)$ prove t/db_dependent/ILSDI_Services.t
t/db_dependent/ILSDI_Services.t .. 12/14     # No tests run!

#   Failed test 'No tests run for subtest "GetAvailability itemcallnumber"'
#   at t/db_dependent/ILSDI_Services.t line 1045.
The method Koha::Item->effective_notforloan is not covered by tests!

Trace begun at /kohadevbox/koha/Koha/Object.pm line 971
Koha::Object::AUTOLOAD('Koha::Item=HASH(0x57fb9279d480)') called at /kohadevbox/koha/C4/ILSDI/Services.pm line 973
C4::ILSDI::Services::_availability(989) called at /kohadevbox/koha/C4/ILSDI/Services.pm line 122
C4::ILSDI::Services::GetAvailability('CGI=HASH(0x57fb901401d8)') called at t/db_dependent/ILSDI_Services.t line 1000
main::__ANON__ at /usr/share/perl/5.28/Test/Builder.pm line 339
eval {...} at /usr/share/perl/5.28/Test/Builder.pm line 339
Test::Builder::subtest('Test::Builder=HASH(0x57fb87b01678)', 'GetAvailability itemcallnumber', 'CODE(0x57fb92334470)') called at /usr/share/perl/5.28/Test/More.pm line 807
Test::More::subtest('GetAvailability itemcallnumber', 'CODE(0x57fb92334470)') called at t/db_dependent/ILSDI_Services.t line 1045
# Looks like your test exited with 255 just after 13.
t/db_dependent/ILSDI_Services.t .. Dubious, test returned 255 (wstat 65280, 0xff00)




-----


Thanks Nick for QAing the rest that was out of my reach 👍


----

(In reply to Martin Renvoize from comment #77)
> Those 'diag' might be better as 'note' but I do like them in as I find they
> split up the output of a prove -vv  more clearly personally.. I'm not
> especially worried though if you want them removed.

Nice, good to know about the difference between diag and note. No problem to have these, just needed to be sure it wasn't forgotten.
Comment 108 Martin Renvoize 2024-07-23 07:45:28 UTC
Created attachment 169393 [details] [review]
Bug 28762: (follow-up) Fix missed not_for_loan_status rename
Comment 109 Martin Renvoize 2024-07-23 15:05:11 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant