[Rt-commit] rt branch 5.0/add-boolean-select-render-type created. rt-5.0.2-294-gef1182a0cf

BPS Git Server git at git.bestpractical.com
Mon Jul 11 21:55:52 UTC 2022


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/add-boolean-select-render-type has been created
        at  ef1182a0cf0a19c35aad271d8c99f245f567bc95 (commit)

- Log -----------------------------------------------------------------
commit ef1182a0cf0a19c35aad271d8c99f245f567bc95
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Jul 11 15:45:20 2022 -0500

    Add option for special rendering of an unset CF
    
    Previously all unset custom fields were rendered as '(no value)'.
    
    Thic change adds logic to look for a comp named:
    '/Elements/ShowCustomField' . ${CFType} . ${CFRenderType} . 'Unset'
    and use that component if it exists.  This allows a custom field
    type/render type combination to customize its display when unset.
    
    Without this change a Checkbox custom field displays as '(no value0"
    instead of as an unchecked box.

diff --git a/share/html/Elements/ShowCustomFieldSelectCheckboxUnset b/share/html/Elements/ShowCustomFieldSelectCheckboxUnset
new file mode 100644
index 0000000000..d5fe6acc8f
--- /dev/null
+++ b/share/html/Elements/ShowCustomFieldSelectCheckboxUnset
@@ -0,0 +1,51 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2022 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<input type="checkbox" disabled="disabled" class="custom-control"/>
+<%ARGS>
+$CustomFieldObj => undef
+</%ARGS>
diff --git a/share/html/Elements/ShowCustomFields b/share/html/Elements/ShowCustomFields
index 25d403f11f..fc6c7dc8b5 100644
--- a/share/html/Elements/ShowCustomFields
+++ b/share/html/Elements/ShowCustomFields
@@ -83,7 +83,12 @@ if ( $CustomField->LookupType eq 'RT::Queue-RT::Ticket-RT::Transaction' ) {
     <div class="value col-<% $ValueCols %> <% $count ? '' : ' no-value' %>">
       <span class="current-value">
 % unless ( $count ) {
+%   my $comp = 'ShowCustomField' . $CustomField->Type . $CustomField->RenderType . 'Unset';
+%   if ( $m->comp_exists( $comp ) ) {
+%       $m->comp( $comp, CustomFieldObj => $CustomField );
+%   } else {
 <&|/l&>(no value)</&>
+%   }
 % } elsif ( $count == 1 ) {
 %   $print_value->( $CustomField, $Values->First );
 % } else {

commit 0b632c7008bd3abb6b5795a1cef3d3196859362c
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Jul 11 14:49:53 2022 -0500

    Preserve error messages on value delete
    
    The previous behavior was for RT::CustomField->DeleteValue and
    RT::CustomField->DeleteValueForObject to always substitute their own
    generic error message when the delete failed, which prevents a
    more-specific message from being displayed to the user.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 589861121a..17c41dc973 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -713,7 +713,7 @@ sub DeleteValue {
 
     my ($ok, $msg) = $val_to_del->Delete;
     unless ( $ok ) {
-        return (0, $self->loc("Custom field value could not be deleted"));
+        return (0, $msg || $self->loc("Custom field value could not be deleted"));
     }
     return ($ok, $self->loc("Custom field value deleted"));
 }
@@ -2121,7 +2121,7 @@ sub DeleteValueForObject {
 
     my ($ok, $msg) = $oldval->Delete();
     unless ($ok) {
-        return(0, $self->loc("Custom field value could not be deleted"));
+        return(0, $msg || $self->loc("Custom field value could not be deleted"));
     }
     return($oldval->Id, $self->loc("Custom field value deleted"));
 }

commit 7572ac78d075f03efc99bfae78fe075aea5a8546
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Jun 27 15:08:18 2022 -0500

    Add the Checkbox RenderType for SelectSingle CFs
    
    This change adds a new RenderType, "Checkbox", Select CFs that accept
    only a single value.
    
    It is recommended, but not required, that only one value be configured
    for a Checkbox CF.

diff --git a/docs/initialdata.pod b/docs/initialdata.pod
index d2343349b0..949b2db408 100644
--- a/docs/initialdata.pod
+++ b/docs/initialdata.pod
@@ -217,11 +217,23 @@ common C<LookupType>.
 =item C<RenderType>
 
 Only valid when C<Type> is "Select".  Controls how the CF is displayed when
-editing it.  Valid values are: C<Select box>, C<List>, and C<Dropdown>.
+editing it.  Valid values are: C<Select box>, C<List>, C<Dropdown>, and C<Checkbox>.
 
 C<List> is either a list of radio buttons or a list of checkboxes depending on
 C<MaxValues>.
 
+C<Checkbox> is only available on a Single type CF that is not based on any
+other custom field.  The CF will render as a checkbox.
+
+A Checkbox CF must always have at least two values.  The first value will be
+used for the C<Unchecked> state and the second value will be used for the
+C<Checked> state.
+
+On an object, being unset will be treated as C<Unchecked> and will be silently
+updated to the designated C<Unchecked> value.  Having any other value will be
+treated as C<Checked> and will be noisily updated to the designated C<Checked>
+value.
+
 =item C<MaxValues>
 
 Determines whether this CF is a Single or Multiple type.  0 means multiple.  1
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index fbf897a452..589861121a 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -87,6 +87,7 @@ our %FieldTypes = (
             single => [ 'Dropdown',                # loc
                         'Select box',              # loc
                         'List',                    # loc
+                        'Checkbox',                # loc
                       ]
         },
 
@@ -731,6 +732,10 @@ sub CanDeleteValue {
         return (0, $self->loc('Permission Denied'));
     }
 
+    if ($self->RenderType eq 'Checkbox' and $self->Values->Count < 3) {
+        return ( 0, $self->loc('A Checkbox must always have at least two values' ));
+    }
+
     return ( 1, '' );
 }
 
@@ -1364,6 +1369,15 @@ sub SetRenderType {
                                 $self->FriendlyType));
     }
 
