This project is archived and is in readonly mode.

#4487 ✓stale
ximus

ActiveModel::Lint variable type of #to_key's output

Reported by ximus | April 27th, 2010 @ 03:43 PM

Hi,

The ActiveModel Lint tests do not specify whether a comliant model needs to produce a String or a Fixnum when the #to_key compliance method is called.

However ActionController is intolerant and expects a String as it calls #join on #to_key's output.

Here is the soft spot requiring a String value for key in action_controller/record_identifier.rb line 81:

def record_key_for_dom_id(record)
  record = record.to_model if record.respond_to?(:to_model)
  key = record.to_key
  key ? sanitize_dom_id(key.join('_')) : key
end

Now is ActionController rightfully intolerant and ActiveModel needs to reflect that?
Or should ActionController take non-string values into account?

This issue arose when today, I started playing with the early ActiveModel compliance that is being built into the neo4j.rb library; it produces a Fixnum.

Both using a Fixnum or a String could make sense and if neither is enforced in the Lint test you should expect either to work.

Here is a link to the Lint test.

(This is my second issue filing ever, if you want patches, failing tests or diffs then I'm going to need some pointers because I'm not sure exactly how to produce or present there things)

Comments and changes to this ticket

  • ximus

    ximus April 27th, 2010 @ 03:45 PM

    • Tag changed from activemodel lint actioncontroller to actioncontroller, activemodel, lint
  • ximus

    ximus April 28th, 2010 @ 01:16 PM

    I mentioned that join is a instance method in String, obviously not, I had a long day. It expects an Array?

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:50 PM

    • State changed from “new” to “open”

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:50 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Pages