Bug 14662: Add empty entries to pull downs on item form for mandatory subfields

The code assumed that if a subfield is marked as mandatory, there should be no
empty entry in the pull downs.

This assumption is not correct, as it leads to the first entry of the
pull down being preselected if there is no default set. Which means you
will never be alerted of any cataloguing errors and errors will be very
hard to find later on.

Correct behaviour would be to preselect the empty value when there is
no default. This means on saving the item an error message is triggered
and the cataloger is forced to set the value.

To test:
- Adapt your frameworks:
  - Make 942$c non-mandatory
  - In 952 make itemtype, classification source and some other pull downs
    like location or collection mandatory
- Add a new item
  - Verify that the first value of each pull down is preselected,
    there is no way to trigger the 'required' error
- Apply patch
  - Add a new item
    - Verify that classification source is preselected according to the
      DefaultClassificationSource system preference
    - Verify that the itemtype is preselected according to 942$c in
      the bibliographic record
    - Verify all mandatory fields can be set to empty
    - Verify that you can't save before correctly setting them
  - Change the 942$c in the record to empty
  - Add another item
    - Verify the itemtype is now empty
  - Change your frameworks and set a default for itemtype (Ex: BK)
  - Repeat default check with another pull down like collection or location

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We do not want empty values for branches (holdingbranch and homebranch
must be mandatory, see bug 21011)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
index 52f56b6..7f4880f 100755 (executable)
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -179,7 +179,7 @@ sub generate_subfield_form {
                 }
             }
             elsif ( $subfieldlib->{authorised_value} eq "itemtypes" ) {
-                  push @authorised_values, "" unless ( $subfieldlib->{mandatory} );
+                  push @authorised_values, "";
                   my $itemtypes = Koha::ItemTypes->search_with_localization;
                   while ( my $itemtype = $itemtypes->next ) {
                       push @authorised_values, $itemtype->itemtype;
@@ -195,7 +195,7 @@ sub generate_subfield_form {
                   #---- class_sources
             }
             elsif ( $subfieldlib->{authorised_value} eq "cn_source" ) {
-                  push @authorised_values, "" unless ( $subfieldlib->{mandatory} );
+                  push @authorised_values, "";
                     
                   my $class_sources = GetClassSources();
                   my $default_source = C4::Context->preference("DefaultClassificationSource");
@@ -212,7 +212,7 @@ sub generate_subfield_form {
                   #---- "true" authorised value
             }
             else {
-                  push @authorised_values, qq{} unless ( $subfieldlib->{mandatory} );
+                  push @authorised_values, qq{};
                   my $av = GetAuthorisedValues( $subfieldlib->{authorised_value} );
                   for my $r ( @$av ) {
                       push @authorised_values, $r->{authorised_value};