Issue with new users in RT CLI

Hi,

For once i’m (hopefully) posting something useful and not just moans
about slow databases :wink:

I’ve started implementing the RT CLI tool in my Java API where i was
writing direct to the DB before, it seems to work oK although the
examples are wrong (there was a patch on a previous mail to fix them),
but I had issues with creating new users. The code seems to create a
user called “New user” then modify it to fit ur parameters, the issue
(as the code points out) is if this user already exists. I suspect the
comment might refer to the case that the user creates a ‘New user’ user
rather than the code breaking but anyway… if you botch the RT CLI
parameters in some way, or something breaks it leaves New user around,
and then cant create a new one any more.

Its inevitable that such a breakage will occur, users entering duplicate
email addresses to be added to RT or some such thing, so I decided to
assume ‘New user’ was an internal-only thing and patched RT to load this
user if it cant create a new one. Theres probably better ways to code
this (i.e check the user exists rather than create and fail - although
this ordering will be minutely faster when the system is working
normally - i.e. no New user user). I’ve no experience with the perl RT
api and the bits i used were near by in the code :wink:

It would also be nice if the ticket id came back from a new ticket so I
didn’t have to use “identifyable subjects” to retrieve the ID (a 20 odd
digit random number :P)

Here is my patch, i apologise for any bad coding, or even a broken
generation of the diff (i used ‘diff -p
share/html/REST/1.0/Forms/user/default.orig
share/html/REST/1.0/Forms/user/default’ but have no clue what i’m doing)

  • this will probably be the first time i’ve submitted a patch for anyone
    elses code so go easy on it :wink: let me know what i should have done
    better etc etc…

Thanks,
Iain

