Forum: Rails Engines [RFC] link_if_authorized bug/TODO patch

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
A63764f318f10379c8b51349b757cf4b?d=identicon&s=25 Jay Levitt (Guest)
on 2006-02-14 03:26
(Received via mailing list)
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 Levitt
A63764f318f10379c8b51349b757cf4b?d=identicon&s=25 Jay Levitt (Guest)
on 2006-02-14 17:43
(Received via mailing list)
On Mon, 13 Feb 2006 21:26:46 -0500, Jay Levitt 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
05d703f649ef1d07e78d7b479fb4c4ac?d=identicon&s=25 James Adam (Guest)
on 2006-02-15 23:08
(Received via mailing list)
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 Levitt <jay+news@jay.fm> 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 *
  ~
This topic is locked and can not be replied to.