[RFC] link_if_authorized bug/TODO patch


#1

There’s a bug in link_if_authorized that includes the :wrap_in hash as
an
attribute of the A tag, even though it tries to delete it. There’s also
a noted TODO that things like :id will propagate to both the A and the
wrapper
element.

I think this patch should solve that. :wrap_in now takes either the old
string element, or a new hash, consisting of (a) :tag=>“li/div/etc”, and
(b) anything else.

I’m a very novice Ruby programmer, so if someone (hi James) could
eyeball
this and tell me if it does anything obviously stupid, I’ll code up some
tests and submit it in Trac.

Index:
C:/dev/src/eclipse/radio/radiostar/vendor/plugins/user_engine/lib/user_engine.rb


C:/dev/src/eclipse/radio/radiostar/vendor/plugins/user_engine/lib/user_engine.rb
(revision 39)
+++
C:/dev/src/eclipse/radio/radiostar/vendor/plugins/user_engine/lib/user_engine.rb
(working copy)
@@ -156,13 +156,22 @@
end

 (block.nil? || (yield block)) && authorized?(auth_options) {
  •  #result = link_to_with_current_styling(name, options, 
    

html_options, *params)

  •  wrap_in = html_options.delete(:wrap_in)
     result = link_to(name, options, html_options, *params)
    
  •  # TODO: won't this pass other things like html_options[:id], 
    

which is EVIL since two

  •  # things shouldn't share the same ID.
    
  •  wrap_tag = html_options.delete(:wrap_in)
    
  •  result = content_tag(wrap_tag, result, html_options) if wrap_tag 
    

!= nil

  •  if wrap_in != nil
    
  •    if Hash === wrap_in
    
  •      wrap_tag = wrap_in.delete(:tag)
    
  •      wrap_options = wrap_in
    
  •      p "it's a hash"
    
  •    else
    
  •      wrap_tag = wrap_in
    
  •      wrap_options = nil
    
  •      p "wrap_tag: "
    
  •      p wrap_tag
    
  •    end
    
  •    result = content_tag(wrap_tag, result, wrap_options)
    
  •  end
    
    }
    result
    end

Jay L.


#2

On Mon, 13 Feb 2006 21:26:46 -0500, Jay L. wrote:

There’s a bug in link_if_authorized that includes the :wrap_in hash as an
attribute of the A tag, even though it tries to delete it. There’s also
a noted TODO that things like :id will propagate to both the A and the wrapper
element.

I think this patch should solve that.

Wow! My newsreader somehow added mediocre, print-statement-style
debugging
code to my patch before posting it! How odd! I’d better go, uh, report
that newsreader bug. Yeah, that’s it.

Jay


#3

Hi Jay,

Could you submit this patch to the Trac site? Also if you could write
some tests for it that would be even better - I know the User Engine
needs some TDD love at the moment…

Thanks!

  • james

On 2/14/06, Jay L. removed_email_address@domain.invalid wrote:

this and tell me if it does anything obviously stupid, I’ll code up some

  •  #result = link_to_with_current_styling(name, options, html_options, *params)
    
  •      wrap_options = wrap_in
    
    }

  • J *
    ~