BUG/PATCH? RT::CurrentUser::LoadByEmail() should use $self->UserObj

Hello!

Here’s the code:

sub LoadByEmail {
my $self = shift;
my $identifier = shift;

$identifier = 
    RT::User::CanonicalizeEmailAddress(undef, $identifier);
     
$self->LoadByCol("EmailAddress",$identifier);

}

Rather than calling CanonicalizeEmailAddress statically, shouldn’t it
call $self->UserObj->CanonicalizeEmailAddress? Are there cases where a
valid RT::CurrentUser object won’t have a valid RT::User object?

As it stands, it forces any downstream functions to worry about whether
or not $self is defined as well as needlessly invalidating any use of
ACLs.

I offer the attached patch.

Cheers!

–j
Jim Meyer, Geek at Large purp@acm.org

RT::CurrentUser.pm.patch (443 Bytes)

call $self->UserObj->CanonicalizeEmailAddress? Are there cases where a
valid RT::CurrentUser object won’t have a valid RT::User object?

Yes. When you first load it.

I think RT::User->CanonicalizeEmailAddress($identifier); variant is better.On 12/16/05, Jesse Vincent jesse@bestpractical.com wrote:

call $self->UserObj->CanonicalizeEmailAddress? Are there cases where a
valid RT::CurrentUser object won’t have a valid RT::User object?

Yes. When you first load it.


Rt-devel mailing list
Rt-devel@lists.bestpractical.com
The rt-devel Archives

Best regards, Ruslan.

Hello!

call $self->UserObj->CanonicalizeEmailAddress? Are there cases where a
valid RT::CurrentUser object won’t have a valid RT::User object?

Yes. When you first load it.

Right. At that point it’s a valid (though empty) user object, still
suitable for calling methods and arguably better than calling methods
statically as those methods can now know that you are unsure of who’s
doing the calling.On Thu, 2005-12-15 at 14:38, Ruslan Zakirov wrote:

I think RT::User->CanonicalizeEmailAddress($identifier); variant is better.

I’d like to hear more of why you feel so because, at present, I don’t
agree. It means that CanonicalizeEmailAddress (and any function which
might be called by it) must support being called without a valid user.
Alternately, CanonicalizeEmailAddress could choose to create an empty
user object (which is what I’m doing at present), but then we’ve broken
the chain connecting the user who’s acting and the actions being taken.
This also makes this function an exception within the RT::User object.

If I seem to advocate strongly for this change, it’s because the
required change seems very straightforward and makes the RT::User object
wholly consistent in this regard.

Cheers!

–j

p.s. This would also require a similar change to two lines in
RT::ticket::_RecordNote, where it would call
$self->CurrentUser->UserObj->CanonicalizeEmailAddress().
Jim Meyer, Geek at Large purp@acm.org