+    if ( $type eq 'Checkbox' ) {
+        if ( $self->Values->Count < 2 ) {
+            return (0, $self->loc("Must have at least two Values for Render Type Checkbox"));
+        }
+        elsif ( $self->BasedOn ) {
+            return (0, $self->loc("We can't currently render as a Checkbox when basing categories on another custom field.  Please use another render type."));
+        }
+    }
+
     return $self->_Set( Field => 'RenderType', Value => $type, @_ );
 }
 
@@ -2273,6 +2287,9 @@ sub SetBasedOn {
     if ( $self->RenderType =~ /List/ ) {
         return (0, $self->loc("We can't currently render as a List when basing categories on another custom field.  Please use another render type."));
     }
+    elsif ( $self->RenderType eq 'Checkbox' ) {
+        return (0, $self->loc("We can't currently render as a Checkbox when basing categories on another custom field.  Please use another render type."));
+    }
 
     return $self->_Set( Field => 'BasedOn', Value => $value, @_ )
 }
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 82bd3d04c6..543105eb98 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3430,6 +3430,22 @@ sub _ProcessObjectCustomFieldUpdates {
                     Value => $value,
                 );
 
+                # this isn't a logical change, but having the field with an explicit
+                # "unchecked" value will make searching easier for users,
+                # so we do it silently
+                if (
+                    $cf_type eq 'Select' and
+                    $cf->RenderType eq 'Checkbox' and
+                    not defined $cf_values->First and
+                    lc $value eq lc CachedCustomFieldValues( $cf )->First->Name
+                ) {
+                    my ( $val, $msg ) = $args{'Object'}->AddCustomFieldValue(
+                        Field             => $cf,
+                        Value             => $value,
+                        RecordTransaction => 0,
+                    );
+                }
+
                 my ( $val, $msg ) = $args{'Object'}->AddCustomFieldValue(
                     Field => $cf,
                     Value => $value
diff --git a/share/html/Admin/Elements/EditCustomFieldValues b/share/html/Admin/Elements/EditCustomFieldValues
index 915b471b80..65271881d2 100644
--- a/share/html/Admin/Elements/EditCustomFieldValues
+++ b/share/html/Admin/Elements/EditCustomFieldValues
@@ -52,7 +52,7 @@
 
 % # we need to allow for an extra col-2 if not combobox and categories are enabled
 % # if so, make the description cols -2 smaller to allow for categories
-% my $description_col_size = ( $CustomField->Type ne 'Combobox' && $Categories ? 4 : 6 );
+% my $description_col_size = ( ( $CustomField->Type ne 'Combobox' && $Categories ) || $renderType eq "Checkbox" ? 4 : 6 );
 
 <div class="form-row">
   <div class="label col-auto">
@@ -75,8 +75,14 @@
       <&|/l&>Category</&>
     </div>
 % }
