Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes
Summary: Nonpublic note for items should be mapped to a database column like itemnotes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 11399
Blocks: 13024
  Show dependency treegraph
 
Reported: 2014-10-02 14:48 UTC by Kyle M Hall
Modified: 2017-07-26 03:02 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.36 KB, patch)
2014-10-02 14:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.37 KB, patch)
2014-10-02 14:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (19.79 KB, patch)
2014-10-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (19.87 KB, patch)
2014-10-16 11:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (17.87 KB, patch)
2014-10-17 14:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (18.23 KB, patch)
2014-10-24 11:20 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.52 KB, patch)
2014-11-12 19:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.53 KB, patch)
2014-12-12 17:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.58 KB, patch)
2014-12-12 19:56 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13023 - Nonpublic note for items should be mapped to a database column like itemnotes (3.59 KB, patch)
2014-12-15 13:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 [QA Followup] - Add db mapping to default marc21 frameworks (13.72 KB, patch)
2014-12-15 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 [QA Followup] - Add db mapping to default marc21 frameworks (13.72 KB, patch)
2014-12-15 13:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 [QA Followup] - Add db mapping to frameworks (30.18 KB, patch)
2014-12-15 14:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 [QA Followup] - Add db mapping to frameworks (31.36 KB, patch)
2014-12-15 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023 [QA Followup] - Use db column for storage on item creation (1.40 KB, patch)
2015-01-29 16:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13023: (RM followup) DBIx updates (2.74 KB, patch)
2015-04-23 15:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-10-02 14:48:56 UTC
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.
Comment 1 Kyle M Hall 2014-10-02 14:51:09 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2014-10-02 14:58:51 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2014-10-04 20:51:12 UTC
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
Comment 4 Kyle M Hall 2014-10-16 11:31:53 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2014-10-16 11:33:10 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2014-10-16 11:35:26 UTC
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
Comment 7 Kyle M Hall 2014-10-17 14:54:37 UTC
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.
Comment 8 Kyle M Hall 2014-10-17 14:58:55 UTC Comment hidden (obsolete)
Comment 9 Paola Rossi 2014-10-24 11:20:41 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2014-11-06 06:08:32 UTC
I am not sure why this is marked as a major bug - improving the display of the internal notes seems like an enhancement?
Comment 11 Jonathan Druart 2014-11-12 14:16:26 UTC
This patch is not clean (see the updatedatabase.pl changes).
Kyle, could you have a look please?
Comment 12 Kyle M Hall 2014-11-12 19:08:06 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-11-29 11:39:08 UTC
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.
Comment 14 Kyle M Hall 2014-12-12 17:55:21 UTC Comment hidden (obsolete)
Comment 15 Brendan Gallagher 2014-12-12 19:56:30 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2014-12-12 20:00:20 UTC
I think we still need the framework changes here - or the problem will not be fully solved - we should fix the new installations.
Comment 17 Brendan Gallagher 2014-12-12 20:01:31 UTC
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
Comment 18 Kyle M Hall 2014-12-15 13:16:51 UTC
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>
Comment 19 Kyle M Hall 2014-12-15 13:17:35 UTC Comment hidden (obsolete)
Comment 20 Kyle M Hall 2014-12-15 13:19:16 UTC Comment hidden (obsolete)
Comment 21 Kyle M Hall 2014-12-15 14:00:47 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2014-12-15 14:01:52 UTC
Created attachment 34428 [details] [review]
Bug 13023 [QA Followup] - Add db mapping to frameworks
Comment 23 David Cook 2014-12-16 06:23:02 UTC
(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!
Comment 24 Kyle M Hall 2015-01-29 16:24:30 UTC
Created attachment 35611 [details] [review]
Bug 13023 [QA Followup] - Use db column for storage on item creation
Comment 25 Tomás Cohen Arazi 2015-04-23 15:43:35 UTC
Created attachment 38428 [details] [review]
Bug 13023: (RM followup) DBIx updates

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 26 Tomás Cohen Arazi 2015-04-23 16:12:09 UTC
Patches pushed to master.

Thanks Kyle!
Comment 27 Marcel de Rooy 2015-05-04 07:52:01 UTC
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?
Comment 28 Katrin Fischer 2015-05-04 08:06:59 UTC
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).
Comment 29 David Cook 2016-06-07 07:05:48 UTC
(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?
Comment 30 Katrin Fischer 2016-06-07 07:54:24 UTC
David, take a look at: misc/batchRebuildItemsTables.pl -c
But beware, it's slow.
Comment 31 David Cook 2016-06-28 05:01:25 UTC
(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 :).
Comment 32 David Cook 2017-07-26 03:01:33 UTC
(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)"
Comment 33 David Cook 2017-07-26 03:02:04 UTC
(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.