*** share/html/REST/1.0/Forms/user/default.orig 2003-10-13
11:06:23.000000000 +0100
— share/html/REST/1.0/Forms/user/default 2003-10-13
13:26:04.000000000 +0100
*************** if ($id ne ‘new’) {
*** 40,46 ****
— 40,55 ----
}
else {
# XXX: Can this fail? What if the user already exists?

  • # IJP 13/10/2003: Yes it can, if you botch RT cli params it can 
    

get left as ‘New user’ preventing all future new users…

  • # solution - pick up the existing one if we can - ASSUMPTION: User 
    

does not have user called ‘New user’ (already an assumption however)
$user->Create(Name => ‘New user’);

  • if (!$user->Id) {
    
  •     $user->Load("New user");
    
  •   if (!$user->Id) {
    
  •           # IJP : Cant make New user, cant load New user, dont 
    

know if this can happen, but program defensively

  •           return [ "# The New User object already existed, but we 
    

failed to load it.", [], {}, 1 ];

  •   }
    
  • }
    

    }

    my ($c, $o, $k, $e) = ("", [], {}, 0);

Iain Price wrote:

Hi,

For once i’m (hopefully) posting something useful and not just moans
about slow databases :wink:

I’ve started implementing the RT CLI tool in my Java API where i was
writing direct to the DB before, it seems to work oK although the
examples are wrong (there was a patch on a previous mail to fix them),
but I had issues with creating new users. The code seems to create a
user called “New user” then modify it to fit ur parameters, the issue
(as the code points out) is if this user already exists. I suspect
the comment might refer to the case that the user creates a ‘New user’
user rather than the code breaking but anyway… if you botch the RT
CLI parameters in some way, or something breaks it leaves New user
around, and then cant create a new one any more.

Always a bad sign when you reply to yourself :stuck_out_tongue:

Even if this patch isn’t used, the default code will proceed to apply ur
modifications to ‘id=NULL’ (according to mysql query trace file) and the
RT cli returns with ‘user creation successful’, even though it got
botched rather badly - rather than take my approach assuming New user is
safe, you could code a error to return here instead (as i initially did
to debug) - anything would be better than pretending it all went fine :slight_smile:
perhaps deeper in the API something should be modified to check if we’re
trying to update ID=NULL and fail, as such a statement surely indicates
a bug higher up in the code.

Iain

At 2003-10-13 13:39:16 +0100, iain.price@post.serco.com wrote:

It would also be nice if the ticket id came back from a new ticket

(Somebody posted a patch for this earlier.)

Here is my patch

I think it might be better to avoid creating a special object altogether
(for reasons discussed earlier, such as bogus email being sent, and more
than one transaction being performed).

Something like the appended should work in theory (I picked Queue rather
than the more complex User/Tickets for this experiment). Unfortunately,
I get “RT::Queue::Validate unimplemented” when I try to run it.

Earlier, the logic was:

if ('new') { Create new object.    }
else       { Load existing object. }

# At this point, we have a valid object.

if (changes) { Make changes to object.          }
else         { Return information about object. }

Now, it is:

if (!new) { Load existing object. }

if (changes) {
    if (new) { Create object all at once. }
    else     { Apply changes one by one.  }
}
else {
    if (new) { Return a suitable default form.  }
    else     { Return information about object. }
}

(Of course, the code below could be cleaned up further. It’s still just
a proof of concept.)

– ams

%# REST/1.0/Forms/queue/default
%#
<%ARGS>
$id
$format => ‘s’
$changes => {}
</%ARGS>
<%perl>
my @data;
my $queue = new RT::Queue $session{CurrentUser};
my @fields = qw(Name Description CorrespondAddress CommentAddress
InitialPriority FinalPriority DefaultDueIn Disabled);
my %fields = map { lc $_ => $_ } @fields;

my ($c, $o, $k, $e) = (“”, , {}, 0);

if ($id ne ‘new’) {
$queue->Load($id);
unless ($queue->Id) {
return [ “# Queue $id does not exist.”, , {}, 1 ];
}
}

my %values;
foreach (%$changes) {
if (exists $fields{lc $}) {
$values{$fields{lc $
}} = $changes->{$};
}
else {
$values{$
} = $changes->{$_};
}
}

if (%values == 0) {
my @data;

if ($id eq 'new') {
    return [
        "# New queue.",
        [ "id", @fields ],
        {
            id => 'queue/new',
            Name => '<queue name>',
            Description => '',
            CommentAddress => '',
            CorrespondAddress => '',
            InitialPriority => '',
            FinalPriority => '',
            DefaultDueIn => '',
            Disabled => 0
        },
        0
    ];
}

push @data, [ id => "queue/".$queue->Id ];
foreach my $key (@fields) {
    push @data, [ $key => $queue->$key ];
}

my %k = map {@$_} @data;
$o = [ map {$_->[0]} @data ];
$k = \%k;

}
else {
if ($id eq ‘new’) {
$queue->Create(%values);
if ($queue->Id) {
$id = $queue->Id;
$c = “# queue/$id created.”;
}
else {
$c = “# Couldn’t create new queue.”;
}
}
else {
my (@comments, $n, $s);

    foreach my $key (keys %values) {
        my $val = $values{$key};

        if (lc $key eq 'name' && $val eq '<queue name>') {
            $n = 0;
            $s = "Please set the Queue name.";
            goto SET;
        }

        if (exists $fields{lc $key}) {
            my $set = "Set$key";
            next if $val eq $queue->$key();
            ($n, $s) = $queue->$set($val);
        }
        elsif ($key ne 'id') {
            $n = 0;
            $s = "Unknown field: $key";
        }

    SET:
        if ($n == 0) {
            $e = 1;
            push @comments, "# $key: $s";
            unless (@$o) {
                @$o = ("id", @fields);
                %$k = %values;
            }
        }
    }

    push(@comments, "# Queue $id updated.") unless @comments;
    $c = join("\n", @comments) if @comments;
}

}

return [ $c, $o, $k, $e ];
</%perl>

I took the plunge and munged Forms/ticket/default to fit into the scheme
described in my earlier post. I changed the logic slightly, though.

if (!new) { Load existing object. }
else {
    if (!changes) { Return default new form. }
    else { Create a new object. }
}

if (!changes) { Return object data. }
else { Apply changes. }

The two visible changes are:

  1. A temporary ticket is no longer created.

  2. You can specify default text for the ticket when you’re creating it,
    by filling in the Text field in the form. For example:

    id: ticket/new
    Queue: General
    Priority: 666
    Text:
    My refrigerator is trying to take over the world.
    I don’t know how to stop it, short of pulling the plug,
    but every time I try to do that, it charges at me.

     Help!
    
     -- ams
    

(A suitable MIMEObj will be created and added to the ticket in the same
transaction that’s used to create it.)

Everything else should work as before.

I would appreciate it if somebody could try this out and let me know how
it works for them. (Just replace $RT/html/REST/1.0/Forms/ticket/default
with this and run rt show/edit etc as usual.)

– ams

default (7.79 KB)