Created attachment 87355 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t
Created attachment 87369 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Signed-off-by: Liz Rea <wizzyrea@gmail.com>
This seems to work well, but something seems not right with course reserves so I couldn't test fully. I have filed bug 22652.
Hello, I 've seen that the problem mentionned in bug22652 also occured here for Shelving location and Home library. And we can't retrieve the field "home library" when we use the batch to add reserve course (gi-bin/koha/course_reserves/batch_add_items.pl), which is not consistent. Sonia
With 22652 pushed maybe time to retest this?
Hello Liz, I've just test it few hours ago on a sandbox, so it was apllied on master. It doesn't work for me... but I'm looking forward to see it signed off!
Created attachment 89037 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Have just retested this on a dockerbox, all looks ok by me, item is modified as I expected (on top of 22652) Cheers, Liz
Ok, I did: - Apply the patch - restart_all - updatedatabase - dbic - restart_all Then I added 2 items to course reserves: 1) Leaving all unchanged - Everything shows up in the table correctly - OK - When editing, they all show "Leave unchanged" - OK - But: change all settings, they are not saved! - BOO! Logs show: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) [for Statement "UPDATE items SET itype=?,homebranch=?,holdingbranch=?,ccode=?,permanent_location=?,location=? WHERE itemnumber=?" with ParamValues: 0="", 1="", 2="", 3="", 4="", 5="", 6=14] at /home/vagrant/kohaclone/C4/Items.pm line 1811. ERROR in _koha_modify_item UPDATE items SET itype=?,homebranch=?,holdingbranch=?,ccode=?,permanent_location=?,location=? WHERE itemnumber=?: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at /home/vagrant/kohaclone/C4/Items.pm line 1814. 2) Change all values - The location is missing from the display summary table (empty) - BOO! - When editing, the home library shows as 'unchanged' - BOO! Sorry for the BOOs, but I think this still needs some work.
(In reply to Katrin Fischer from comment #9) > Ok, I did: > > - Apply the patch > - restart_all > - updatedatabase > - dbic > - restart_all > > > Then I added 2 items to course reserves: > > 1) Leaving all unchanged > - Everything shows up in the table correctly - OK > - When editing, they all show "Leave unchanged" - OK > - But: change all settings, they are not saved! - BOO! I just tested this again, and they were saved? > > Logs show: > > DBD::mysql::st execute failed: Cannot add or update a child row: a foreign > key constraint fails (`koha_kohadev`.`items`, CONSTRAINT `items_ibfk_2` > FOREIGN KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE > CASCADE) [for Statement "UPDATE items SET > itype=?,homebranch=?,holdingbranch=?,ccode=?,permanent_location=?,location=? > WHERE itemnumber=?" with ParamValues: 0="", 1="", 2="", 3="", 4="", 5="", > 6=14] at /home/vagrant/kohaclone/C4/Items.pm line 1811. > ERROR in _koha_modify_item UPDATE items SET > itype=?,homebranch=?,holdingbranch=?,ccode=?,permanent_location=?,location=? > WHERE itemnumber=?: Cannot add or update a child row: a foreign key > constraint fails (`koha_kohadev`.`items`, CONSTRAINT `items_ibfk_2` FOREIGN > KEY (`homebranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) > at /home/vagrant/kohaclone/C4/Items.pm line 1814. I did not receive this message. > > 2) Change all values > - The location is missing from the display summary table (empty) - BOO! This appears on my screen, between collection and home library? > - When editing, the home library shows as 'unchanged' - BOO! Legit, I missed this. :( > > Sorry for the BOOs, but I think this still needs some work. It does, but possibly not as much as all of this.
Created attachment 89741 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89742 [details] [review] Bug 22630: Do not modify item fields if course reserve item says LEAVE UNCHANGED
Created attachment 89743 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page
Initial patch rebased on master. The last two patches should fix mentioned issues
Patch tested with a sandbox, by SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90023 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90024 [details] [review] Bug 22630: Do not modify item fields if course reserve item says LEAVE UNCHANGED Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90025 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90026 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90027 [details] [review] Bug 22630: Do not modify item fields if course reserve item says LEAVE UNCHANGED Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 90028 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
I have followed the test plan and everything was OK. I have also tested the situation where you remove one item from the course : the item's homebranch is reset to his initial value. But I haven't done the 7th step : prove t/db_dependent/CourseReserves/CourseItems.t Don't know how to do that....
Hi, I was ready to sign this off but we have a test failure: # Failed test 'Item ccode should be set back to empty' # at t/db_dependent/CourseReserves/CourseItems.t line 167. # got: 'DVD' # expected: '' Have a look? Cheers, Liz
*** Bug 23083 has been marked as a duplicate of this bug. ***
2 problems here, this is failed QA as an enhancement, but also happens to fix a critical bug - bug 23083. Could the bug fix be separated from the bug?
Created attachment 91177 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91178 [details] [review] Bug 22630: Do not modify item fields if course reserve item says LEAVE UNCHANGED Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91179 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91180 [details] [review] Bug 22630: Allow _RevertFields to set an empty value to item field Sponsored-by: Université de Lyon 3
Applying: Bug 22630: Allow to change homebranch in course reserves Using index info to reconstruct a base tree... M C4/CourseReserves.pm M t/db_dependent/CourseReserves/CourseItems.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/CourseReserves/CourseItems.t CONFLICT (content): Merge conflict in t/db_dependent/CourseReserves/CourseItems.t Auto-merging C4/CourseReserves.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 22630: Allow to change homebranch in course reserves
Created attachment 91575 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91576 [details] [review] Bug 22630: Do not modify item fields if course reserve item says LEAVE UNCHANGED Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91577 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 91578 [details] [review] Bug 22630: Allow _RevertFields to set an empty value to item field Sponsored-by: Université de Lyon 3
Hello, there was a conflict when I tryed to install it on a sandbox : CONFLICT (content): Merge conflict in t/db_dependent/CourseReserves/CourseItems.t
Created attachment 92579 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Created attachment 92580 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr>
Patches rebased on master. Removed two patches because a similar fix was pushed in bug 22142
I tested it according to the plan test and everything was fine. I also tried to modify the value changed by the reserve course, to delete one item in the course and to delete a course. Everything works fine, the value of the item return to their original values. It's OK for me.
Patch tested with a sandbox, by sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 92603 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr> Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 92604 [details] [review] Bug 22630: Fix homebranch select's value in course item edit page Sponsored-by: Université de Lyon 3 Signed-off-by: SONIA BOUIS <sonia.bouis@univ-lyon3.fr> Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
The behaviour when editing items is really weird atm, I have filed a new bug 23727 - once this if fixed, this should be pretty straight forward.
Tiny thing: Capitalization on course summary page needs to be fixed.
I'd love for this to move now we got 23727 fixed, but sadly this no longer applies :( Could we get a rebase please? CONFLICT (content): Merge conflict in t/db_dependent/CourseReserves/CourseItems.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging course_reserves/add_items.pl CONFLICT (content): Merge conflict in course_reserves/add_items.pl Auto-merging C4/CourseReserves.pm
Created attachment 103556 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Patch rebased on master
Forgot to comment earlier, there is a QA tool fail about the boolean/tinyint. Can you please check/fix?
Created attachment 103634 [details] [review] Bug 22630: Update Koha::Schema::Result::CourseItem
Created attachment 103635 [details] [review] Bug 22630: Set is_boolean flag for course_items.homebranch_enabled
Created attachment 103733 [details] [review] Bug 22630: Allow to change homebranch in course reserves Test plan: 1. Create a course (disabled) 2. Add a reserve to this course for an item and set a homebranch different from the item's homebranch 3. Enable the course 4. Verify that the item's homebranch has changed 5. Disable the course 6. Verify that the item's homebranch was reset to its initial value 7. prove t/db_dependent/CourseReserves/CourseItems.t Sponsored-by: Université de Lyon 3 Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 103734 [details] [review] Bug 22630: Update Koha::Schema::Result::CourseItem Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 103735 [details] [review] Bug 22630: Set is_boolean flag for course_items.homebranch_enabled Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 103736 [details] [review] Bug 22630: Fix capitalization Home Library -> Home library Holding Library -> Holding library Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This is not a big change actually as it follows what is done for homebranch already. I'd be happy to see this in 20.05 as for the issues with the item editing in course reserves it already missed a few releases.
Nice work everyone! Pushed to master for 20.05
not backported to 19.11
Created attachment 105324 [details] [review] Bug 22630: Display correct "in storage" value
(In reply to Jonathan Druart from comment #59) > Created attachment 105324 [details] [review] [review] > Bug 22630: Display correct "in storage" value Last patch pushed to master for 20.05.