Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing information, ref[#522c0fd] #15074

Merged
merged 1 commit into from
May 14, 2014

Conversation

kuldeepaggarwal
Copy link
Contributor

No description provided.

@@ -357,7 +357,7 @@ def column_for_attribute(name)

# Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
# "2004-12-12" in a date column is cast to a date object, like Date.new(2004, 12, 12)). It raises
# <tt>ActiveModel::MissingAttributeError</tt> if the identified attribute is missing.
# <tt>ActiveModel::MissingAttributeError</tt> if the identified attribute is missing except +id+.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really did't understand what you are trying to say here. Why I would talk about id if it will be always present?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is because rails will treat "id" as an alias for primary key. From read_attribute definition:

            if name == 'id' && self.class.primary_key != name
              read_attribute(self.class.primary_key)
            end

I believe others will have the same question though. So maybe instead we could write:

# <tt>ActiveModel::MissingAttributeError</tt> if the identified attribute is missing.
# Note: +:id+ will default to the record's primary key if not present

Or something similar. It isn't that :id is special cased (and will always be present) but that if not present it will look for primary_key, which could still possibly not exist if there is a problem with migrations or the database.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think it is not about primary key being not present. It always is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do:

# Note: +:id+ is always present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems good 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Please update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update it.

Thanks and Regards
KD
On May 14, 2014 1:05 AM, "Rafael Mendonça França" notifications@github.com
wrote:

In activerecord/lib/active_record/attribute_methods.rb:

@@ -357,7 +357,7 @@ def column_for_attribute(name)

 # Returns the value of the attribute identified by

attr_name after it has been typecast (for example,
# "2004-12-12" in a date column is cast to a date object, like
Date.new(2004, 12, 12)). It raises

  • ActiveModel::MissingAttributeError if the identified

attribute is missing.

  • ActiveModel::MissingAttributeError if the identified

attribute is missing except +id+.

Right. Please update the PR.


Reply to this email directly or view it on GitHub.

@kuldeepaggarwal
Copy link
Contributor Author

Because in the below example:

person = Person.select('id').first
person[:name]            # => ActiveModel::MissingAttributeError: missing attribute: name

but if we will fetch only name then id wont raise any exception

person = Person.select('name').first
person[:id]            # => nil (this will not raise any exception)

@rafaelfranca
Copy link
Member

It is because the "the identified attribute is not missing". So the documentation seems correct to me.

@kuldeepaggarwal
Copy link
Contributor Author

@rafaelfranca updated.

rafaelfranca added a commit that referenced this pull request May 14, 2014
Add missing information, ref[#522c0fd]
@rafaelfranca rafaelfranca merged commit 96da5e7 into rails:master May 14, 2014
@kuldeepaggarwal kuldeepaggarwal deleted the docs-changes branch May 14, 2014 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants