Bug 7263 - Determine maximum length of some fields or subfields when cataloguing
Summary: Determine maximum length of some fields or subfields when cataloguing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: 3.8
Hardware: All All
: P3 critical (vote)
Assignee: Juan Romay Sieira
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks: 7850 7855 8018
  Show dependency treegraph
 
Reported: 2011-11-24 17:54 UTC by Juan Romay Sieira
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Path to this enhancement (15.80 KB, patch)
2011-11-24 18:00 UTC, Juan Romay Sieira
Details | Diff | Splinter Review
Patch to apply in current master (15.80 KB, patch)
2011-12-19 09:42 UTC, Juan Romay Sieira
Details | Diff | Splinter Review
Patch to current master (15.24 KB, patch)
2012-02-09 17:39 UTC, Juan Romay Sieira
Details | Diff | Splinter Review
Patch to apply in current master (15.39 KB, patch)
2012-03-20 15:22 UTC, Juan Romay Sieira
Details | Diff | Splinter Review
patch signed off (15.50 KB, patch)
2012-03-21 18:23 UTC, laurenthdl
Details | Diff | Splinter Review
patch signed off (15.49 KB, patch)
2012-03-23 09:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Juan Romay Sieira 2011-11-24 17:54:54 UTC
Determine maximum length of some fields or subfields when cataloguing a biblio or an item.
Some libraries need to specify the maximum number of characters you can have a field or subfield, whether cataloguing a biblio or an item.
For default it will be 9999 max length.
Comment 1 Juan Romay Sieira 2011-11-24 18:00:57 UTC Comment hidden (obsolete)
Comment 2 Duncan 2011-12-18 22:45:07 UTC
After fixing a merge error in the updatedatabase.pl file the patch applied fine.
The max length field can successfully be found where it is supposed to be.

However, applying the patch also prevented me from being able to add new MARC records. On going to the add screen none of the input fields are present and it just remains blank.
Comment 3 Katrin Fischer 2011-12-19 05:20:02 UTC
Hi Duncan, from your description I think this is 'failed qa'.
Comment 4 Juan Romay Sieira 2011-12-19 09:42:28 UTC Comment hidden (obsolete)
Comment 5 Juan Romay Sieira 2011-12-19 09:47:14 UTC
I applied the patch again and haven't found any error. I setup a max length to  one of the subfields of the 245 and has done what it supposed to do.

I uploaded another patch compatible with the current master.
Comment 6 Juan Romay Sieira 2012-02-09 17:39:25 UTC Comment hidden (obsolete)
Comment 7 Jared Camins-Esakov 2012-03-11 22:17:27 UTC
The update included with this patch is MARC21-specific, but will operate on UNIMARC databases as well. Please correct the update to handle the three supported MARC flavours appropriately.
Comment 8 Juan Romay Sieira 2012-03-20 15:22:03 UTC Comment hidden (obsolete)
Comment 9 laurenthdl 2012-03-21 18:23:37 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2012-03-23 09:59:16 UTC
Created attachment 8561 [details] [review]
patch signed off

Previous signedoff patch does not apply (fatal: sha1 information is lacking or useless (C4/Biblio.pm).
Repository lacks necessary blobs to fall back on 3-way merge.)

This one is good.

Marked as Passed QA
Comment 11 Magnus Enger 2012-03-30 07:04:41 UTC
As far as I can see, this adds the maxlength column to installations that are upgraded, but what about fresh installs? Shouldn't maxlength be added to all existing frameworks, or am I missing something?
Comment 12 Jonathan Druart 2012-03-30 07:32:49 UTC
Hi Magnus, 

The patch adds a default value for this new column:
  `maxlength` int(4) NOT NULL DEFAULT '9999'

You shouldn't have a problem for a fresh install with a default value.
Comment 13 Magnus Enger 2012-03-30 07:41:21 UTC
But updatedatabase changes the default for a couple of fields: 

