Bug 14237

Summary: Allow bibs to be added to course without items
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: Course reservesAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: new feature    
Priority: P2 CC: aleisha, amber, christian.stelzenmueller, emily, hayleypelham, indradg, jonathan.druart, jzairo, katrin.fischer, kyle, lobrien, martin.renvoize, nick, rogan.hamby, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Large patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch adds a biblionumber column to course_items, adds a relationship between course_items.biblionumber and biblio.biblionumber, and changes course_items.itemnumber to allow null values. This feature allows a patron to add bibliographic records to course reserves. They can be added individually or in a batch. The courses that have reserved this record will also show on the record's detail page.
Version(s) released in:
21.11.00
Bug Depends on:    
Bug Blocks: 30911    
Attachments: Bug 14237: Schema updates
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Database updates
Bug 14237: Schema updates
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Database updates
Bug 14237: Schema updates
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Database updates
Bug 14237: Schema updates
Bug 14237: Tests
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Database updates
Bug 14237: Schema updates
Bug 14237: Tests
Bug 14237: (follow-up) Set course_items.biblionumber for existing rows
Bug 14237: Set course_items.biblionumber in the DBrev
Bug 14237: Add individual bibliographic records to course reserves
Bug 14237: Database updates
Bug 14237: Schema updates
Bug 14237: Tests
Bug 14237: Set course_items.biblionumber in the DBrev
Bug 14237: (follow-up) Make the routines exclusively take itemnumber, biblionumber, or ci_id

Description Nicole C. Engard 2015-05-20 15:39:59 UTC
It would be nice if you could add a bib to a course even if it has no items  - this would be great for e-reserves.
Comment 1 Jessie Zairo 2016-09-25 20:08:21 UTC
It would be great to add a listing for an electronic resource to a course reserve without having to create a dummy item record. Causes some confusion for the student. It would be easier for the user to just click on the link from the bib.
Comment 2 Katrin Fischer 2016-10-19 14:10:01 UTC
We just had a request for adding records without items too - It would not only helfpul for electronic, but also for analytical records.
Comment 3 Amber Hunt 2018-01-25 18:27:55 UTC
This would be so helpful for our library as well. We have items records for many of electronic resources, but they do not have barcodes so I have to add a fake barcode to add them to course reserves. It be similar to the "Add to" option for Cart and List that is available through the detailed record.
Comment 4 Leslie O'Brien 2018-09-10 20:20:16 UTC
We would like to be able to add course reserve items without using a dummy item record or barcode. We had this functionality in our Sierra system.
Comment 5 Emily Dunn 2020-04-07 15:52:48 UTC
I just wanted to say this is still a relevant issue. With more books being published electronically, the need for a barcode isn't really relevant so it would be nice to be able to add to a course reserve without having to apply barcodes to items that otherwise don't really need them.
Comment 6 Hayley Pelham 2021-01-14 23:58:02 UTC
Hi devs,

I've done a bit of surface work on this bug, and now need to decide how we want to store record-level course items for lookup.

Some options are:
- We add a flag to the course_items table (for example a biblionumber column), but I think we would need to allow itemnumber to be NULL, which is not allowed at the moment.
- We create a new table called course_biblios and keep the data stored separate.

If you'd like to see where this would be used you can look at line 70 of course_reserves/add_items.pl where there is a check for if $action is lookup and there is an item, where I'd like to add an elsif $action is lookup and there is a biblio instead of an item.

Any thoughts or advice appreciated!

