Rather obscure unless you’re digging around in the core API and dealing
with CFs and validation etc. but important. Multi-valued Select field
values were being passed into the ticket Create() sub in two different
ways:
- When there was only one value, as a string
- When multi-valued, as an array ref
This made some things I was doing with CF validation changes really hard
and probably should be standardised anyway (multiple Enter fields always
get passed in as array refs, nomatter how many values) so that if
$cf->SingleValue is false, then the value of a CF passed in to Create()
can be relied upon to be an array ref.
Patch is against 3.6.3 Web.pm
PK
Philip Kime
NOPS Systems Architect
310 401 0407
Web.pm.patch (746 Bytes)
Rather obscure unless you’re digging around in the core API and dealing
with CFs and validation etc. but important. Multi-valued Select field
values were being passed into the ticket Create() sub in two different
ways:
- When there was only one value, as a string
- When multi-valued, as an array ref
This made some things I was doing with CF validation changes really hard
and probably should be standardised anyway (multiple Enter fields always
get passed in as array refs, nomatter how many values) so that if
$cf->SingleValue is false, then the value of a CF passed in to Create()
can be relied upon to be an array ref.
That sounds like it’s going to be an API change that’s backwards
incompatible. Why can’t you just do canonicalization once it’s in sub
Create?
Jesse
I tested this a lot and I don’t think it’s a problem - EnterMultiplValue
fields always return an array ref as value, even if there is one value,
for example. I was going to canonocalise it but it’s neater to
standardise where it’s created …
PKFrom: Jesse Vincent [mailto:jesse@bestpractical.com]
Sent: Wednesday, February 21, 2007 10:09 AM
To: Philip Kime
Cc: rt-devel@lists.bestpractical.com
Subject: Re: [Rt-devel] [PATCH]: Data normalisation for multi-valued CFs
in GUI create
Rather obscure unless you’re digging around in the core API and
dealing with CFs and validation etc. but important. Multi-valued
Select field values were being passed into the ticket Create() sub in
two different
ways:
- When there was only one value, as a string
- When multi-valued, as an array ref
This made some things I was doing with CF validation changes really
hard and probably should be standardised anyway (multiple Enter fields
always get passed in as array refs, nomatter how many values) so that
if $cf->SingleValue is false, then the value of a CF passed in to
Create() can be relied upon to be an array ref.
That sounds like it’s going to be an API change that’s backwards
incompatible. Why can’t you just do canonicalization once it’s in sub
Create?
Jesse
Patch is against 3.6.3 Web.pm
PK
–
Philip Kime
NOPS Systems Architect
310 401 0407
List info:
The rt-devel Archives
I tested this a lot and I don’t think it’s a problem - EnterMultiplValue
fields always return an array ref as value, even if there is one value,
for example. I was going to canonocalise it but it’s neater to
standardise where it’s created …
Don’t forget that RT:
:Create gets called by third party code,
lots and lots of third party code. Generally changing RT’s API in a
backwards-incompatible way is something that I’m not willing to do
during a stable series. (I also tend to be a fan of Postel’s Law, when
it comes to APIs: Be liberal in what you accept.)
-jesse
As an off-the-wall suggestion that might let you have your cake and eat
it to: Contextual::Return
It allows you to specify what gets returned by context. I’m not saying
it will work, but it came to mind when I read the last couple posts.
Andrew Sterling Hanenkamp
Interaction Developer
Boomer Consulting, Inc.
1.785.537.2358 ext. 17
1.888.266.6375 ext. 17
1.785.537.4545 (fax)
610 Humboldt
Manhattan, KS 66502
http://www.boomer.com/about/team/andrew-hanenkamp.html
andrew.hanenkamp@boomer.com
[mailto:rt-devel-bounces@lists.bestpractical.com] On Behalf Of Jesse
VincentSent: Wednesday, February 21, 2007 1:41 PM
To: Philip Kime
Cc: rt-devel@lists.bestpractical.com
Subject: Re: [Rt-devel] [PATCH]: Data normalisation for multi-valued CFs
inGUI create
I tested this a lot and I don’t think it’s a problem -
EnterMultiplValue fields always return an array ref as value, even if
there is one value, for example. I was going to canonocalise it but
it’s neater to standardise where it’s created …
Don’t forget that RT::Create gets called by third party code,
lots and lots of third party code. Generally changing RT’s API in a
backwards-incompatible way is something that I’m not willing to do
during a stable series. (I also tend to be a fan of Postel’s Law, when
it comes to APIs: Be liberal in what you accept.)
-jesse
PK
List info:
The rt-devel Archives
That sounds like it’s going to be an API change that’s backwards
incompatible. Why can’t you just do canonicalization once it’s in sub
Create?
I think there maybe some misunderstanding - this isn’t an API patch -
it’s a patch to the GUI-only CreateTicket() sub in Web.pm, not the API
Ticket::Create() sub. CreateTicket is a wrapper which massages MASONy
things into the right shape for a call to Ticket::Create() later. I’ve
just altered the massaging a little. Ticket::Create() copes fine with
arrays of one value as values for multi-CFs.
PK
That sounds like it’s going to be an API change that’s backwards
incompatible. Why can’t you just do canonicalization once it’s in sub
Create?
I think there maybe some misunderstanding - this isn’t an API patch -
it’s a patch to the GUI-only CreateTicket() sub in Web.pm, not the API
Ticket::Create() sub. CreateTicket is a wrapper which massages MASONy
things into the right shape for a call to Ticket::Create() later. I’ve
just altered the massaging a little. Ticket::Create() copes fine with
arrays of one value as values for multi-CFs.
I did misunderstand. However, given that, what’s the point of the
change? You really don’t want to be doing any sort of error checking or
data manipulation at that level.
I consider it a bugfix really. I want to be able, after calling
->SingleValue on a CF to know that I have a piece of text or an array
ref to deal with. If I have to data type check in addition after this,
because the nice OO call doesn’t encapsulate the data type, it makes the
nice ->SingleValue interface less useful and the code more confusing. It
behaves nicely for EnterMultipleValue fields, just not
SelectMultipleValue for some reason. I’d like to do it at this point
because this is the point where this discrepancy is introduced. I’m not
doing any error checking or manipulation at this point, but I am later
on and that’s when you find out that the datatype for
SelectMultipleValue CFs isn’t consistent and needs checking for data
type again. SelectMultipleValue CFs created via REST for example, don’t
need this extra check as they are are always array refs, nomatter how
many values, so at the very least, there is a discrepancy between the
GUI and REST (and CommandByMail, as it happens).
PKFrom: Jesse Vincent [mailto:jesse@bestpractical.com]
Sent: Wednesday, February 21, 2007 7:53 PM
To: Philip Kime
Cc: rt-devel@lists.bestpractical.com
Subject: Re: [Rt-devel] RE: [PATCH]: Data normalisation for multi-valued
CFs in GUI create
That sounds like it’s going to be an API change that’s backwards
incompatible. Why can’t you just do canonicalization once it’s in
sub Create?
I think there maybe some misunderstanding - this isn’t an API patch -
it’s a patch to the GUI-only CreateTicket() sub in Web.pm, not the API
Ticket::Create() sub. CreateTicket is a wrapper which massages MASONy
things into the right shape for a call to Ticket::Create() later. I’ve
just altered the massaging a little. Ticket::Create() copes fine with
arrays of one value as values for multi-CFs.
I did misunderstand. However, given that, what’s the point of the
change? You really don’t want to be doing any sort of error checking or
data manipulation at that level.
PK
List info:
The rt-devel Archives
I consider it a bugfix really. I want to be able, after calling
->SingleValue on a CF to know that I have a piece of text or an array
ref to deal with. If I have to data type check in addition after this,
because the nice OO call doesn’t encapsulate the data type, it makes the
nice ->SingleValue interface less useful and the code more confusing. It
behaves nicely for EnterMultipleValue fields, just not
SelectMultipleValue for some reason. I’d like to do it at this point
because this is the point where this discrepancy is introduced. I’m not
doing any error checking or manipulation at this point, but I am later
on and that’s when you find out that the datatype for
SelectMultipleValue CFs isn’t consistent and needs checking for data
type again. SelectMultipleValue CFs created via REST for example, don’t
need this extra check as they are are always array refs, nomatter how
many values, so at the very least, there is a discrepancy between the
GUI and REST (and CommandByMail, as it happens).
From your patch, it looks like all you’re doing is changing how
RT::Interface::Web::CreateTicket calls RT::Ticket->Create();
RT::Ticket->Create() is designed to accept any custom field value as
either a single value or an array ref, no? Even for a Single valued
custom field. The value is automatically coerced into an arrayref on
the fly.
Changing how R:I:W:CreateTicket calls Ticket->Create doesn’t seem to
me to win you anything and if you’re writing code in Ticket->Create()
that expects only one or the other, then you’re doing it in the wrong
place.
Jesse
Changing how R:I:W:CreateTicket calls Ticket->Create doesn’t seem to
me to win you
anything and if you’re writing code in Ticket->Create() that expects
only one or the
other, then you’re doing it in the wrong place.
Well, you’re right - I am actually adding code to Ticket->Create() to
reject ticket creation if their mandatory fields aren’t set. As part of
this processing, I’d like to use ->SingleValue to tell if I should be
looking at empty strings or empty arrays. It’s more OO and nice that
way. If I have to do "ref $blah eq ‘ARRAY’, it’s fine but not as neat.
I’m really only thinking of regularising it - the GUI behaves
differently to REST and CommandByMail which put even single values in
arrays so you always know what you’re getting.
PK
Changing how R:I:W:CreateTicket calls Ticket->Create doesn’t seem to
me to win you
anything and if you’re writing code in Ticket->Create() that expects
only one or the
other, then you’re doing it in the wrong place.
Well, you’re right - I am actually adding code to Ticket->Create() to
reject ticket creation if their mandatory fields aren’t set. As part of
this processing, I’d like to use ->SingleValue to tell if I should be
looking at empty strings or empty arrays. It’s more OO and nice that
way. If I have to do "ref $blah eq ‘ARRAY’, it’s fine but not as neat.
We’re already doing that:
from sub Create:
foreach my $arg ( keys %args ) {
next unless ( $arg =~ /^CustomField-(\d+)$/i );
my $cfid = $1;
foreach
my $value ( UNIVERSAL::isa( $args{$arg} => 'ARRAY' ) ? @{ $args{$arg} } : ( $args{$arg} ) )
{
next unless ( length($value) );
# Allow passing in uploaded LargeContent etc by hash reference
$self->_AddCustomFieldValue(
(UNIVERSAL::isa( $value => 'HASH' )
? %$value
: (Value => $value)
),
Field => $cfid,
RecordTransaction => 0,
);
}
}
Which likely means that you want to work on _AddCustomFieldValue…which is getting your values all pulled apart already.