Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Support for multiparameter attribute assignment on virtual attribute writers

#2675

Currently, using multiparameter assignment on a non-column or aggregation throws an error. Since the existing implementation used the reflection to determine whether to create a Date or Time instance to send to the writer, and there's no easy way to infer the desired datatype from the method name or arity, I figured that sending a Time instance for virtual attributes is best in all cases (since a time inherently contains a date.)

Please see attached patch.

Reported by Brennan Dunn · May 19th, 2009 @ 01:33 AM

State: wontfix
Milestone: 3.x
Assigned to: Michael Koziarski Michael Koziarski
Importance: none

Activity

  1. Michael Koziarski
    Michael Koziarski
    • Assigned user set to Eloy Duran

    May 19th, 2009 @ 10:36 PM

  2. Ken Collins
    Ken Collins

    This bit us today too. The fallback to time is a real nice way to handle this IMHO.

    June 2nd, 2009 @ 09:38 PM

  3. nagu
    nagu

    This bit me too.

    June 2nd, 2009 @ 10:03 PM

  4. Michael Koziarski
    Michael Koziarski

    Couldn't you use composed_of instead of attr_accessor here? That'll give you the necessary bits to make this work?

    Time seems like a nice format but it's a little magical here?

    June 10th, 2009 @ 04:35 AM

  5. Eloy Duran
    Eloy Duran

    I'm not quite sure why this was assigned to me.

    If it was because of the composed_of replacement that I wrote in the past; Manfred has taken another stab at it: #950 AttributeDecorator, a new take on aggregation.

    June 10th, 2009 @ 09:20 AM

  6. Brennan Dunn
    Brennan Dunn
    • Assigned user changed from Eloy Duran to Michael Koziarski

    Koz,

    The reasoning behind not generally resorting to composed_of was done for the sake of consistency. Being that update_attributes blindly sends to the attributes' writer methods, whether or not said writer methods map to an actual DB column, it made sense that special form helpers - i.e., date_select, should 'just work'. Currently, due to the column reflection that happens in #execute_callstack_for_multiparameter_attributes, only writer methods that concretely map to a column are supported.

    ...I guess I just feel using #composed_of here and mapping the (1i), (2i), etc. parameters is overkill, and all the form helper methods should be able to support model writer methods using #update_attributes.

    Lastly, every Time contains a Date, and #foo_time= and #foo_date= would 1) look silly and 2) tack on a lot more foundational code. Otherwise, I don't see any other means to really infer the desired format.

    June 10th, 2009 @ 01:12 PM

  7. Michael Koziarski
    Michael Koziarski

    I'm just not sold that a default of Time makes sense. The multi
    parameter code handles more than just Dates and Times and you can tell
    it what the type is by specifying composed_of or something similar.

    I'd prefer to fix it 'right' rather than just default to time if we're
    unsure. Perhaps the best way to do this is to make sure that #950 AttributeDecorator, a new take on aggregation. has
    something simple for this case?

    June 26th, 2009 @ 05:52 AM

  8. Jon Leighton
    Jon Leighton

    Would be great to get this fixed. In the meantime, I thought I'd drop in and say that I found a very hack solution:

    class Whatever < ActiveRecord::Base
      ...
      
      attr_accessor :arrival_time
      columns_hash["arrival_time"] = ActiveRecord::ConnectionAdapters::PostgreSQLColumn.new("arrival_time", nil, "time")
    end
    

    Obviously will need some adaptation depending on the database used etc.

    October 2nd, 2009 @ 03:54 PM

  9. felipekk
    felipekk

    Correcting Jon Leighton:

    class Whatever < ActiveRecord::Base
      ...
      
      attr_accessor :arrival_time
      columns_hash["arrival_time"] = ActiveRecord::ConnectionAdapters::Column.new("arrival_time", nil, "time")
    end
    

    The above code fixed it for me. Of course, this is an ugly hack. Hoping for a fix soon.

    October 3rd, 2009 @ 12:52 AM

  10. Steve Graham
  11. brekaa_osama (at yahoo)
    brekaa_osama (at yahoo)

    I tried the above fix added by "felipekk"
    but didn't work

    here is my code

    Mobile = Struct.new(:mobile_country_code, :mobile_pure_number)
    class User
        attr_accessor :mobile_country_code, :mobile_pure_number
        columns_hash["mobile_country_code"] = ActiveRecord::ConnectionAdapters::Column.new("mobile_country_code", nil, "character varying(255)")
        columns_hash["mobile_pure_number"] = ActiveRecord::ConnectionAdapters::Column.new("mobile_pure_number", nil, "character varying(255)")
        composed_of :mobile,
                    :class_name => 'Mobile',
                    :mapping => [%w(mobile_country_code mobile_country_code), %w(mobile_pure_number mobile_pure_number)],
                    :constructor => Proc.new { |mobile_country_code, mobile_pure_number|
                      Mobile.new(mobile_country_code.to_s.gsub(/^\+/, '00'), mobile_pure_number)
                    }
    end
    

    when i try this code in console

    u = User.new(:mobile_pure_number => '123491484', :mobile_country_code => '0020') ; puts u.mobile.mobile_pure_number
    

    i get nil

    any clue ??

    February 1st, 2010 @ 05:06 PM

  12. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  13. Rohit Arondekar
    Rohit Arondekar
    • Importance changed from to

    Any updates here? This is still an issue?

    October 9th, 2010 @ 04:15 AM

  14. Stephan Kaag
    Stephan Kaag

    Any updates here? This is still an issue?

    October 11th, 2010 @ 01:55 PM

  15. Aditya Sanghi
    Aditya Sanghi
    • State changed from new to wontfix
    • Tag changed from 2.x, activerecord to 2.x, activerecord, multiparameter, multiparameter_attributes

    I assume that not everyone is sold on defaulting on Time as a fallback for multiparameter attributes.

    To specify the class, @felipekk 's response above works and seems like a good way to specify the class for your virtual attribute.

    Marking wontfix for now.

    Please see #4346 time_select not interpreting the multi-parameter correctly for a comprehensive discussion on other multiparameter and positional parameters issue.

    October 17th, 2010 @ 07:54 PM