Cheers,
Hayley
Comment 7 Katrin Fischer 2021-01-15 16:42:53 UTC
I am still really torn about what the best approach here would be :(

Add a bilbionumber column to course_items

+ All contents of a course reserve are in one table
- We will only be using ci_id, biblionumber, and timestamp columns, other columns won't be used as they are all item specific

Add a new table, course_records or similar

- Need to check 2 tables to get all contents
+ Table is more specific, ci_id, biblionumber and timestamp will be enough

? We do something similar in the reserves table already (item and record level holds are distinguised by a filled itemnumber column). But not sure if that is a good pattern to follow. Also all other columns are used for both, so it seems to make mor sense there.

What I am not sure is about what is better for database constraints and such?
If a record is deleted, I think we'd want to delete it from the table by constraint as well.
Comment 8 Kyle M Hall 2021-01-15 17:25:06 UTC
(In reply to Katrin Fischer from comment #7)
> I am still really torn about what the best approach here would be :(
> 
> Add a bilbionumber column to course_items
> 
> + All contents of a course reserve are in one table
> - We will only be using ci_id, biblionumber, and timestamp columns, other
> columns won't be used as they are all item specific
> 
> Add a new table, course_records or similar
> 
> - Need to check 2 tables to get all contents
> + Table is more specific, ci_id, biblionumber and timestamp will be enough
> 
> ? We do something similar in the reserves table already (item and record
> level holds are distinguised by a filled itemnumber column). But not sure if
> that is a good pattern to follow. Also all other columns are used for both,
> so it seems to make mor sense there.
> 
> What I am not sure is about what is better for database constraints and such?
> If a record is deleted, I think we'd want to delete it from the table by
> constraint as well.

I'm fine with either solution. Using the existing table does make some sense, but would require rewriting a decent amount of the existing code. A separate table would be a cleaner add-on, as existing code wouldn't need to be touched much. In either case, ON DELETE CASCADE fk constraints should definitely be implemented.
Comment 9 Aleisha Amohia 2021-02-03 01:12:24 UTC
Created attachment 116236 [details] [review]
Bug 14237: Schema updates
Comment 10 Aleisha Amohia 2021-02-03 01:12:29 UTC
Created attachment 116237 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves
Comment 11 Aleisha Amohia 2021-02-03 01:12:48 UTC
Created attachment 116238 [details] [review]
Bug 14237: Database updates

This patch adds a biblionumber column to course_items, adds a
relationship between course_items.biblionumber and biblio.biblionumber,
and changes course_items.itemnumber to allow null values.
Comment 12 Aleisha Amohia 2021-02-03 01:12:52 UTC
Created attachment 116239 [details] [review]
Bug 14237: Schema updates
Comment 13 Aleisha Amohia 2021-02-03 01:12:57 UTC
Created attachment 116240 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves
Comment 14 Aleisha Amohia 2021-02-03 01:14:05 UTC
Not yet ready for testing, just need to get the batch adding working!
Comment 15 Katrin Fischer 2021-02-05 16:17:42 UTC
Hi Aleisha, looking good! I like the solution in the GUI for adding individual records. Works well so far!
Comment 16 Aleisha Amohia 2021-02-09 02:19:47 UTC
Created attachment 116535 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Comment 17 Aleisha Amohia 2021-02-09 23:29:41 UTC
Created attachment 116619 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Comment 18 PTFS Europe Sandboxes 2021-02-10 08:11:57 UTC
Created attachment 116638 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 19 Katrin Fischer 2021-02-10 12:32:19 UTC
Created attachment 116652 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 20 Katrin Fischer 2021-02-10 12:32:23 UTC
Created attachment 116653 [details] [review]
Bug 14237: Database updates

This patch adds a biblionumber column to course_items, adds a
relationship between course_items.biblionumber and biblio.biblionumber,
and changes course_items.itemnumber to allow null values.

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 21 Katrin Fischer 2021-02-10 12:32:26 UTC
Created attachment 116654 [details] [review]
Bug 14237: Schema updates

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 22 Katrin Fischer 2021-02-10 12:33:14 UTC
Resorted patches and added Christian's sign-off line to the other 2  patches.
Comment 23 Tomás Cohen Arazi 2021-03-04 14:41:52 UTC
Hi, it all looks good. It is only lacking tests for the biblio-only case. Please add that, and I will resume the QA step.
Comment 24 Aleisha Amohia 2021-03-14 20:16:56 UTC
Created attachment 118220 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 25 Aleisha Amohia 2021-03-14 20:17:03 UTC
Created attachment 118221 [details] [review]
Bug 14237: Database updates

This patch adds a biblionumber column to course_items, adds a
relationship between course_items.biblionumber and biblio.biblionumber,
and changes course_items.itemnumber to allow null values.

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 26 Aleisha Amohia 2021-03-14 20:17:11 UTC
Created attachment 118222 [details] [review]
Bug 14237: Schema updates

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Comment 27 Aleisha Amohia 2021-03-14 20:17:18 UTC
Created attachment 118223 [details] [review]
Bug 14237: Tests

Confirm all tests pass: t/db_dependent/CourseReserves/CourseItems.t
Comment 28 Katrin Fischer 2021-03-14 22:39:23 UTC
Hi Aleisha, 

was only the new test added or were there any additional changes? If only the test was added, I think you can move this back to Signed off.
Comment 29 Katrin Fischer 2021-03-14 22:40:07 UTC
To explain: small changes requested by QA are usually best done as a follow-up and don't require a new sign-off.
Comment 30 Aleisha Amohia 2021-03-14 22:55:56 UTC
(In reply to Katrin Fischer from comment #28)
> Hi Aleisha, 
> 
> was only the new test added or were there any additional changes? If only
> the test was added, I think you can move this back to Signed off.

Yes sorry the changes are all in the latest patch! Switching back to signed off
Comment 31 Kyle M Hall 2021-03-22 11:14:03 UTC
Comment on attachment 118220 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

Review of attachment 118220 [details] [review]:
-----------------------------------------------------------------

::: C4/CourseReserves.pm
@@ +944,4 @@
>  =head2 GetItemCourseReservesInfo
>  
>      my $arrayref = GetItemCourseReservesInfo( itemnumber => $itemnumber );
> +    my $arrayref = GetItemCourseReservesInfo( biblionumber => $biblionumber );

This line is masking the variable above it. I imagine that's not what you were intending!
Comment 32 Aleisha Amohia 2021-03-22 19:44:35 UTC
(In reply to Kyle M Hall from comment #31)
> Comment on attachment 118220 [details] [review] [review]
> Bug 14237: Add individual bibliographic records to course reserves
> 
> Review of attachment 118220 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/CourseReserves.pm
> @@ +944,4 @@
> >  =head2 GetItemCourseReservesInfo
> >  
> >      my $arrayref = GetItemCourseReservesInfo( itemnumber => $itemnumber );
> > +    my $arrayref = GetItemCourseReservesInfo( biblionumber => $biblionumber );
> 
> This line is masking the variable above it. I imagine that's not what you
> were intending!

This was intentional, it's just part of the documentation to show how the subroutine can be called. Setting back to Signed off.
Comment 33 Kyle M Hall 2021-04-23 17:19:32 UTC
(In reply to Aleisha Amohia from comment #32)
> (In reply to Kyle M Hall from comment #31)
> > Comment on attachment 118220 [details] [review] [review] [review]
> > Bug 14237: Add individual bibliographic records to course reserves
> > 
> > Review of attachment 118220 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: C4/CourseReserves.pm
> > @@ +944,4 @@
> > >  =head2 GetItemCourseReservesInfo
> > >  
> > >      my $arrayref = GetItemCourseReservesInfo( itemnumber => $itemnumber );
> > > +    my $arrayref = GetItemCourseReservesInfo( biblionumber => $biblionumber );
> > 
> > This line is masking the variable above it. I imagine that's not what you
> > were intending!
> 
> This was intentional, it's just part of the documentation to show how the
> subroutine can be called. Setting back to Signed off.

Oof, how did I not notice that is POD!
Comment 34 Kyle M Hall 2021-04-23 17:26:12 UTC
Created attachment 120128 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2021-04-23 17:47:50 UTC
Created attachment 120129 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2021-04-23 17:48:01 UTC
Created attachment 120130 [details] [review]
Bug 14237: Database updates

This patch adds a biblionumber column to course_items, adds a
relationship between course_items.biblionumber and biblio.biblionumber,
and changes course_items.itemnumber to allow null values.

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Kyle M Hall 2021-04-23 17:48:06 UTC
Created attachment 120131 [details] [review]
Bug 14237: Schema updates

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Kyle M Hall 2021-04-23 17:48:10 UTC
Created attachment 120132 [details] [review]
Bug 14237: Tests

Confirm all tests pass: t/db_dependent/CourseReserves/CourseItems.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Katrin Fischer 2021-04-24 09:58:55 UTC
Thx a lot, Kyle!
Comment 40 Jonathan Druart 2021-05-06 13:47:22 UTC
I wanted to provide more tests because IMO this is lacking tests.

However I cannot, there is something wrong in the design, biblionumber cannot be null, so:

0. Don't apply the patches
1. Create a course, add an item
2. Apply the patch, run updatedatabase:

Atomic update generated errors: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`#sql-1_f9`, CONSTRAINT `fk_course_items_biblionumber` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement " ALTER TABLE course_items ADD CONSTRAINT `fk_course_items_biblionumber` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE "]  at /usr/share/perl5/DBIx/Class/Schema.pm line 1118.
Comment 41 Aleisha Amohia 2021-05-07 02:09:28 UTC
Created attachment 120652 [details] [review]
Bug 14237: (follow-up) Set course_items.biblionumber for existing rows

This atomic update will run after the first atomic update which adds the
biblionumber column to course_items.
Comment 42 Jonathan Druart 2021-05-07 07:16:20 UTC
(In reply to Aleisha Amohia from comment #41)
> Created attachment 120652 [details] [review] [review]
> Bug 14237: (follow-up) Set course_items.biblionumber for existing rows
> 
> This atomic update will run after the first atomic update which adds the
> biblionumber column to course_items.

It won't work, the column won't be created because of the NOT NULL clause.
Comment 43 Jonathan Druart 2021-05-07 07:23:14 UTC
Created attachment 120659 [details] [review]
Bug 14237: Set course_items.biblionumber in the DBrev

We need to create the new column without the NOT NULL clause, set the
correct values, then add the NOT NULL clause.
Comment 44 Jonathan Druart 2021-05-07 07:26:24 UTC
I think this patch is the way to go (DBrev)

However I am still struggling with something: shouldn't we allow biblionumber xor itemnumber for the different C4::CourseReserves subroutines?
It does not make sense to me to pass the biblionumber when we have already the itemnumber.

Asking a QA feedback on this question.

Kyle or Tomas maybe?
Comment 45 Kyle M Hall 2021-05-11 17:44:58 UTC
(In reply to Jonathan Druart from comment #44)
> I think this patch is the way to go (DBrev)
> 
> However I am still struggling with something: shouldn't we allow
> biblionumber xor itemnumber for the different C4::CourseReserves subroutines?
> It does not make sense to me to pass the biblionumber when we have already
> the itemnumber.
> 
> Asking a QA feedback on this question.
> 
> Kyle or Tomas maybe?

I'm not opposed to that. It should be easy enough to add an exception if both are passed in. It could be re-engineered to by pk/type as well.
Comment 46 Nick Clemens 2021-06-11 19:21:25 UTC
Created attachment 121876 [details] [review]
Bug 14237: Add individual bibliographic records to course reserves

This feature allows a patron to add bibliographic records to course
reserves. They can be added individually or in a batch. The courses that
have reserved this record will also show on the record's detail page.

To test:
1) Update database, refresh schema, and restart services
2) Enable the system preference UseCourseReserves
3) Set up a couple of biblios and a couple of items (attached to
    different biblios)
4) Go to Course Reserves and add a new course
5) Click Add reserves and put something in both the barcode field and
biblionumber field. Click submit and confirm you get an error.
6) Add a barcode in the barcode field and submit. Confirm the item is
reserved for the course as expected.
7) Add a biblionumber in the barcode field and submit. Add notes and
Save. Confirm the record is reserved for the course as expected and the
notes are saved correctly.
8) Edit the record-level course reserve that you just added. Confirm the
correct record shows and any edits save as expected.
9) Go back to the course and try removing reserves. Use both the Remove
action button for individual reserves and the Remove all reserves
button. Confirm both work as expected.
10) Go to Batch add reserves and put something in both the barcodes
field and biblionumbers field. Click submit and confirm you get an
error.
11) Remove the barcodes and put some biblionumbers in the biblionumbers
field. Add notes and Submit. Confirm the records are all added as
expected.
12) Click on one of the biblios that has been reserved for the course.
Confirm that the course shows under 'Courses that have reserved this
title' on the biblio detail page.
13) Go back to the course and click Add reserves. Try and add a
biblionumber that is already reserved. Confirm it detects that the
biblio has already been reserved.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 47 Nick Clemens 2021-06-11 19:21:30 UTC
Created attachment 121877 [details] [review]
Bug 14237: Database updates