+% elsif ( $renderType eq 'Checkbox' ) {
+    <div class="label categoryheader col-2 text-left">
+      <&|/l&>Checkbox Type</&>
+    </div>
+% }
 </div>
 
+% my $firstValue = 1;
 % while ( my $value = $values->Next ) {
 % my $paramtag = "CustomField-". $CustomField->Id ."-Value-". $value->Id;
 <div class="form-row">
@@ -106,6 +112,12 @@
 %   }
     </select>
   </div>
+% }
+% if ( $renderType eq 'Checkbox' ) {
+  <div class="col-2">
+        <% $firstValue ? loc('Unchecked') : loc('Checked') %>
+%   $firstValue = 0;
+  </div>
 % }
   <div class="col-1">
     <input type="button" class="delete_custom_field_value button btn btn-primary" data-cfv-id="<% $value->id %>" value="<&|/l&>Delete</&>" onclick="delete_custom_field_value(<% $value->id %>)" />
@@ -139,6 +151,9 @@ my $Categories;
 if ($BasedOnObj and $BasedOnObj->Id) {
     $Categories = $BasedOnObj->Values;
 }
+
+my $renderType = $CustomField->RenderType;
+
 </%init>
 <%args>
 $CustomField => undef
diff --git a/share/html/Elements/EditCustomFieldSelect b/share/html/Elements/EditCustomFieldSelect
index acc8e9e5f3..f0c876163c 100644
--- a/share/html/Elements/EditCustomFieldSelect
+++ b/share/html/Elements/EditCustomFieldSelect
@@ -78,6 +78,18 @@
 %   }
 </div>
 </fieldset>
