Skip to content

Conversation

@sgrif
Copy link
Contributor

@sgrif sgrif commented Jun 22, 2014

reload is meant to put a record in the same state it would be if you
were to do Post.find(post.id). This means we should fully replace the
attributes hash.

`reload` is meant to put a record in the same state it would be if you
were to do `Post.find(post.id)`. This means we should fully replace the
attributes hash.
senny added a commit that referenced this pull request Jun 22, 2014
`reload` should fully reload attributes
@senny senny merged commit 7035a20 into rails:master Jun 22, 2014
@sgrif sgrif deleted the sg-reload-attributes branch June 22, 2014 20:54
senny added a commit that referenced this pull request Jun 22, 2014
@senny
Copy link
Member

senny commented Jun 22, 2014

This introduces a slight backwards incompatibility. You were able to access attributes from a custom select after a reload. Those attributes won't change after the reload so I look at this as a bug fix.

It would be nice to issue a deprecation in these cases but I don't think we can easily detect wether someone is accessing these attributes.

/cc @rafaelfranca

@rafaelfranca
Copy link
Member

hmm, not sure about this change. I'd expect the select attributes to be there after the reload.

@senny
Copy link
Member

senny commented Jun 26, 2014

but you would also expect, that they were reloaded, no?

@rafaelfranca
Copy link
Member

Ah, yes. If they are not being reloaded so it make sense to remove this behavior.

@senny
Copy link
Member

senny commented Jun 26, 2014

@rafaelfranca exactly. Removing them is not perfect but it's better than keeping them around without updating them. This could lead to bad bugs.

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.

3 participants