PATCH: autohandler WebExt* attr handling fixes

Hello!

I saw a fixme in the code that seemed approachable, so I did. I have
zero way to test this as I have no easy way to set up all the stuff
to try single-signon. Still, seems like it should work and be a touch
more efficient as it walks the list of attrs received from
WebExternalAutoInfo() rather than the list of all possible attrs.
Also, I left “PagerEmailAddress” in even though it’s not represented
anywhere else in the code … just in case. It could be dropped just
as easily.

This patch is dependent on my earlier indentation patch
(Carbon60: Cloud Consulting - Services and Solutions).

Cheers!

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

autohandler.attr_handling.patch (2.73 KB)

bzzt Illegal use of internals. 200 line penalty. (Also, this code
feels less readable to me. But it might be a lot better with only a very
little bit of feedback

  •            # Fetch the core attrs, dropping any we want to ignore
    
  •            my @core_attrs = grep {!/^${ignore_regex}$/}
    
  •              keys(%{$UserObj->_CoreAccessible});On Thu, Mar 30, 2006 at 02:29:39PM -0800, Jim Meyer wrote:
    

Hello!

I saw a fixme in the code that seemed approachable, so I did. I have
zero way to test this as I have no easy way to set up all the stuff
to try single-signon. Still, seems like it should work and be a touch
more efficient as it walks the list of attrs received from
WebExternalAutoInfo() rather than the list of all possible attrs.
Also, I left “PagerEmailAddress” in even though it’s not represented
anywhere else in the code … just in case. It could be dropped just
as easily.

This patch is dependent on my earlier indentation patch
(Carbon60: Managed Cloud Services).

Cheers!

–j

Jim Meyer, Geek at Large purp@acm.org


List info: The rt-devel Archives

Best Practical is hiring! Come hack Perl for us: Careers — Best Practical Solutions

Hello!On 3/30/06, Jesse Vincent jesse@bestpractical.com wrote:

bzzt Illegal use of internals. 200 line penalty. (Also, this code
feels less readable to me. But it might be a lot better with only a very
little bit of feedback

  •            # Fetch the core attrs, dropping any we want to ignore
    
  •            my @core_attrs = grep {!/^${ignore_regex}$/}
    
  •              keys(%{$UserObj->_CoreAccessible});
    

I was hoping that’d be acceptable in something as deep and mysterious
as the autohandler, which feels like an internal itself.

Would you accept a patch to RT::Record which adds
AccessibleAttributes(), a function which consolidates the
attribute-finding logic in _BuildTableAttributes() and isn’t an
internal? I’ll throw in refactoring _BuildTableAttributes() for free
(and reduce it to one “foreach my $attr …”, even =) and rebase that
autohandler work on $UserObj->AccessibleAttributes() instead.

As to readability, I’m wide open to feedback. =]

Cheers!

–j

p.s. the previous patch also had a typo, calling for “$valid_attr”
instead of “$valid_attrs”. Sorry. =
Jim Meyer, Geek at Large purp@acm.org

Would you accept a patch to RT::Record which adds
AccessibleAttributes(), a function which consolidates the
attribute-finding logic in _BuildTableAttributes() and isn’t an
internal? I’ll throw in refactoring _BuildTableAttributes() for free
(and reduce it to one “foreach my $attr …”, even =) and rebase that
autohandler work on $UserObj->AccessibleAttributes() instead.

->ReadableAttributes and WritableAttributes are in there somewhere
already, iirc

Hello!

->ReadableAttributes and WritableAttributes are in there somewhere
already, iirc

Hmmm. A recursive grep didn’t find 'em anywhere.

Hints?

–j

grep -rl ReadableAtt lib share/html
grep -r Attrib lib | grep sub | less
lib/RT/Ticket_Overlay.pm:sub _Parse822HeadersForAttributes {
lib/RT/SearchBuilder.pm:# {{{ sub LimitAttribute
lib/RT/SearchBuilder.pm:sub LimitAttribute {
lib/RT/Record.pm:sub Attributes {
lib/RT/Record.pm:sub AddAttribute {
lib/RT/Record.pm:sub SetAttribute {
lib/RT/Record.pm:sub DeleteAttribute {
lib/RT/Record.pm:sub FirstAttribute {
lib/RT/Record.pm:sub _BuildTableAttributes {
Jim Meyer, Geek at Large purp@acm.org