This patch adds a biblionumber column to course_items, adds a
relationship between course_items.biblionumber and biblio.biblionumber,
and changes course_items.itemnumber to allow null values.

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 48 Nick Clemens 2021-06-11 19:21:35 UTC
Created attachment 121878 [details] [review]
Bug 14237: Schema updates

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 49 Nick Clemens 2021-06-11 19:21:39 UTC
Created attachment 121879 [details] [review]
Bug 14237: Tests

Confirm all tests pass: t/db_dependent/CourseReserves/CourseItems.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 50 Nick Clemens 2021-06-11 19:21:44 UTC
Created attachment 121880 [details] [review]
Bug 14237: Set course_items.biblionumber in the DBrev

We need to create the new column without the NOT NULL clause, set the
correct values, then add the NOT NULL clause.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 51 Nick Clemens 2021-06-11 19:21:48 UTC
Created attachment 121881 [details] [review]
Bug 14237: (follow-up) Make the routines exclusively take itemnumber, biblionumber, or ci_id

This patch changes the parameters for several of the CourseReserves routines to take a single
identifier exclusively.

Following the existing pattern we simply return if the params are incorrect

This patch also:
removes an unused 'title' variable
adds a prefetch to save some db calls where we fetch related objects
adjusts tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 52 Nick Clemens 2021-06-11 19:23:47 UTC
(In reply to Kyle M Hall from comment #45)
> (In reply to Jonathan Druart from comment #44)
> > I think this patch is the way to go (DBrev)
> > 
> > However I am still struggling with something: shouldn't we allow
> > biblionumber xor itemnumber for the different C4::CourseReserves subroutines?
> > It does not make sense to me to pass the biblionumber when we have already
> > the itemnumber.
> > 
> > Asking a QA feedback on this question.
> > 
> > Kyle or Tomas maybe?
> 
> I'm not opposed to that. It should be easy enough to add an exception if
> both are passed in. It could be re-engineered to by pk/type as well.

I adjusted the calls, followed existing return if bad params - we could add exception on another bug if needed - PQA but feel free to get a second set of eyes on follow-up
Comment 53 Jonathan Druart 2021-07-12 13:48:18 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 54 Katrin Fischer 2021-07-13 10:42:28 UTC
Thx everyone, this one makes me especially happy :)
Comment 55 Kyle M Hall 2021-07-16 12:25:21 UTC
Enhancement not backported for 21.05.x
Comment 56 Katrin Fischer 2021-11-22 22:29:02 UTC
Should/could we promote this to 'new feature'?