Cli does not remove values from multi-value fields

I brought this up with rt-bugs (ticket #14423) and it was suggested I
bring it up here, too.

Details are in the ticket, but the upshot is the cli does not deal with
multi-value custom fields very well. If you try to remove a value from
the list, unless it is the first one in the list, it is not removed.
Also, spaces are added to list values when a value fails to be removed.

I also noticed that when I try to add a value to a multi-value field,
spaces are added to the front of the added value, much like in the
erroneous delete I describe in the ticket.

My best guess is that there is a bug somewhere in the vsplit and/or
vpush subs, but I am not quite knowledgeable in perl enough yet to say
for sure.

We use the cli extensively in conjunction with custom fields in our
automated processes, so finding a fix for this is rather important.

I would be happy to help fix this in any way I can.

Thanks.
Seth

Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790

I think I have managed to kill this bug. A diff patch for the rt cli is
attached.

There were two parts to the bug:

  1. multi-value fields were not getting parsed properly when attempting
    to delete a value from the list
  2. when adding values back to a multi-value field, additional space
    characters were inserted before the individual values

Part 1 was because of vsplit() splitting the values on a newline char
when they were actually being returned in the REST form as
comma-separated values. My first fix was to just split on commas, but I
then added the newline char back to the regex match. This should
provide flexibility and preserve any legacy code from other users.

Part 2 was in Form::compose() where new values were added back to the
multi-value list string separated by a comma and then a space. By
removing the space character from the append string, extra spaces are no
longer inserted.

This patch works with my own testing. Please try it for yourself and
make sure I haven’t broken any other functionality.

I hope others find this useful and that it is considered for inclusion
in the next release.

SethOn 04/15/2010 12:01 PM, Seth Galitzer wrote:

I brought this up with rt-bugs (ticket #14423) and it was suggested I
bring it up here, too.

Details are in the ticket, but the upshot is the cli does not deal with
multi-value custom fields very well. If you try to remove a value from
the list, unless it is the first one in the list, it is not removed.
Also, spaces are added to list values when a value fails to be removed.

I also noticed that when I try to add a value to a multi-value field,
spaces are added to the front of the added value, much like in the
erroneous delete I describe in the ticket.

My best guess is that there is a bug somewhere in the vsplit and/or
vpush subs, but I am not quite knowledgeable in perl enough yet to say
for sure.

We use the cli extensively in conjunction with custom fields in our
automated processes, so finding a fix for this is rather important.

I would be happy to help fix this in any way I can.

Thanks.
Seth

Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790

rt.diff (748 Bytes)

Hi Seth

thanks for reporting this bug.
I just updated rt for this, with a little bit different patch.
the patches are c8cd34f and ea2f91d

best wishes
sunnavyOn 10-04-16 11:50, Seth Galitzer wrote:

I think I have managed to kill this bug. A diff patch for the rt
cli is attached.

There were two parts to the bug:

  1. multi-value fields were not getting parsed properly when
    attempting to delete a value from the list
  2. when adding values back to a multi-value field, additional space
    characters were inserted before the individual values

Part 1 was because of vsplit() splitting the values on a newline
char when they were actually being returned in the REST form as
comma-separated values. My first fix was to just split on commas,
but I then added the newline char back to the regex match. This
should provide flexibility and preserve any legacy code from other
users.

Part 2 was in Form::compose() where new values were added back to
the multi-value list string separated by a comma and then a space.
By removing the space character from the append string, extra spaces
are no longer inserted.

This patch works with my own testing. Please try it for yourself
and make sure I haven’t broken any other functionality.

I hope others find this useful and that it is considered for
inclusion in the next release.

Seth

On 04/15/2010 12:01 PM, Seth Galitzer wrote:

I brought this up with rt-bugs (ticket #14423) and it was suggested I
bring it up here, too.

Details are in the ticket, but the upshot is the cli does not deal with
multi-value custom fields very well. If you try to remove a value from
the list, unless it is the first one in the list, it is not removed.
Also, spaces are added to list values when a value fails to be removed.

I also noticed that when I try to add a value to a multi-value field,
spaces are added to the front of the added value, much like in the
erroneous delete I describe in the ticket.

My best guess is that there is a bug somewhere in the vsplit and/or
vpush subs, but I am not quite knowledgeable in perl enough yet to say
for sure.

We use the cli extensively in conjunction with custom fields in our
automated processes, so finding a fix for this is rather important.

I would be happy to help fix this in any way I can.

Thanks.
Seth


Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790

— rt-3.8.7/bin/rt 2009-12-11 12:34:44.000000000 -0600
+++ rt-working/bin/rt 2010-04-16 11:35:21.139276000 -0500
@@ -1347,7 +1347,7 @@
$line .= “,\n$sp$v”;
}
else {

  •                    $line = $line ? "$line, $v" : "$key: $v";
    
  •                    $line = $line ? "$line,$v" : "$key: $v";
                   }
               }
    

@@ -1510,7 +1510,7 @@
my ($word, @words);
my @values = ref $val eq ‘ARRAY’ ? @$val : $val;

  • foreach my $line (map {split /\n/} @values) {
  • foreach my $line (map {split /[,\n]/} @values) {
    # XXX: This should become a real parser, ?? la Text::ParseWords.
    $line =~ s/^\s+//;
    $line =~ s/\s+$//;

List info: http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel

from rt’s git repository:


the patches are:

and

best wishes
sunnavyOn 10-04-28 15:07, Seth Galitzer wrote:

Where would I get these patches from? I was just curious about the
subtle differences. Glad I could help.

Thanks.
Seth

On 04/27/2010 04:57 PM, sunnavy wrote:

Hi Seth

thanks for reporting this bug.
I just updated rt for this, with a little bit different patch.
the patches are c8cd34f and ea2f91d

best wishes
sunnavy


Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790

Looks like these patches aren’t included in 3.8.8. Guess they’ll be in
3.8.9 then?

Thanks.
SethOn 04/28/2010 04:31 PM, sunnavy wrote:

from rt’s git repository:
http://github.com/bestpractical/rt/
the patches are:
http://github.com/bestpractical/rt/commit/c8cd34f
and
http://github.com/bestpractical/rt/commit/ea2f91d

best wishes
sunnavy
On 10-04-28 15:07, Seth Galitzer wrote:

Where would I get these patches from? I was just curious about the
subtle differences. Glad I could help.

Thanks.
Seth

On 04/27/2010 04:57 PM, sunnavy wrote:

Hi Seth

thanks for reporting this bug.
I just updated rt for this, with a little bit different patch.
the patches are c8cd34f and ea2f91d

best wishes
sunnavy

Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790

yes, they’ll be in 3.8.9.

best wishes
sunnavyOn 10-05-07 10:52, Seth Galitzer wrote:

Looks like these patches aren’t included in 3.8.8. Guess they’ll be
in 3.8.9 then?

Thanks.
Seth

On 04/28/2010 04:31 PM, sunnavy wrote:

from rt’s git repository:
http://github.com/bestpractical/rt/
the patches are:
http://github.com/bestpractical/rt/commit/c8cd34f
and
http://github.com/bestpractical/rt/commit/ea2f91d

best wishes
sunnavy
On 10-04-28 15:07, Seth Galitzer wrote:

Where would I get these patches from? I was just curious about the
subtle differences. Glad I could help.

Thanks.
Seth

On 04/27/2010 04:57 PM, sunnavy wrote:

Hi Seth

thanks for reporting this bug.
I just updated rt for this, with a little bit different patch.
the patches are c8cd34f and ea2f91d

best wishes
sunnavy


Seth Galitzer
Systems Coordinator
Computing and Information Sciences
Kansas State University
http://www.cis.ksu.edu/~sgsax
sgsax@ksu.edu
785-532-7790


List info: http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel