Description
Nicole C. Engard
2015-05-20 15:39:59 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. We just had a request for adding records without items too - It would not only helfpul for electronic, but also for analytical records. 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. 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. 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. 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 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. (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. Created attachment 116236 [details] [review] Bug 14237: Schema updates Created attachment 116237 [details] [review] Bug 14237: Add individual bibliographic records to course reserves 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. Created attachment 116239 [details] [review] Bug 14237: Schema updates Created attachment 116240 [details] [review] Bug 14237: Add individual bibliographic records to course reserves Not yet ready for testing, just need to get the batch adding working! Hi Aleisha, looking good! I like the solution in the GUI for adding individual records. Works well so far! 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) 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) 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> 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> 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> Created attachment 116654 [details] [review] Bug 14237: Schema updates Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> Resorted patches and added Christian's sign-off line to the other 2 patches. 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. 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> 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> Created attachment 118222 [details] [review] Bug 14237: Schema updates Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> Created attachment 118223 [details] [review] Bug 14237: Tests Confirm all tests pass: t/db_dependent/CourseReserves/CourseItems.t 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. To explain: small changes requested by QA are usually best done as a follow-up and don't require a new sign-off. (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 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! (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. (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! 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> 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> 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> 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> 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> Thx a lot, Kyle! 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. 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. (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. 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. 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? (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. 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> 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> 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> 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> 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> 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> (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 Pushed to master for 21.11, thanks to everybody involved! Thx everyone, this one makes me especially happy :) Enhancement not backported for 21.05.x Should/could we promote this to 'new feature'? |