Bug 23838 highlighted the need to have a clear distinction between renewals and original issues. I propose the addition of a new table, issue_renewals, to store this data.
Torn, Statistics contains the majority of the data we want.. but, none of the fields are setup as foreign keys so we have no relationships and it will lead to some funky work having to take place in the API controllers. Contemplating
Created attachment 131593 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 131594 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 131595 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 131596 [details] [review] Bug 30275: Record renewals
Created attachment 131597 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 131598 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 131599 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count
Created attachment 131600 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 131601 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 131602 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout
Created attachment 131603 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Still needs unit tests.
Created attachment 131604 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 131605 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 131606 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 131607 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 131608 [details] [review] Bug 30275: Record renewals
Created attachment 131609 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 131610 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 131611 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count
Created attachment 131612 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 131613 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 131614 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout
Created attachment 131615 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Created attachment 131616 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 131879 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 131880 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 131881 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 131882 [details] [review] Bug 30275: Record renewals
Created attachment 131883 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 131884 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 131885 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset.
Created attachment 131886 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 131887 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 131888 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects.
Created attachment 131889 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Created attachment 131890 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 131897 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 131898 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 131899 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 131900 [details] [review] Bug 30275: Record renewals
Created attachment 131901 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 131902 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 131903 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset.
Created attachment 131904 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 131905 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 131906 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects.
Created attachment 131907 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Created attachment 131908 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 131909 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'.
Created attachment 131910 [details] [review] Bug 30275: Add basic unit test for fetching renewals
The simplest way to test this is to test bug 23838 which relies on this foundational patch.
Created attachment 133908 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 133909 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 133910 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 133911 [details] [review] Bug 30275: Record renewals
Created attachment 133912 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 133913 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 133914 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset.
Created attachment 133915 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 133916 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 133917 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects.
Created attachment 133918 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Created attachment 133919 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 133920 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'.
Created attachment 133921 [details] [review] Bug 30275: Add basic unit test for fetching renewals
Created attachment 133922 [details] [review] Bug 30275: (follow-up) Rebase fixes
Created attachment 133929 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate
Created attachment 133930 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch.
Created attachment 133931 [details] [review] Bug 30275: DBIC Schema Changes
Created attachment 133932 [details] [review] Bug 30275: Record renewals
Created attachment 133933 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean.
Created attachment 133934 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes
Created attachment 133935 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset.
Created attachment 133936 [details] [review] Bug 30275: renewals_count DBIC Update
Created attachment 133937 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema
Created attachment 133938 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects.
Created attachment 133939 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals`
Created attachment 133940 [details] [review] Bug 30275: Add alias to create renewal in api routes
Created attachment 133941 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'.
Created attachment 133942 [details] [review] Bug 30275: Add basic unit test for fetching renewals
Created attachment 133943 [details] [review] Bug 30275: (follow-up) Rebase fixes
Created attachment 133944 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline.
Created attachment 133969 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133970 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133971 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133972 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133973 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133974 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133975 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133976 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133977 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133978 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133979 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133980 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133981 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133982 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133983 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 133984 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 134177 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134178 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134179 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134180 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134181 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134182 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134183 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134184 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134185 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134186 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134187 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134188 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134189 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134190 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134191 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134192 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134193 [details] [review] Bug 30275: (QA follow-up) Rename columns to match API This patch performs the following column renames: * id => renewal_id * issue_id => checkout_id The idea is that no translation is needed for the API, and also, being a new table, we can educate the users into the 'to be' terminology we are leaning towards, instead of having them learn one naming to create reports and then need to translate them once we normalize things in a future. That said, this is simple to review. Apply this patch and repeat the test plan. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I'll finish my review tomorrow. Overall, this is what I wanted to see on bug 23838, so I'm happy for this, Martin. Great! It is lacking (at least) tests for the new Koha::Checkouts::Renewal->store method. I can add it, but got late today to do it. I would like to see some higher-level method, like Koha::Checkout->renew and start removing old code but we can do that in a follow-up, this is solid as it stands.
(In reply to Tomás Cohen Arazi from comment #118) > I would like to see some higher-level method, like Koha::Checkout->renew and > start removing old code but we can do that in a follow-up +1
Awesome, thanks for the review Tomas. I'm happy to work on tests today, revisiting it yesterday my intention was to add some more tests as I also found them to be lacking. I got held up as it appears Circulation.t is failing already due to the constraint added here.. but I can't for the life of me work out why as the data does appear to be present and correct in k-t-d where I was testing.
Created attachment 134210 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user.
Created attachment 134228 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Sponsored-by: Loughborough University Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134229 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134230 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134231 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134232 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134233 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134234 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134235 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134236 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134237 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134238 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134239 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134240 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134241 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134242 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134243 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134244 [details] [review] Bug 30275: (QA follow-up) Rename columns to match API This patch performs the following column renames: * id => renewal_id * issue_id => checkout_id The idea is that no translation is needed for the API, and also, being a new table, we can educate the users into the 'to be' terminology we are leaning towards, instead of having them learn one naming to create reports and then need to translate them once we normalize things in a future. That said, this is simple to review. Apply this patch and repeat the test plan. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134245 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user.
Created attachment 134246 [details] [review] Bug 30275: Add unit tests for AddRenewal addition This patch adds a test for the AddRenewal addition that creates Renewal lines. Test plan 1) Run the unit tests and confirm it passes
Created attachment 134247 [details] [review] Bug 30275: Add Unit test for renewals relation
Created attachment 134248 [details] [review] Bug 30275: Unit Tests for Checkouts::Renewal
Lots of unit tests.. all praise the unit tests :)
Created attachment 134470 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134471 [details] [review] Bug 30275: Add unit tests for AddRenewal addition This patch adds a test for the AddRenewal addition that creates Renewal lines. Test plan 1) Run the unit tests and confirm it passes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134472 [details] [review] Bug 30275: Add Unit test for renewals relation Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134473 [details] [review] Bug 30275: Unit Tests for Checkouts::Renewal Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134869 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Sponsored-by: Loughborough University Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134870 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134871 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134872 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134873 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134874 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134875 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134876 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134877 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134878 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134879 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134880 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134881 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134882 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134883 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134884 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134885 [details] [review] Bug 30275: (QA follow-up) Rename columns to match API This patch performs the following column renames: * id => renewal_id * issue_id => checkout_id The idea is that no translation is needed for the API, and also, being a new table, we can educate the users into the 'to be' terminology we are leaning towards, instead of having them learn one naming to create reports and then need to translate them once we normalize things in a future. That said, this is simple to review. Apply this patch and repeat the test plan. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134886 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134887 [details] [review] Bug 30275: Add unit tests for AddRenewal addition This patch adds a test for the AddRenewal addition that creates Renewal lines. Test plan 1) Run the unit tests and confirm it passes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134888 [details] [review] Bug 30275: Add Unit test for renewals relation Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134889 [details] [review] Bug 30275: Unit Tests for Checkouts::Renewal Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Just another rebase to keep it fresh
Please run the QA scripts, there are tiny issues to fix. There are also several tests that are failing.
Created attachment 135123 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Sponsored-by: Loughborough University Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135124 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135125 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135126 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135127 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135128 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135129 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135130 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135131 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135132 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135133 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135134 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135135 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135136 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135137 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135138 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135139 [details] [review] Bug 30275: (QA follow-up) Rename columns to match API This patch performs the following column renames: * id => renewal_id * issue_id => checkout_id The idea is that no translation is needed for the API, and also, being a new table, we can educate the users into the 'to be' terminology we are leaning towards, instead of having them learn one naming to create reports and then need to translate them once we normalize things in a future. That said, this is simple to review. Apply this patch and repeat the test plan. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135140 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135141 [details] [review] Bug 30275: Add unit tests for AddRenewal addition This patch adds a test for the AddRenewal addition that creates Renewal lines. Test plan 1) Run the unit tests and confirm it passes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135142 [details] [review] Bug 30275: Add Unit test for renewals relation Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 135143 [details] [review] Bug 30275: Unit Tests for Checkouts::Renewal Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #170) > Please run the QA scripts, there are tiny issues to fix. > > There are also several tests that are failing. QA Script re-run... I've fixed the executable bit issue, and fixed up the touched lines where the script detected tabs (as discussed on IRC, I fixed those up, but the rest of the file is still tabbed) The git manip in this case I feel is helpful in the history to be able to follow the thought process.. I could possibly merge it and add a not to the commit merged.. but I've not delved into it yet. POD issue is just the standard koha_object_class stuff we add in all Schema files. Finally.. I couldn't replicate any unit test failures.. for me they all pass first time (after a reset) and subsequent runs also pass too :?
(In reply to Martin Renvoize from comment #192) > Finally.. I couldn't replicate any unit test failures.. for me they all > pass first time (after a reset) and subsequent runs also pass too :? Hum, confirmed. Cannot recreate today. Certainly something wrong with my DB yesterday.
(In reply to Jonathan Druart from comment #193) > (In reply to Martin Renvoize from comment #192) > > Finally.. I couldn't replicate any unit test failures.. for me they all > > pass first time (after a reset) and subsequent runs also pass too :? > > Hum, confirmed. Cannot recreate today. Certainly something wrong with my DB > yesterday. Maybe worth adding a test for 2022-05-18? Kidding
Created attachment 137146 [details] [review] Bug 30275: Add checkout_renewals table This patch adds the new checkout_renewals table using an atomicupdate Sponsored-by: Loughborough University Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137147 [details] [review] Bug 30275: Generated kohastructure.sql This kohastructure update was generated using `koha-dump --schema-only` in koha-testing-docker once the atomicupdate had been run against the previous atomicupdate patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137148 [details] [review] Bug 30275: DBIC Schema Changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137149 [details] [review] Bug 30275: Record renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137150 [details] [review] Bug 30275: DBIC Schema Additions Add checkout and old_checkout relations to CheckoutRenewal schema and mark 'seen' as a boolean. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137151 [details] [review] Bug 30275: Add Koha::Objects for Renewals Add Koha::Checkouts::Renewals|Renewal classes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137152 [details] [review] Bug 30275: Rename issues.renewals to issues.renewals_count Rename the issues.renewals field to renewals_count to prevent a method name collision with the new relation accessor introduced by this patchset. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137153 [details] [review] Bug 30275: renewals_count DBIC Update Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137154 [details] [review] Bug 30275: Add 'renewals' relation to Issue Schema Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137155 [details] [review] Bug 30275: Add renewals relation to Koha::Checkout Add the new 'renewals' relation to Koha::Checkout to return a list of Koha::Checkouts::Renewal objects. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137156 [details] [review] Bug 30275: Add `/api/v1/checkouts/{checkout_id}/renewals` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137157 [details] [review] Bug 30275: Add alias to create renewal in api routes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137158 [details] [review] Bug 30275: Fix checkouts.t unit test We no longer require a mocked userenv for these tests to pass, we're setting the userenv in api auth now. The mock just serves to break tests for the renew method now that AddRenewal required a correctly set userenv for 'number'. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137159 [details] [review] Bug 30275: Add basic unit test for fetching renewals Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137160 [details] [review] Bug 30275: (follow-up) Rebase fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137161 [details] [review] Bug 30275: Add renewals relation to Koha::Old::Checkout Add the new 'renewals' relation to Koha::Old::Checkout to return a list of Koha::Checkouts::Renewal objects. We also add the same relation to the OldIssue schema inline. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137162 [details] [review] Bug 30275: (QA follow-up) Rename columns to match API This patch performs the following column renames: * id => renewal_id * issue_id => checkout_id The idea is that no translation is needed for the API, and also, being a new table, we can educate the users into the 'to be' terminology we are leaning towards, instead of having them learn one naming to create reports and then need to translate them once we normalize things in a future. That said, this is simple to review. Apply this patch and repeat the test plan. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137163 [details] [review] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137164 [details] [review] Bug 30275: Add unit tests for AddRenewal addition This patch adds a test for the AddRenewal addition that creates Renewal lines. Test plan 1) Run the unit tests and confirm it passes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137165 [details] [review] Bug 30275: Add Unit test for renewals relation Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 137166 [details] [review] Bug 30275: Unit Tests for Checkouts::Renewal Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.11. Nice work everyone, thanks!
Created attachment 137217 [details] [review] Bug 30275: (QA follow-up) Fix TestBuilder.t This patch adds 'CheckoutRenewal.checkout_id' to the list of non-foreign key relations found in _should_be_fk.
Follow-up pushed to master. Thanks Martin!
Created attachment 137598 [details] [review] Bug 30275: (QA follow-up) Trivial fix to schema file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Enhancement will not be backported to the 22.05.x series
This introduced a backward compatibility break in the REST API by renaming the renewals field in checkouts to renewals_count. Isn't the idea of a v1 API to keep it backward compatible? Could the renewals field be added back as an alias for renewals_count?
(In reply to Ere Maijala from comment #221) > Isn't the idea of a v1 API to keep it backward compatible? I think that's the idea although I don't think we've really done that in practice, unfortunately. I suppose one could argue that the v1 API isn't even complete yet... (In reply to Ere Maijala from comment #221) > Could the renewals field be added back as an alias for renewals_count? I don't know the API code well enough, but that sounds like a reasonable idea to me.
(In reply to Ere Maijala from comment #221) > API to keep it backward compatible? Could the renewals field be added back > as an alias for renewals_count? I agree this is annoying. The main reason for the change in the name was to reuse the 'renewals' attribute for x-koha-embed(ding) the renewals. Not sure what to do. If this is critical for the vuFind integration maintainers we could reconsider.
(In reply to Tomás Cohen Arazi from comment #223) > (In reply to Ere Maijala from comment #221) > > API to keep it backward compatible? Could the renewals field be added back > > as an alias for renewals_count? > > I agree this is annoying. The main reason for the change in the name was to > reuse the 'renewals' attribute for x-koha-embed(ding) the renewals. > > Not sure what to do. If this is critical for the vuFind integration > maintainers we could reconsider. Fortunately this one is not critical, and since the change had already shipped in a released Koha version, going back could cause even worse confusion. VuFind has been adjusted (see https://github.com/vufind-org/vufind/commit/7d8b08fb43027dfaec2ef8417ba34c8aadcfe2e1), though a new release is needed for everyone to get the fix. Since renewals will be re-used (and I think for a good function), my suggestion for an alias is not a good one either. At this point I suppose what's left is a wish to avoid such changes in the future, but if they're needed, at least publish a prominent note about the BC break in release notes. v1 API being incomplete is not a problem as long as new functionality doesn't break compatibility. And even if the v1 API was considered "alpha" level and subject to change, it's really difficult for an API consumer to manage as they might not even be aware of a Koha upgrade being done. I understand it's difficult to always stay compatible, and others have failed as well even when they've introduced changes with new API versions.
There was a problem reported on the koha-devel mailing list that could relate to this patch set or the feature in general: https://lists.koha-community.org/pipermail/koha-devel/2024-April/048583.html "I have updated our database from koha 21.11.10 to the current Koha 23.11.04. When searching the catalogue I get a result list, but when clicking a single hit I get an error 500. In file "plack-error.log" I have found the following line which could be the reason: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'issue.renewals_count' in 'field list' at /usr/share/koha/lib/Koha/Objects.pm line 317" Could you maybe have a look if you can help to identify the source of the issue?