Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  YoNoSoyTu February 15th, 2009 @ 07:09 PM

  Fails on me with this:

  
    1) Error:
  test_attribute_reset(DirtyTest):
  NoMethodError: undefined method `catchphrase=' for nil:NilClass
      ./test/cases/dirty_test.rb:67:in `test_attribute_reset'
      ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:57:in `__send__'
      ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:57:in `run'
  

  Seems like the fixtures are not loaded by that test case. I suggest using

  
  Pirate.create!(:catchphrase => 'Yar.')
  

  which seems to work OK.

  Otherwise, nice patch, useful to have.

• You flagged this item as spam.
  

  Guillermo Álvarez February 16th, 2009 @ 11:00 AM

  Thanks for the comment.

  I run test with

  rake test TEST=find . -name dirty_test.rb

  and

  rake test

  That seems not to fail for me, so i didn't care.

  But you are right since there is no explicit creation of pirates.

  I will submit a patch along today.

• 

  CancelProfileIsBroken April 22nd, 2009 @ 10:20 PM

  • State changed from “new” to “stale”

  Staling out. Please reopen if you have the fixed patch.

• You flagged this item as spam.
  

  Paul Gillard July 4th, 2009 @ 12:27 PM

  • Tag changed from “activerecord, dirty, reset, test” to “activerecord, dirty, patch, reset, test”

  Updated patch attached. Original implementation by Guillermo is unchanged but test case fixed along the lines of comment by YoNoSoyTu. I've also updated the documentation at the top of dirty.rb to:
  * include a separate section for the reset functionality * change 'uncle bob' to 'Uncle Bob' as I felt difference in capitalisation distracted my reading of the example * altered the name used in the modify in place section from 'uncle bob' to 'Bill' as the record is saved as 'Bill' earlier in the example.

  All tests pass in MySQL, SQLite3 and PostgreSQL. Try as I might I couldn't get the SQLite2 tests to run.

  • dirty_attribute_reset.diff 3.7 KB
• 

  Matt Jones July 16th, 2009 @ 03:55 PM

  • State changed from “stale” to “open”

  Switching back to open as requested.

  One small style thing: my first impression was that the _reset method should possibly have a !, to match other methods that modify objects in place. (chomp!, gsub!, etc)

• You flagged this item as spam.
  

  Paul Gillard July 16th, 2009 @ 06:25 PM

  Very good point on the missing bang. Updated patch. All tests pass in MySQL, SQLite3 and PostgreSQL.

  • dirty_attribute_reset.diff 3.7 KB
• 

  Michael Koziarski July 19th, 2009 @ 08:09 AM

  The _reset name doesn't do it for me, it's backwards:

  
    foo.reset_attribute(:name)
  

  Would see more natural to me

• You flagged this item as spam.
  

  Paul Gillard July 25th, 2009 @ 01:52 PM

  Makes sense to me. I'll work something together. At first glance it looks AttributeMethods will need to expand to include more than just #attribute_method_suffix. Possibly something along the lines of #attribute_method_prefix and/or #attribute_method_affix.

• You flagged this item as spam.
  

  Paul Gillard July 29th, 2009 @ 06:39 PM

  I wish I'd read my above comment once more. I meant your comment made sense to me rather than I thought the attribute_reset! name made sense. reset_attribute! is far more appropriate.

  I've now worked a patch together which implements this functionality as reset_attribute! rather than attribute_reset! It's now possible to create attributes methods with prefixes and/or suffixes using the class methods attribute_method_prefix, attribute_method_suffix and attribute_method_affix. Every method set up through these methods is represented by an instance of a new MethodMatcher class. All these instances are stored in the class variable @@attribute_method_matchers.

  match_attribute_method? is now an instance method rather than a class method. This allows it access to @attributes and in doing so we can know that a positive match represents a valid method. Previously those methods calling match_attribute_method? (method_missing and responds_to?) had to check that the attribute name part of the match was valid. They no longer need to do so which makes them read better.

  The matching of a method name is split between the instance method match_attribute_method? and the class method matching_method_signatures. matching_method_signatures returns all MethodMatcher instances which could possibly match and match_attribute_method? picks out one that actually does. For example, say we have a method suffix of 'default' and another method suffix of 'original_default'. When matching a method name of 'title_original_default' the matching method signatures would be:

  • { :prefix => '', :base => 'title_original', :suffix => '_default' }
  • { :prefix => '', :base => 'title', :suffix => '_original_default' }

  match_attribute_method? would then select the latter as this is the one with a valid attribute name.

  As this is my first contribution I'd welcome any feedback, however small. Can anyone think of a better name than attribute_method_affix for an attribute method with both a prefix and suffix?

  • dirty_attribute_reset.diff 18.3 KB
• 

  Michael Koziarski August 1st, 2009 @ 09:28 AM

  • Assigned user set to “josh”

  Josh is actually working / tinkering with attribute methods right now, so I'm assigning this to him.

  Josh, paul's latest patch here looks a little like what we were discussing n campfire the other day.

• 

  josh August 3rd, 2009 @ 03:56 AM

  • Milestone cleared.

  +1 overall

  @Paul Any chance I could have you rebase the patch against master (please)? Might be cleaner to have the first patch add attribute_method_affix feature, then the second to add reset_attribute to dirty tracking.

• You flagged this item as spam.
  

  Paul Gillard August 4th, 2009 @ 08:08 PM

  I've reworked the changes into two patches against master as requested.

  The one thing I don't like about it is that there are now two ways in which method_missing and respond_to? call attribute query methods.

  1. self.send(method_id, *args, &block)
     
  2. __send__("#{match.prefix}attribute#{match.suffix}", match.base, *args, &block)

  The first only gets called when no methods have yet been added to generated_methods after that the code relies on the second which seems a shame as it's messier. So I've also attached a third 'work in progress' patch which you might be interested in looking at which does a rough job of hinting at one way of tidying it up. If you're interested in me developing it further let me know.

  • attribute_method_affix.diff 17.8 KB
  • dirty_attribute_reset.diff 4.9 KB
  • attribute_method_wip.diff 4.1 KB
• 

  josh August 4th, 2009 @ 10:32 PM

  • State changed from “open” to “resolved”

  Awesome work!

  I like the direction of the wip patch, I was trying to do the same thing myself. However we need to support both ways (which sucks). We need the dynamic dispatch for join tables and other magic association attributes and we don't always want to define those. We only want the column names to stick around.

  If you have any ideas please open a different ticket.

• 

  Ryan Bigg October 9th, 2010 @ 10:04 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”

  [bulk edit]

• 

  Ryan Bigg October 21st, 2010 @ 03:34 AM

  Automatic cleanup of spam.

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 01:36 AM

  omega watch