Currently different ILL backends return metadata in disparate property names, for example, an article will have the property "./metadata/titleLevel/title" for it's containing journal title on the BLDSS backend, whereas the FreeForm backend will store it in "title". Some work was done by Martin in Bug 20640 that introduced the mapping of certain metadata properties to a common schema. For example, the property described above is mapped to "container_title". This mapping work currently only exists for the BLDSS backend. Work on Bug 21063 has highlighted the fact that these metadata properties vary according to the backend. This bug proposes extending the mapping to other backends. This will be carried out for the FreeForm backend by Andrew Isherwood.
It's more than likely this work won't touch core Koha, I just thought it was worth creating a bug to highlight that this will need to be done for any other backends in existence. I'll post details of what was done to the FreeForm backend once it is complete.
Created attachment 77035 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties: - container_title (this should correspond with 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author')
Comment on attachment 77035 [details] [review] Bug 21079: Unify metadata schema across backends Patch contains debugging code, obsoleting
Created attachment 77036 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties: - container_title (this should correspond with 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author')
So this bug did touch Koha core after all. We need to transform existing article FreeForm request metadata in order to copy some properties to new metadata properties with names that correspond with our unified schema.
Created attachment 79720 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties: - container_title (this should correspond with 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author')
Created attachment 79725 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties (if their values were originally populated): - container_title (this should correspond with what *was* 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author')
Created attachment 80288 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties (if their values were originally populated): - container_title (this should correspond with what *was* 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author') Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Created attachment 81740 [details] [review] Bug 21079: Unify metadata schema across backends This patch contains a database upgrade that takes appropriate metadata properties relating to articles in the FreeForm backend and creates metadata that corresponds with the metadata being created by the BLDSS backend. This enables us to create templates that can display metadata equally across any backends that contain this metadata; To test: - Ensure you have at least one article request created with the FreeForm backend - Check the metadata for the request: => TEST: You should have properties such as 'article_title', 'article_author' populated => TEST: You should NOT have properties such as 'container_title' & 'pages' - Run the upgrade => TEST: For the same requests you should now have the following properties (if their values were originally populated): - container_title (this should correspond with what *was* 'title') - title (this should correspond with 'article_title') - pages (this should correspond with 'article_pages') - author (this should correspond with 'article_author') Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
This adds Try::Tiny to the updatedatabase, I think this is okay and there seems to be agreement polling the QA team. The AutoCommit and RaiseError flags need to be reset after the attempt. It appears the version is set even if the update fails, is this intentional?
(In reply to Nick Clemens from comment #10) > This adds Try::Tiny to the updatedatabase, I think this is okay and there > seems to be agreement polling the QA team. There will have been some logic behind taking this approach, though I can't remember what it was now. Investigating, the common approach in the DBI community appears to be to "eval" the contents of what I've got inside my "try" block, then test for "$@" to determine an error. I'm happy to switch to that approach if it would make people happier? > The AutoCommit and RaiseError flags need to be reset after the attempt. Good spot! Thanks. > It appears the version is set even if the update fails, is this intentional? Nope, an error, thanks!
Created attachment 82092 [details] [review] Bug 21079: (follow-up) Move from Try::Tiny to eval In line with similar examples already in Koha, we now eval the transaction, instead of putting it in a try/catch block. This patch also addressed the other comments made in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21079#c10 - AutoCommit & RaiseError are now reset - Don't set the version if the update fails
Awesome work all! Pushed to master for 18.11
The enhancement will not be backported to 18.05.x series.