Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Joel Chippindale November 3rd, 2008 @ 09:07 AM

  The attached patch fixes this and makes Action::Mailer.method_missing just catch methods which begin with create and deliver

• You flagged this item as spam.
  

  Joel Chippindale November 3rd, 2008 @ 09:20 AM

  • Title changed from “[PATCH]: Make ActionMailer::Base.method_missing more precise” to “Make ActionMailer::Base.method_missing more precise”

• You flagged this item as spam.
  

  Tom Lea November 3rd, 2008 @ 02:04 PM

  Well spotted! I have a couple of enhancements to the patch that could be made.

  1. match = matches_dynamic_method?(method_symbol) looks clearly broken as of 3cf773b. This should be fixed separately, possibly in a separate commit, with another failing testcase (ex1 is a fairly ugly way of doing this).
  2. Commit notes should be referring to Prefix not Suffix ;)
  3. Having two else conditions with calls to super in a row does not scan well. How about using (ex2) at the top? It also gets rid of a layer of nesting, which is nice.

  ex1:

  
    def test_should_still_raise_exception_with_expected_message_when_calling_an_undefined_method
      begin
        RespondToMailer.not_a_method
        fail("Expected a NoMethodError.")
      rescue NoMethodError => e
        assert_match(/undefined method.*not_a_method/, e.message)
      rescue Exception => e
        fail("Expected a NoMethodError, got #{e.class}.")
      end
    end
  

  ex2:

  
  return super unless match = matches_dynamic_method?(method_symbol)
  

• You flagged this item as spam.
  

  Joel Chippindale November 5th, 2008 @ 10:05 AM

  • Title changed from “Make ActionMailer::Base.method_missing more precise” to “Fix for matches_dynamic_method? on ActionMailer”

  Thanks for the suggestions Tom.

  1. As you suggested it seems like a good idea to split these two issues.

  I have created a separate ticket #1330 for the problem with method_missing not raising an exception correctly when an undefined method is called and included a test case for this (based on ex1 above).

  I removed the first call to super in this (because the line of code is never reached) but did not implement ex2 (above) because I think it is clearer to have an if/else statement in this case.

  1. Oops. Fixed

  A new patch is attached with just the fix for matches_dynamic_method in it

  • patch_for_matches_dynamic_method_on_action_mailer.diff 1.8 KB

• You flagged this item as spam.
  

  James Mead November 11th, 2008 @ 01:43 PM

  +1 Joel, Tom - Thanks for fixing my original commit and sorry for introducing the bug. Cheers, James.

• 

  Repository November 11th, 2008 @ 03:47 PM

  • State changed from “new” to “resolved”

  (from [c65075feb6c4ce15582bc08411e6698d782249a7]) Fixed method_missing for ActionMailer so it no longer matches methods where deliver or create are not a suffix [#1318 state:resolved]

  Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...