Make code select validation consistent.

The validation of code select field code is now consistent between the element type profile, the field type profile, and the SOAP element type interface. Thanks to Krzysztof Rudnik for the report [#140 state:resolved].
bricoleurs · Mar 23, 2010 · b60d925 · b60d925
1 parent d03780c
commit b60d925
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 21 deletions.
diff --git a/lib/Bric/App/Callback/Profile/FieldType.pm b/lib/Bric/App/Callback/Profile/FieldType.pm
@@ -7,7 +7,7 @@ use constant CLASS_KEY => 'field_type';
 use strict;
 use Bric::App::Event qw(log_event);
 use Bric::App::Authz qw(:all);
-use Bric::App::Util qw(:history);
+use Bric::App::Util qw(:history :elem);
 
 my $type = CLASS_KEY;
 my $disp_name = 'Field';
@@ -70,6 +70,10 @@ sub save : Callback {
             $set_meta_string->($ed, $f, $param);
         }
 
+        for my $f (qw(multiple)) {
+            $set_meta_boolean->($ed, $f, $param);
+        }
+
         # The default value for checkboxes is boolean.
         if ($ed->get_widget_type eq 'checkbox') {
             $set_meta_boolean->($ed, 'default_val', $param);
@@ -78,17 +82,10 @@ sub save : Callback {
         # All other default vals are strings.
         else {
             $set_meta_string->($ed, 'default_val', $param);
+            # Return if it's a code select with broken code.
+            return if $ed->get_widget_type eq 'codeselect' && ! eval_codeselect $param->{vals};
         }
 
-        for my $f (qw(multiple)) {
-            $set_meta_boolean->($ed, $f, $param);
-        }
-
-        # The occurrence specs are string variables
-        #for my $f (qw(min_occurrence max_occurrence)) {
-        #    $set_meta_string->($ed, $f, $param);
-        #}
-
         $self->add_message(qq{$disp_name profile "[_1]" saved.}, $name);
         log_event("$type\_save", $ed);
     }

diff --git a/lib/Bric/App/Callback/Profile/FormBuilder.pm b/lib/Bric/App/Callback/Profile/FormBuilder.pm
@@ -571,10 +571,7 @@ $add_new_attrs = sub {
             $$no_save = 1;
         } else {
             if ($param->{fb_type} eq 'codeselect') {
-                # XXX: change if comp/widgets/profile/displayAttrs.mc changes..
-                my $code = $param->{fb_vals};
-                my $items = eval_codeselect($code);
-                unless (ref $items eq 'HASH' or ref $items eq 'ARRAY') {
+                unless ( eval_codeselect $param->{fb_vals} ) {
                     $$no_save = 1;
                     return;
                 }

diff --git a/lib/Bric/Changes.pod b/lib/Bric/Changes.pod
@@ -180,6 +180,17 @@ C<ENABLE_CATEGORY_BROWSER> F<bricolage.conf> directive is enabled without
 getting the error "Please select a primary category." Broken in 1.11.3. Thanks
 to Vincent Stemen for the report (Bug #143). [David]
 
+=item *
+
+The field type profile now properly validates code select code and lets the
+user know if it's not valid. [David]
+
+=item *
+
+The validation of code select field code is now consistent between the element
+type profile, the field type profile, and the SOAP element type interface.
+Thanks to Krzysztof Rudnik for the report (Bug #140). [David]
+
 =back
 
 =head1 Version 1.11.3 (2010-01-28)

diff --git a/lib/Bric/SOAP/ElementType.pm b/lib/Bric/SOAP/ElementType.pm
@@ -10,6 +10,7 @@ use Bric::Biz::ElementType;
 use Bric::Biz::ATType;
 use Bric::Biz::Site;
 use Bric::App::Session qw(get_user_id);
+use Bric::App::Util    qw(:elem :msg);
 use Bric::App::Authz   qw(chk_authz READ);
 use Bric::App::Event   qw(log_event);
 use Bric::Util::Fault  qw(throw_ap throw_dp);
@@ -644,14 +645,8 @@ sub load_asset {
               unless defined $field->{key_name};
 
             # Verify the code if it's a codeselect
-            # XXX: triplicated now... (cf. comp/widgets/profile/displayAttrs.mc
-            # and lib/Bric/App/Callback/Profile/FormBuilder.pm)
             if ($field->{widget_type} eq 'codeselect') {
-                my $code = $field->{options};
-                my $items = eval "$code";
-                unless (ref $items eq 'ARRAY' and !(@$items % 2)) {
-                    throw_dp "Invalid codeselect code (didn't return an array ref of even size)";
-                }
+                throw_dp get_msg unless eval_codeselect $field->{options};
             }
 
             # get a data object