Description
Kyle M Hall (khall)
2014-10-02 14:48:56 UTC
Created attachment 31979 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Run misc/maintenance/touch_all_items.pl 5) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Created attachment 31980 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Run misc/maintenance/touch_all_items.pl 5) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic I tried testing created items with non-public notes apply patch upgrade database map itemnotes_nonpublic to 952x run touch_all_items.pl all non-public notes still seemed to be in more_sbfields_xml new notes worked as expected but I couldn't get old notes to transfer Created attachment 32400 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Run misc/migration_tools/copy_item_xml_field_to_db.pl --subfield x --column itemnotes_nonpublic -c -v 5) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Created attachment 32401 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/migration_tools/copy_item_xml_field_to_db.pl --subfield x --column itemnotes_nonpublic -c -v 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Thanks for testing! The script touch_all_items does not do quite what I thought it did. We need a facility to copy data from more_subfields_xml to the database. As far as I can tell Koha does not have one, so I've written a script to do so and have included it in the patch. Please give it another try! Please note I've updated the test plan to include this. (In reply to Nick Clemens from comment #3) > I tried testing > > created items with non-public notes > apply patch > upgrade database > map itemnotes_nonpublic to 952x > run touch_all_items.pl > > all non-public notes still seemed to be in more_sbfields_xml > > new notes worked as expected but I couldn't get old notes to transfer I've discovered bug 11399 has a much better implementation for updating items. I will be removing my script from this patch and adding that bug as a dependency. Created attachment 32513 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/batchRebuildItemsTables.pl 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Created attachment 32663 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes I've applied against mater 3.17.00.030 IMO everything is OK. I pass the patch to "Signed OFF" status. ------------------- On "Koha to MARC mapping", items' table, I saw these 2 rows about 952-x: itemnotes_nonpublic 952 x Non-public note paidfor 952 x Non-public note I asked to edit paidfor: no text was at the "900s" row. Anyway I unmapped paidfor, and in the items' table I saw: itemnotes_nonpublic 952 x Non-public note paidfor ------------------------- I am not sure why this is marked as a major bug - improving the display of the internal notes seems like an enhancement? This patch is not clean (see the updatedatabase.pl changes). Kyle, could you have a look please? Created attachment 33501 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/batchRebuildItemsTables.pl 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Signed-off-by: Paola Rossi <paola.rossi@cineca.it> I have been working on a patch to clean up the frameworks, as we still have some mappings to 'paidfor' in the fast add frameworks (inconsistent mappings - ew!). But I think with this bug it would make more sense to update all the frameworks to include a mapping to the new column. Created attachment 34361 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/batchRebuildItemsTables.pl 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Created attachment 34366 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/batchRebuildItemsTables.pl 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> I think we still need the framework changes here - or the problem will not be fully solved - we should fix the new installations. So this one has been sitting there for 1 months time. I've done some full out testing and I am passing this. Since this was done by Kyle I would encourage another QA to look at this too, if you have the time. Release manager if you feel that this must have another QA, let me know and I'll see who I can bug. This is pretty solid though and I think it would be ok to push. I leave that up to you all - and will acceptable whatever is decided. (My Goal is to just get a little more attention to this) Thanks, Brendan Gallagher Created attachment 34416 [details] [review] Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes An item's public notes are mapped to a db column, but the librarian only notes are not. We need to map these so in the future we may display them in places other than the editor. Test Plan: 1) Create an item with a non-public note 2) Apply this patch 3) Run updatedatabase 4) Update your frameworks to map your nonpublic item note to items.itemnotes_nonpublic 5) Run misc/batchRebuildItemsTables.pl 6) Inspect the database, note that your non-public note is now in items.itemnotes_nonpublic Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 34417 [details] [review] Bug 13023 [QA Followup] - Add db mapping to default marc21 frameworks Created attachment 34418 [details] [review] Bug 13023 [QA Followup] - Add db mapping to default marc21 frameworks Created attachment 34427 [details] [review] Bug 13023 [QA Followup] - Add db mapping to frameworks Created attachment 34428 [details] [review] Bug 13023 [QA Followup] - Add db mapping to frameworks (In reply to Brendan Gallagher from comment #17) > So this one has been sitting there for 1 months time. I've done some full > out testing and I am passing this. Since this was done by Kyle I would > encourage another QA to look at this too, if you have the time. Release > manager if you feel that this must have another QA, let me know and I'll see > who I can bug. This is pretty solid though and I think it would be ok to > push. I leave that up to you all - and will acceptable whatever is decided. > (My Goal is to just get a little more attention to this) > > Thanks, > Brendan Gallagher Thanks for posting this Brendan! I'll be so glad when this finally gets through. Martin recently suggested that I should join the QA team. When he suggested it, I didn't think I had the time, but that would've come in handy about now. Hope this gets the attention it deserves! Created attachment 35611 [details] [review] Bug 13023 [QA Followup] - Use db column for storage on item creation Created attachment 38428 [details] [review] Bug 13023: (RM followup) DBIx updates Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Patches pushed to master. Thanks Kyle! Why does the db rev of this report not update kohafield for existing installs? If 952$x has not been connected to some other field, what would be the problem in connecting it to itemnotes_nonpublic just as you do for the new install? Because the data is still in more_subfields_xml or items.paidfor (depending on the age of the installation) you will have problems accessing your data. I did a lot of tests when we tried to do that in the first iteration and there always remained problems (see bug 4222). (In reply to Katrin Fischer from comment #28) > Because the data is still in more_subfields_xml or items.paidfor (depending > on the age of the installation) you will have problems accessing your data. > > I did a lot of tests when we tried to do that in the first iteration and > there always remained problems (see bug 4222). I'm looking at this right now... I used admin/koha2marclinks.pl to update my frameworks to use the itemnotes_nonpublic column for 952$x, and if I manually edit an item, I see the data from more_subfields_xml, and when I re-save it, it moves into itemnotes_nonpublic. However, I'm trying to automate this to do thousands of items... and I'm finding "touch_all_items.pl" isn't doing the trick... -- It looks like admin/koha2marclinks.pl just runs the following SQL: "update marc_subfield_structure set kohafield='$tablename.$kohafield' where tagfield='$temp[0]' and tagsubfield='$temp[1]'" So that can be scripted easily enough... As for updating the item records...I think maybe what I need to do is run GetMarcItem, which runs Item2Marc, and then maybe run ModItemFromMarc. my $item_marc = GetMarcItem($biblionumber, $itemnumber); my $item = ModItemFromMarc($item_marc, $biblionumber, $itemnumber); I've tried that on a few records, and that seems to do the trick. -- I wonder what we should do about this... add it as a maintenance script? Do we add it to updatedatabase.pl? David, take a look at: misc/batchRebuildItemsTables.pl -c But beware, it's slow. (In reply to Katrin Fischer from comment #30) > David, take a look at: misc/batchRebuildItemsTables.pl -c > But beware, it's slow. You're a champion, Katrin :). (In reply to Katrin Fischer from comment #30) > David, take a look at: misc/batchRebuildItemsTables.pl -c > But beware, it's slow. I only had a few thousand records to touch in a database with over a million records, so I gave it one of these and it completed pretty quickly: perl batchRebuildItemsTables.pl -c --where="biblio.biblionumber in (select biblionumber from items where more_subfields_xml is not null group by biblionumber)" (In reply to David Cook from comment #32) > (In reply to Katrin Fischer from comment #30) > > David, take a look at: misc/batchRebuildItemsTables.pl -c > > But beware, it's slow. > > I only had a few thousand records to touch in a database with over a million > records, so I gave it one of these and it completed pretty quickly: > > perl batchRebuildItemsTables.pl -c --where="biblio.biblionumber in (select > biblionumber from items where more_subfields_xml is not null group by > biblionumber)" That is, it took about 75 seconds to do 2354 records. |