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.
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
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
Created attachment 123236 [details] [review] Bug 28762: handle notforloan better
Created attachment 123237 [details] [review] Bug 28762: handle notforloan better
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
Created attachment 124661 [details] [review] Bug 28762: (follow-up) fix template logic with Available
(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'
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>
Created attachment 124667 [details] [review] Bug 28762: handle notforloan better Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Created attachment 124668 [details] [review] Bug 28762: (follow-up) fix template logic with Available Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
QA: Looking here
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?
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?
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? ;)
Obviously you would need to pass staff => 1 now too in the course reserves page in staff client.. If we go this direction?
(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.
+ [% IF ( OPACShowCheckoutName ) %] this is wrong for staff, there is no test on "opac".
(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 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.
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.
Changing status for comment17 and comment20
*** Bug 29095 has been marked as a duplicate of this bug. ***
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.
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.
My follow-up should allow us to switch out the `isa` in the template include now I believe.
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.
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.
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.
Created attachment 125233 [details] [review] Bug 28762: Move notforloan fallback into object
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.
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.
Created attachment 161563 [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>
Created attachment 161564 [details] [review] Bug 28762: handle notforloan better Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Created attachment 161565 [details] [review] Bug 28762: (follow-up) fix template logic with Available Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Created attachment 161566 [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.
Created attachment 161567 [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.
Created attachment 161568 [details] [review] Bug 28762: Move notforloan fallback into object
Created attachment 161569 [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>
Created attachment 161570 [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>
Created attachment 161571 [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>
Created attachment 161572 [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>
Created attachment 161573 [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>
Created attachment 161574 [details] [review] Bug 28762: Move notforloan fallback into object Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 161584 [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>
Created attachment 161585 [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>
Created attachment 161586 [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>
Created attachment 161587 [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>
Created attachment 161588 [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>
Created attachment 161589 [details] [review] Bug 28762: Move notforloan fallback into object Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 161590 [details] [review] Bug 28762: Add test for new 'instructors' accessor
Created attachment 161591 [details] [review] Bug 28762: Unit tests for not_for_loan accessor
Created attachment 163310 [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>
Created attachment 163311 [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>
Created attachment 163312 [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>
Created attachment 163313 [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>
Created attachment 163314 [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>
Created attachment 163315 [details] [review] Bug 28762: Move notforloan fallback into object Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163316 [details] [review] Bug 28762: Add test for new 'instructors' accessor
Created attachment 163317 [details] [review] Bug 28762: Unit tests for not_for_loan accessor
Another rebase done.
there might be an issue found in comment 30
(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.
Created attachment 165843 [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>
Created attachment 165844 [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>
Created attachment 165845 [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>
Created attachment 165846 [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>
Created attachment 165847 [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>
Created attachment 165848 [details] [review] Bug 28762: Move notforloan fallback into object Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 165849 [details] [review] Bug 28762: Add test for new 'instructors' accessor
Created attachment 165850 [details] [review] Bug 28762: Unit tests for not_for_loan accessor
Just another quick rebase