Bug 29002 adds a bookings foundation, but as the code progressed with feedback coming in the original unit tests didn't keep up. Tomas has agreed to push it without the tests so we can continue to test more widely and can onboard more developers working on the code.. but I do need to revisit it all and write tests for all the new modules and functions. This bug serves to store the tests as I write them.
Created attachment 161061 [details] [review] Bug 35248: Add unit test for Koha::Item->bookings relation This patch adds a simple unit test for the 'bookings' relation accessor added to Koha::Item. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161062 [details] [review] Bug 35248: Add unit tests for Koha::Item->find_booking This patch adds basic unit tests for the Koha::Item->find_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161608 [details] [review] Bug 35248: Unit tests for CanBookBeIssued Whilst writing the test, I found a minor flaw in the logic and fixed that in CanBookBeIssued at the same time.
Created attachment 161611 [details] [review] Bug 35248: Add unit tests for Koha::Item->check_booking This patch adds basic unit tests for the Koha::Item->check_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161620 [details] [review] Bug 35248: Drop ->place_booking These methods were introduced and used in an earlier iteration of the code, they are no longer called from anywhere (the code in Koha::Booking->store handles all cases instead) and thus I choose to remove them rather than add unit tests for them.. They are simple methods that could be re-introduced as required at a future date.
Created attachment 161621 [details] [review] Bug 35248: Add test for Koha::Exceptions::Booking
Created attachment 161632 [details] [review] Bug 35248: Remove Koha::Booking->intersects Another method left behind from an earlier iteration.. not actually called anywhere in the final submission in Koha.
Created attachment 161636 [details] [review] Bug 35248: Add unit tests for Koha::Booking relations This patch adds unit tests for the relations introduced in the new Koha::Booking class. Test plan 1) Run t/db_dependent/Koha/Booking.t
Created attachment 161651 [details] [review] Bug 35248: Add unit test for Koha::Item->bookings relation This patch adds a simple unit test for the 'bookings' relation accessor added to Koha::Item. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161652 [details] [review] Bug 35248: Add unit tests for Koha::Item->find_booking This patch adds basic unit tests for the Koha::Item->find_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161653 [details] [review] Bug 35248: Unit tests for CanBookBeIssued Whilst writing the test, I found a minor flaw in the logic and fixed that in CanBookBeIssued at the same time.
Created attachment 161654 [details] [review] Bug 35248: Add unit tests for Koha::Item->check_booking This patch adds basic unit tests for the Koha::Item->check_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161655 [details] [review] Bug 35248: Drop ->place_booking These methods were introduced and used in an earlier iteration of the code, they are no longer called from anywhere (the code in Koha::Booking->store handles all cases instead) and thus I choose to remove them rather than add unit tests for them.. They are simple methods that could be re-introduced as required at a future date.
Created attachment 161656 [details] [review] Bug 35248: Add test for Koha::Exceptions::Booking
Created attachment 161657 [details] [review] Bug 35248: Remove Koha::Booking->intersects Another method left behind from an earlier iteration.. not actually called anywhere in the final submission in Koha.
Created attachment 161658 [details] [review] Bug 35248: Add unit tests for Koha::Booking relations This patch adds unit tests for the relations introduced in the new Koha::Booking class. Test plan 1) Run t/db_dependent/Koha/Booking.t
Created attachment 161659 [details] [review] Bug 35248: Add Koha::Biblio->bookings unit test This patch adds unit tests for the bookings relationship accessor on Koha::Biblio objects. Test plan 1) Run t/db_dependant/Koha/Biblio.t
Created attachment 161660 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->check_booking This patch adds basic unit tests for Koha::Biblio->check_booking Test plan 1) Run t/db_dependent/Koha/Biblio.t
There's still more to come here.. but I really think as people are starting to enhance and expand on bookings now we need these in sooner rather than later... setting to NSO to start getting eyes and I'll keep throwing more tests into further bugs.
Created attachment 161821 [details] [review] Bug 35248: Add unit test for Koha::Item->bookings relation This patch adds a simple unit test for the 'bookings' relation accessor added to Koha::Item. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161822 [details] [review] Bug 35248: Add unit tests for Koha::Item->find_booking This patch adds basic unit tests for the Koha::Item->find_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161823 [details] [review] Bug 35248: Unit tests for CanBookBeIssued Whilst writing the test, I found a minor flaw in the logic and fixed that in CanBookBeIssued at the same time.
Created attachment 161824 [details] [review] Bug 35248: Add unit tests for Koha::Item->check_booking This patch adds basic unit tests for the Koha::Item->check_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t
Created attachment 161825 [details] [review] Bug 35248: Drop ->place_booking These methods were introduced and used in an earlier iteration of the code, they are no longer called from anywhere (the code in Koha::Booking->store handles all cases instead) and thus I choose to remove them rather than add unit tests for them.. They are simple methods that could be re-introduced as required at a future date.
Created attachment 161826 [details] [review] Bug 35248: Add test for Koha::Exceptions::Booking
Created attachment 161827 [details] [review] Bug 35248: Remove Koha::Booking->intersects Another method left behind from an earlier iteration.. not actually called anywhere in the final submission in Koha.
Created attachment 161828 [details] [review] Bug 35248: Add unit tests for Koha::Booking relations This patch adds unit tests for the relations introduced in the new Koha::Booking class. Test plan 1) Run t/db_dependent/Koha/Booking.t
Created attachment 161829 [details] [review] Bug 35248: Add Koha::Biblio->bookings unit test This patch adds unit tests for the bookings relationship accessor on Koha::Biblio objects. Test plan 1) Run t/db_dependant/Koha/Biblio.t
Created attachment 161830 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->check_booking This patch adds basic unit tests for Koha::Biblio->check_booking Test plan 1) Run t/db_dependent/Koha/Biblio.t
Created attachment 161831 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->bookable_items This patch adds basic unit tests for Koha::Biblio->bookable_items Test plan 1) Run t/db_dependent/Koha/Biblio.t
Created attachment 161861 [details] [review] Bug 35248: Add unit test for Koha::Item->bookings relation This patch adds a simple unit test for the 'bookings' relation accessor added to Koha::Item. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161862 [details] [review] Bug 35248: Add unit tests for Koha::Item->find_booking This patch adds basic unit tests for the Koha::Item->find_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161863 [details] [review] Bug 35248: Unit tests for CanBookBeIssued Whilst writing the test, I found a minor flaw in the logic and fixed that in CanBookBeIssued at the same time. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161864 [details] [review] Bug 35248: Add unit tests for Koha::Item->check_booking This patch adds basic unit tests for the Koha::Item->check_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161865 [details] [review] Bug 35248: Drop ->place_booking These methods were introduced and used in an earlier iteration of the code, they are no longer called from anywhere (the code in Koha::Booking->store handles all cases instead) and thus I choose to remove them rather than add unit tests for them.. They are simple methods that could be re-introduced as required at a future date. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161866 [details] [review] Bug 35248: Add test for Koha::Exceptions::Booking Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161867 [details] [review] Bug 35248: Remove Koha::Booking->intersects Another method left behind from an earlier iteration.. not actually called anywhere in the final submission in Koha. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161868 [details] [review] Bug 35248: Add unit tests for Koha::Booking relations This patch adds unit tests for the relations introduced in the new Koha::Booking class. Test plan 1) Run t/db_dependent/Koha/Booking.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161869 [details] [review] Bug 35248: Add Koha::Biblio->bookings unit test This patch adds unit tests for the bookings relationship accessor on Koha::Biblio objects. Test plan 1) Run t/db_dependant/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161870 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->check_booking This patch adds basic unit tests for Koha::Biblio->check_booking Test plan 1) Run t/db_dependent/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161871 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->bookable_items This patch adds basic unit tests for Koha::Biblio->bookable_items Test plan 1) Run t/db_dependent/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com>
I ran the tests before and after the patches were applied. I hope that is sufficient for testing. Testing notes using koha-testing-docker: 1. Tests pass before and after patches applied: prove t/db_dependent/Circulation.t prove t/Koha/Exceptions.t prove t/db_dependent/Koha/Item.t prove t/db_dependent/Koha/Booking.t (new tests) prove t/db_dependent/Koha/Biblio.t 2. Note: path to run the tests is t/db_dependent/Koha/Item.t (not db_dependant) for these patches: - Add unit test for Koha::Item->bookings relation - Add unit tests for Koha::Item->find_booking - Add unit tests for Koha::Item->check_booking - Add Koha::Biblio->bookings unit test
Created attachment 161890 [details] [review] Bug 35248: Add tests for Koha::Booking->store This patch adds tests for the Koha::Booking->store method. Test plan 1) Run t/db_dependent/Koha/Booking.t and confirm they all pass
Created attachment 161891 [details] [review] Bug 35248: Refactor - Move assign_item_for_booking This routine really belongs inside Koha::Booking rather than Koha::Biblio. This patch moves it there, renames with _ to highlight it's private nature. Test plan 1) Confirm that t/db_dependant/Koha/Booking.t still passes, specifically the 'store' test.
Thanks David, that's great :) I've added a couple of further patches on top now too.. and there's one last one outstanding that I'm working on today.. resetting to assigned whilst I get that one done.. the more you look the more you see aye.
Created attachment 161893 [details] [review] Bug 35248: Add tests for Koha::Booking->store This patch adds tests for the Koha::Booking->store method. Test plan 1) Run t/db_dependent/Koha/Booking.t and confirm they all pass
Created attachment 161894 [details] [review] Bug 35248: Refactor - Move assign_item_for_booking This routine really belongs inside Koha::Booking rather than Koha::Biblio. This patch moves it there, renames with _ to highlight it's private nature. Test plan 1) Confirm that t/db_dependant/Koha/Booking.t still passes, specifically the 'store' test.
Created attachment 161895 [details] [review] Bug 35248: Add tests for Koha::Booking->_assign_item_for_booking
Created attachment 161896 [details] [review] Bug 35248: Add tests for Koha::Booking->_assign_item_for_booking
Created attachment 161897 [details] [review] Bug 35248: Whitespace clean Jonathan highlighted some trailing whitespace.. I only see a few cases where a line only contains whitespace and I didn't see these caught by the QA script at the time of submission. Anyway, this removes the spaces
Created attachment 161898 [details] [review] Bug 35248: Add unit test for Koha::Item->bookings relation This patch adds a simple unit test for the 'bookings' relation accessor added to Koha::Item. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161899 [details] [review] Bug 35248: Add unit tests for Koha::Item->find_booking This patch adds basic unit tests for the Koha::Item->find_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161900 [details] [review] Bug 35248: Unit tests for CanBookBeIssued Whilst writing the test, I found a minor flaw in the logic and fixed that in CanBookBeIssued at the same time. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161901 [details] [review] Bug 35248: Add unit tests for Koha::Item->check_booking This patch adds basic unit tests for the Koha::Item->check_booking method. Test plan 1) Run t/db_dependant/Koha/Item.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161902 [details] [review] Bug 35248: Drop ->place_booking These methods were introduced and used in an earlier iteration of the code, they are no longer called from anywhere (the code in Koha::Booking->store handles all cases instead) and thus I choose to remove them rather than add unit tests for them.. They are simple methods that could be re-introduced as required at a future date. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161903 [details] [review] Bug 35248: Add test for Koha::Exceptions::Booking Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161904 [details] [review] Bug 35248: Remove Koha::Booking->intersects Another method left behind from an earlier iteration.. not actually called anywhere in the final submission in Koha. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161905 [details] [review] Bug 35248: Add unit tests for Koha::Booking relations This patch adds unit tests for the relations introduced in the new Koha::Booking class. Test plan 1) Run t/db_dependent/Koha/Booking.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161906 [details] [review] Bug 35248: Add Koha::Biblio->bookings unit test This patch adds unit tests for the bookings relationship accessor on Koha::Biblio objects. Test plan 1) Run t/db_dependant/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161907 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->check_booking This patch adds basic unit tests for Koha::Biblio->check_booking Test plan 1) Run t/db_dependent/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161908 [details] [review] Bug 35248: Add unit tests for Koha::Bibilio->bookable_items This patch adds basic unit tests for Koha::Biblio->bookable_items Test plan 1) Run t/db_dependent/Koha/Biblio.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161909 [details] [review] Bug 35248: Add tests for Koha::Booking->store This patch adds tests for the Koha::Booking->store method. Test plan 1) Run t/db_dependent/Koha/Booking.t and confirm they all pass Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161910 [details] [review] Bug 35248: Refactor - Move assign_item_for_booking This routine really belongs inside Koha::Booking rather than Koha::Biblio. This patch moves it there, renames with _ to highlight it's private nature. Test plan 1) Confirm that t/db_dependant/Koha/Booking.t still passes, specifically the 'store' test. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161911 [details] [review] Bug 35248: Add tests for Koha::Booking->_assign_item_for_booking Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 161912 [details] [review] Bug 35248: Whitespace clean Jonathan highlighted some trailing whitespace.. I only see a few cases where a line only contains whitespace and I didn't see these caught by the QA script at the time of submission. Anyway, this removes the spaces Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Some trouble with applying the dependencies here - will revisit later.
I just applied the whole tree this morning against the current master and it was seamless.. not sure what problems your facing?
I looked into this again: my problem is a "could not build fake ancestor" on bug 35782 right at the top of this tree. As it all still applies for you, maybe you could reattach the patches there for me?
Seems we're having sha1 issues.. branch pushed to https://github.com/PTFS-Europe/koha/tree/bug_35248
The last patch wouldn't apply or more, it doesn't appear to be needed anymore: M koha-tmpl/intranet-tmpl/prog/js/place_booking_modal.js Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/js/place_booking_modal.js No changes -- Patch already applied.
Pushed for 24.05! Well done everyone, thank you!
Sure about dependancy with Bug 35788 ? Can this be backported to 23.11.x ?
This should apply happily for 23.11.x Fridolin. The dependency was there to ensure we pushed in the right order for master and to discourage others working on bookings code prior to the tests being present. We aren't testing the code removed in the dependency bugs, but I think that's OK for 23.11.x at this stage.
Pushed to 23.11.x for 23.11.04
Missing 23.05.x dependencies, no backport.