preceeding_publication_title_id should be preceding_publication_title_id This causes issues in importing KBART files for bug 34788
Created attachment 155607 [details] [review] Bug 34789: Database and API update Currently the erm_eholdings_titles table has a field called preceeding_publication_title_id. This should be preceding_publication_title_id
Created attachment 155608 [details] [review] Bug 34789: DBIC file
Created attachment 155609 [details] [review] Bug 34789: Update usage throughout Koha modules and Vue components
Created attachment 155610 [details] [review] Bug 34789: Update unit tests
Test plan: prove -v t/db_dependent/api/v1/erm_eholdings_titles.t Run cypress test suite
Created attachment 155730 [details] [review] Bug 34789: Database and API update Currently the erm_eholdings_titles table has a field called preceeding_publication_title_id. This should be preceding_publication_title_id Signed-off-by: David Nind <david@davidnind.com>
Created attachment 155731 [details] [review] Bug 34789: DBIC file Signed-off-by: David Nind <david@davidnind.com>
Created attachment 155732 [details] [review] Bug 34789: Update usage throughout Koha modules and Vue components Signed-off-by: David Nind <david@davidnind.com>
Created attachment 155733 [details] [review] Bug 34789: Update unit tests Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. After applying patches run: updatedatabase, dbic, yarn build 2. To run cypress tests: time perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only (hope this is the correct way to do this - from https://wiki.koha-community.org/wiki/User:Victor_Grousset_-_tuxayo/Setup_Koha_environment_with_koha-testing-docker - see output - all cypress tests pass[1]) [1] Results from running Cypress tests: Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ ✔ KohaTable_spec.ts 00:03 1 1 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Agreements_spec.ts 00:31 5 5 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Dialog_spec.ts 00:09 4 4 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Licenses_spec.ts 00:20 5 5 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Packages_spec.ts 00:20 5 5 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Searchbar_spec.ts 00:04 4 4 - - - │ ├────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ✔ ERM/Titles_spec.ts 00:37 6 6 - - - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ✔ All specs passed! 02:07 30 30 - - - Done in 139.81s. all good Running [koha-shell kohadev -c "touch testing.success"]... real 2m20.061s user 0m31.341s sys 0m18.387s
Created attachment 155750 [details] [review] Bug 34789: Database and API update Currently the erm_eholdings_titles table has a field called preceeding_publication_title_id. This should be preceding_publication_title_id Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 155751 [details] [review] Bug 34789: DBIC file Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 155752 [details] [review] Bug 34789: Update usage throughout Koha modules and Vue components Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 155753 [details] [review] Bug 34789: Update unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 155754 [details] [review] Bug 34789: (QA follow-up) Improve output of database update * Adds description * Adds column names to success messages Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Hi Matt, small things: * the DBIC file should be marked with [DO NOT PUSH] to make it stand out a bit more. * the db update was missing the description, fixed in follow-up.
Noted, thanks Cait!
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.04
NOTE but it might be very urgent: I am making our regular "piloting production build", we're following 23.05, and here is the issue we just bumped into: ALTER TABLE erm_eholdings_titles RENAME COLUMN preceding_publication_title_id TO preceding_publication_title_id; This gives an atomic update error on older mariaDB because it appeared only: RENAME COLUMN: MariaDB starting with 10.5.2 Older compatible syntax probably should be: ALTER TABLE erm_eholdings_titles CHANGE COLUMN ...
( ALTER TABLE erm_eholdings_titles CHANGE COLUMN preceeding_publication_title_id preceding_publication_title_id varchar(255) DEFAULT NULL )
They are using preceeding_publication_title_id there: https://developer.ebsco.com/knowledge-services/holdingsiq/docs/downloading
(In reply to Jonathan Druart from comment #23) > They are using preceeding_publication_title_id there: > https://developer.ebsco.com/knowledge-services/holdingsiq/docs/downloading Fun... I looked it up - preceding seems to be UK spelling?
The official NISO standard documentation for KBART uses "preceding" - see page 29 here: https://groups.niso.org/higherlogic/ws/public/download/16900/RP-9-2014_KBART.pdf "preceeding" is an incorrect spelling, I think EBSCO need to change their docs! https://www.merriam-webster.com/dictionary/preceeding I'll add a follow-up to address Andrii's comments
I'll also test the EBSCO import - the titles will probably need amending before storing
Created attachment 155887 [details] [review] Bug 34789: (follow-up) Fix atomic update file Atomic update file had a versioning issue - now changed to be compatible with older versions Please backport
Tomas - see follow-up patch that needs pushing to master
(In reply to Matt Blenkinsop from comment #28) > Tomas - see follow-up patch that needs pushing to master Pushed to master, thanks.
follow-up pushed to 23.05.x for 23.05.04
Nice work everyone! Pushed to oldstable for 22.11.x