Bug 8018 - New subfields have a default max length of zero
Summary: New subfields have a default max length of zero
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P3 major (vote)
Assignee: Pablo AB
QA Contact:
URL:
Keywords:
Depends on: 7263 7855
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-27 21:26 UTC by Jared Camins-Esakov
Modified: 2015-06-04 23:24 UTC (History)
4 users (show)

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


Attachments
Set new subfields maxlenght by default to 9999. (1.27 KB, patch)
2013-12-12 13:35 UTC, Pablo AB
Details | Diff | Splinter Review
Bug 8018: maxlenght should default to 9999 for subfields (1.36 KB, patch)
2013-12-12 15:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 8018: (followup) make the default more explicit (3.26 KB, patch)
2013-12-12 15:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 8018: (followup) make the default more explicit (3.28 KB, patch)
2013-12-12 15:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 8018: maxlenght should default to 9999 for subfields (1.42 KB, patch)
2013-12-22 21:22 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8018: (followup) make the default more explicit (3.36 KB, patch)
2013-12-22 21:22 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 8018: maxlenght should default to 9999 for subfields (1.49 KB, patch)
2013-12-23 18:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 8018: (followup) make the default more explicit (3.80 KB, patch)
2013-12-23 18:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 8018: Follow-up - fixing some wrong capitalization (2.25 KB, patch)
2013-12-23 18:56 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-04-27 21:26:52 UTC
If you add a new subfield to a framework, the maximum length of the field defaults to 0, and the "Max length" box is hidden under "Display more constraints." The result is that if someone simply adds a new subfield, they will be unable to type in it, with no indication of what the problem is.
Comment 1 Pablo AB 2013-12-11 17:44:50 UTC
I think I solved this problem, just adding something like $row_data{maxlength} = "4"; near line 365 on marc_subfields_structure.pl:

@ -360,6 +360,9 @@
             -multiple => 0,
         );
 
+        # This (I think) fix Bug #8018
+        $row_data{maxlength}      = "4";
+
         $row_data{link}   = CGI::escapeHTML( $data->{'link'} );
         $row_data{row}    = $i;
         push( @loop_data, \%row_data );
Comment 2 Pablo AB 2013-12-11 18:42:01 UTC
9999 as default value is much more reasonable.
Comment 3 Pablo AB 2013-12-12 13:35:40 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2013-12-12 15:39:36 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2013-12-12 15:40:47 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2013-12-12 15:44:42 UTC Comment hidden (obsolete)
Comment 7 Chris Cormack 2013-12-22 21:22:39 UTC Comment hidden (obsolete)
Comment 8 Chris Cormack 2013-12-22 21:22:59 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2013-12-23 18:56:04 UTC
Created attachment 23789 [details] [review]
[PASSED QA] Bug 8018: maxlenght should default to 9999 for subfields

The default value for the marc_subfield_structure.maxlenght is 9999
in the DB. Currently the template passes an empty value which is casted to
0 by the CGI.

This simple patch validates the input and converts to the default (9999)
if not defined or 0.

Another approach could be changing the 9999 default and/or treating 0 as
'no-limit'.

PabloAB

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Works by defaulting 0 or "" to 9999.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 10 Katrin Fischer 2013-12-23 18:56:23 UTC
Created attachment 23790 [details] [review]
[PASSED QA] Bug 8018: (followup) make the default more explicit

This patch:
- Makes the new subfield tab show maxlength=9999 as default (instead of empty-then-zero).
- Updates the help to make exlpicit that 0 or empty defaults to 9999.
- Assumes all the subfields created with maxlength=0 inadvertedly are meant to mean
 "no limit" and hence update the database to reflect that.

To test (this patch and Pablo's):
- Edit a MARC framework, edit some field's subfields.
- Use the 'New' tab to create a new subfield (choose an unused letter).
- See in "More constrains" that the "Max length" field is empty. Leave it as-is.
- Save the changes (the new subfield).
- Edit the field again, verify that "Max length" is 0.
- Try tu use the framework and the the field/subfield just created > FAIL
- Apply the patches, upgrade
- Try to use the framework/field/subfield > SUCCESS (0 was converted to 9999)
- Repeat from the beggining, "Max length" should show 9999 on the new subfield tab.
- Leave it empty, it is saved as 9999.

Regards
To+

Edit: small typo

Sponsored-by: Universidad Nacionald de Cordoba
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests in t and xt.
Tested:
- deleting an existing subfield
- adding a new subfield with new default 9999
- editing the new subfield, changing value to 8888
- deleting new subfield
- adding new subfield, using 8888 as length
Comment 11 Katrin Fischer 2013-12-23 18:56:54 UTC
Created attachment 23791 [details] [review]
Bug 8018: Follow-up - fixing some wrong capitalization

Fixes capitalization in 2 places:

- Delete a subfield - Yes/No options
- Edit a subfield - Edit subfield
Comment 12 Galen Charlton 2013-12-27 00:39:41 UTC
Pushed to master.  Thanks Pablo and Tomás!
Comment 13 Fridolin Somers 2014-01-16 16:58:56 UTC
Patches pushed to 3.14.x, will be in 3.14.2.
(creates version 3.14.01.005)
Comment 14 Tomás Cohen Arazi 2014-02-07 14:56:46 UTC
This patch has been pushed to 3.12.x, will be in 3.12.11.

Thanks Pablo and Katrina!