[Rt-commit] rt branch, 4.2/confirm-loaded-queue, created. rt-4.2.4-9-ga38b879

[Jim Brandt]

The branch, 4.2/confirm-loaded-queue has been created
        at  a38b879928f021e6c28ee2149a0c9661a6eb07ff (commit)

- Log -----------------------------------------------------------------
commit 2c714ae3e79c4ca7d6f6a17f00c86c064f99ba12
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Mon May 12 16:23:03 2014 -0400

    Confirm loaded queue object before setting context object
    
    For cases where an invalid Queue value is passed as an arg,
    either a name or id of a non-existent queue or even 0, previously
    the emtpy queue object would still be set as the context object
    even though the load failed. This caused the search to add a
    Limit on a LookupType of RT::Queue, which won't find any
    custom fields.
    
    Check the return code to avoid setting the empty queue object and
    to warn so there is some help in the logs to track down a possible
    typo in the queue name/id.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index fdc66a2..520809d 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -417,10 +417,20 @@ sub LoadByName {
     # if we're looking for a queue by name, make it a number
     if ( defined $args{'Queue'} && ($args{'Queue'} =~ /\D/ || !$self->ContextObject) ) {
         my $QueueObj = RT::Queue->new( $self->CurrentUser );
-        $QueueObj->Load( $args{'Queue'} );
-        $args{'Queue'} = $QueueObj->Id;
-        $self->SetContextObject( $QueueObj )
-            unless $self->ContextObject;
+        my ($ret, $msg) = $QueueObj->Load( $args{'Queue'} );
+
+        # Only set the context object if we successfully loaded a queue.
+        # This avoids creating a LookupType of RT::Queue below based on an empty
+        # queue object.
+
+        if ( $ret ){
+            $args{'Queue'} = $QueueObj->Id;
+            $self->SetContextObject( $QueueObj )
+                unless $self->ContextObject;
+        }
+        else {
+            RT::Logger->warning("Unable to load queue with id " . $args{'Queue'});
+        }
     }
     if ( defined $args{'Queue'} ) {
         # Set a LookupType for backcompat, otherwise we'll calculate

commit 1e4bab3e046eec7c4e038a1410e03d7849dcd3e6
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Mon May 12 16:35:46 2014 -0400

    Explicitly exclude 0 from queue lookup
    
    A Queue value of 0 represents a global CF, but will not load a
    queue successfully. Explicitly exclude 0 from the queue load and
    context object setting. Although 2c714ae3 will now prevent this,
    it logs a warning that is inappropriate when using the documented
    API for the method.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 520809d..08f7ad8 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -415,7 +415,10 @@ sub LoadByName {
     }
 
     # if we're looking for a queue by name, make it a number
-    if ( defined $args{'Queue'} && ($args{'Queue'} =~ /\D/ || !$self->ContextObject) ) {
+    # Exclude 0 since it is a valid parameter, but will not load a valid queue
+    if ( defined $args{'Queue'}
+         && ($args{'Queue'} =~ /^\d+$/ ? $args{'Queue'} != 0 : 1)
+         && ($args{'Queue'} =~ /\D/ || !$self->ContextObject) ) {
         my $QueueObj = RT::Queue->new( $self->CurrentUser );
         my ($ret, $msg) = $QueueObj->Load( $args{'Queue'} );
 

commit a38b879928f021e6c28ee2149a0c9661a6eb07ff
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue May 13 16:50:04 2014 -0400

    Tests for LoadByName with queue 0

diff --git a/t/api/customfield.t b/t/api/customfield.t
index 6be50bb..0e635a6 100644
--- a/t/api/customfield.t
+++ b/t/api/customfield.t
@@ -2,7 +2,7 @@
 use strict;
 use warnings;
 use RT;
-use RT::Test nodata => 1, tests => 29;
+use RT::Test nodata => 1, tests => undef;
 use Test::Warn;
 
 
@@ -36,7 +36,6 @@ ok(my ($bad_id, $bad_msg)=  $cf->Create( Name => 'TestingCF-bad',
                                  Type=> 'SelectSingleton'), 'Created a global CustomField with a bogus type');
 is($bad_id , 0, 'Global custom field correctly decided to not create a cf with a bogus type ');
 
-
 }
 
 {
@@ -71,3 +70,12 @@ ok(!$cf->ValidateType('SelectFooMultiple'));
 
 }
 
+{
+
+    my $cf = RT::CustomField->new(RT->SystemUser);
+    $cf->LoadByName(Queue => 0, Name => 'TestingCF');
+    ok($cf->Id, 'Loaded CF ' . $cf->Id);
+    ok(!$cf->ContextObject, 'Context object not set when queue is 0');
+}
+
+done_testing;

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