+% } elsif ( $RenderType eq 'Checkbox' ) {
+%   my $CFVs = CachedCustomFieldValues($CustomField);
+%   my $FalseValue = lc $CFVs->First->Name;
+%   my $TrueValue  = lc $CFVs->Next->Name;
+%   # a checkbox is considered unchecked only if it either:
+%   #     1) has no value
+%   #  OR 2) has the "false" value set (with or without any other values)
+%   #        Note that multiple values are only possible with data from prior to the
+%   #             conversion of the CF to the Checkbox RenderType
+%   my $isChecked = (scalar keys %default) && (not exists $default{$FalseValue});
+  <input type="checkbox" class="custom-control" <% $isChecked ? "checked" : "" %> onclick="document.getElementById('<% $name |n%>').value = this.checked ? '<% $TrueValue %>' : '<% $FalseValue %>'" />
+  <input type="hidden" id="<% $name %>" name="<% $name %>" class="custom-control" value="<% $isChecked ? $TrueValue : $FalseValue %>" />
 % } else {
 % if (@category) {
 %# this hidden select is to supply a full list of values,
diff --git a/share/html/Elements/ShowCustomFieldSelect b/share/html/Elements/ShowCustomFieldSelect
new file mode 100644
index 0000000000..e04aa6afb7
--- /dev/null
+++ b/share/html/Elements/ShowCustomFieldSelect
@@ -0,0 +1,60 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2022 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<%INIT>
+my $cf = $Object->CustomFieldObj;
+my $content = $Object->Content;
+</%INIT>
+% if ( $cf->RenderType eq 'Checkbox' )  {
+    <input type="checkbox" disabled="disabled" class="custom-control" <% ( defined $content and lc $content ne lc CachedCustomFieldValues($cf)->First->Name ) ? "checked" : "" %> />
+% }
+% else {
+    <% $content |h %>
+% }
+<%ARGS>
+$Object => undef
+</%ARGS>

commit 035890899c54fc66067a017d97c27055db2def4e
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Jul 11 16:17:56 2022 -0500

    Missing permission checks deleting CustomFieldValues
    
    Previously any API user could delete custom field values with
    RT::CustomFieldValue->Delete even if they would be denied using
    RT::CustomField->DeleteValue.
    
    This change adds a method to RT::CustomField that can be used by other
    classes for the permissions check.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index c0112ae7da..fbf897a452 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -686,8 +686,6 @@ sub AddValue {
 }
 
 
-
-
 =head3 DeleteValue ID
 
 Deletes a value from this custom field by id.
@@ -719,6 +717,24 @@ sub DeleteValue {
     return ($ok, $self->loc("Custom field value deleted"));
 }
 
+=head3 CanDeleteValue OBJECT
+
+Performs checks at the custom field level to make sure that the value can be deleted.
+
+=cut
+
+sub CanDeleteValue {
+    my $self = shift;
+    my $id_or_obj = shift;
+
+    unless ( $self->CurrentUserHasRight('AdminCustomField') || $self->CurrentUserHasRight('AdminCustomFieldValues') ) {
+        return (0, $self->loc('Permission Denied'));
+    }
+
+    return ( 1, '' );
+}
+
+
 =head2 ValidateValue Value
 
 Make sure that the supplied value is valid
diff --git a/lib/RT/CustomFieldValue.pm b/lib/RT/CustomFieldValue.pm
index 2c991f2b1e..55beb9c181 100644
--- a/lib/RT/CustomFieldValue.pm
+++ b/lib/RT/CustomFieldValue.pm
@@ -102,7 +102,11 @@ sub ValidateName {
 
 sub Delete {
     my $self = shift;
-    my ( $ret, $msg ) = $self->SUPER::Delete;
+    my ( $ret, $msg ) = $self->CustomFieldObj->CanDeleteValue( $self );
+    if ( !$ret ) {
+        return ( 0, $msg || $self->loc('Unable to delete custom field value.  Disallowed by custom field' ));
+    }
+    ( $ret, $msg ) = $self->SUPER::Delete;
     $self->CustomFieldObj->CleanupDefaultValues;
     return ( $ret, $msg );
 }

commit 6f08909a85aa697d1fa98227801a1f6af005584b
Author: Brian Conry <bconry at bestpractical.com>
Date:   Mon Jul 11 16:40:14 2022 -0500

    Add test for CFV->Delete without permissions
    
    Add a test to verify that a user needs rights to delete a custom field
    value using RT::CustomFieldValue->Delete.

diff --git a/t/api/cf_rights.t b/t/api/cf_rights.t
index e8a75c1656..a76bd0d1c3 100644
--- a/t/api/cf_rights.t
+++ b/t/api/cf_rights.t
@@ -2,7 +2,7 @@ use warnings;
 use strict;
 
 use RT;
-use RT::Test tests => 12;
+use RT::Test tests => 13;
 
 my $q = RT::Queue->new($RT::SystemUser);
 my ($id,$msg) =$q->Create(Name => "CF-Rights-".$$);
@@ -41,6 +41,9 @@ ok ($id,$msg);
 ($id,$msg) = $u->PrincipalObj->RevokeRight( Object => $cf, Right => 'AdminCustomFieldValues' );
 ok ($id,$msg);
 
+($id,$msg) = $cfv->Delete;
+ok (!$id,$msg);
+
 ($id,$msg) = $u->PrincipalObj->GrantRight( Object => $cf, Right => 'AdminCustomField' );
 ok ($id,$msg);
 

-----------------------------------------------------------------------


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list