+$DBversion = "3.07.00.XXX";
+if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
+    $dbh->do("
+       ALTER TABLE  `marc_subfield_structure` ADD  `maxlength` INT( 4 ) NOT NULL DEFAULT  '9999';
+       ");
+       $dbh->do("
+       UPDATE `marc_subfield_structure` SET maxlength=24 WHERE tagfield='000';
+       ");
+       $dbh->do("
+       UPDATE marc_subfield_structure SET maxlength = IF ((SELECT value FROM systempreferences WHERE variable = 'marcflavour')='MARC21','40','9999') WHERE tagfield='008';
+       ");
+       $dbh->do("
+       UPDATE marc_subfield_structure SET maxlength = IF ((SELECT value FROM systempreferences WHERE variable = 'marcflavour')='UNIMARC','36','9999') WHERE tagfield='100';
+       ");
+    print "Upgrade to $DBversion done (Add new field maxlength to marc_subfield_structure)\n";
+    SetVersion($DBversion);
+}
+

So all marcflavours on upgraded installations will have maxlength = 24 for 000, while fresh installs will have 9999. 

And upgraded MARC21 installations will have maxlength = 40 for field 008 while fresh installations will have maxlength = 9999. 

Might not be much of a problem, but it will lead to subtle differences in installations depending on their history and perhaps a potential for future confusion.
Comment 14 Juan Romay Sieira 2012-03-30 07:58:03 UTC
You're not missing nothing, I missed to include in fresh installs this change. I'll push in a few minutes those files to update fields like 000, 008, 100 (unimarc)...
Comment 15 Magnus Enger 2012-03-30 08:03:00 UTC
(In reply to comment #14)
> You're not missing nothing, I missed to include in fresh installs this
> change. I'll push in a few minutes those files to update fields like 000,
> 008, 100 (unimarc)...

Yay! 

I'm working on a patch to change some things in the NORMARC frameworks anyway (bug 7850), so I'll include the maxlength column for all subfields there, with 000 = 24 and 008 = 40. That should take care of fresh NORMARC installs. 

To be complete, updatedatabase.pl should do this part for NORMARC too: 

UPDATE marc_subfield_structure SET maxlength = IF ((SELECT value FROM systempreferences WHERE variable = 'marcflavour')='NORMARC','40','9999') WHERE tagfield='008';
Comment 16 Katrin Fischer 2012-04-05 05:44:44 UTC
Hm, bit confused here - from the comments I think we still need a patch to fix the fields on upgraded installations?
I am changing bug status - please reset if I missed something!
Comment 17 Magnus Enger 2012-04-05 06:44:45 UTC
There is a separate bug for the followup: Bug 7855.
Comment 18 Juan Romay Sieira 2012-04-05 06:46:43 UTC
No this is not obsolete. Magnus and me uploaded two patches that complemented this one, bug 7855 and bug 7850.
On this patch, 7263, in a fresh install the length of the fields 000, 008, 100 ... is 9999, not the correct length. With patches 7855 and 7850 we fixed this sutuation.
Comment 19 Katrin Fischer 2012-04-05 06:49:04 UTC
Ah, I thought the patch had been pushed already - do you want to reset the patch here? I am adding the other bugs as dependending on this.
Comment 20 Juan Romay Sieira 2012-04-05 07:00:59 UTC
yeah, kf, that's ok.
Comment 21 Magnus Enger 2012-04-05 14:10:19 UTC
As far as I can see this was originally signed off - setting it back to that. 

And I sure hope this and bug 7855 make it into 3.8, because bug 7850 relies on these and it is already pushed to master...
Comment 22 Paul Poulain 2012-04-06 13:57:43 UTC
(In reply to comment #21)
> As far as I can see this was originally signed off - setting it back to
> that. 
> 
> And I sure hope this and bug 7855 make it into 3.8, because bug 7850 relies
> on these and it is already pushed to master...

Magnus, I don't understand why this has been switched to signed-off : the patch attached has been pushed (020c095 Bug 7263 - Determine maximum length of some fields or subfields when cataloguing a biblio or an item)

The follow-up 7855 is still to be signed-off and pushed though
Comment 23 Magnus Enger 2012-04-06 14:05:29 UTC
Yes sorry - I got confused!
Comment 24 Magnus Enger 2012-04-06 14:06:00 UTC
Setting it back to "Pushed to master" now!
Comment 25 Jared Camins-Esakov 2012-12-31 00:54:39 UTC
There have been no further reports of problems so I am marking this bug resolved.