Remember the stored attributes in a config attribute. #5412

Closed
wants to merge 14 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

tilsammans commented Mar 13, 2012

This allows you to retrieve the list of attributes you've defined.
Usable for e.g. selects in the view, or interators based on the
attributes you wish to store in the serialized column.

It follows the serialize API, with its serialized_attributes hash.

tilsammans added some commits Mar 13, 2012

Remember the stored attributes in a config attribute.
This allows you to retrieve the list of attributes you've defined.
Usable for e.g. selects in the view, or interators based on the
attributes you wish to store in the serialized column.

My thoughts are there a constant with the list of attributes would probably do in this case. Anyway, in case it goes in, I'd have to ask you to squash your commits into one. Thanks!

/cc @rafaelfranca @josevalim

Contributor

tilsammans commented May 2, 2012

Will gladly squash the commits. And I will wait a couple of days for the others to chime in with regards to the chosen solution.

Contributor

josevalim commented May 2, 2012

Why are we using config_attribute instead of class_attribute?

Owner

rafaelfranca commented May 2, 2012

@josevalim good question. We have this code using config_attribute too.

I think this pull request will be useful. 👍

@tilsammans could you try to use class_attibute in your code?

Contributor

josevalim commented May 2, 2012

It seems @jonleighton added it when he added ActiveRecord::Model.
I would like to understand from Jon why AR is now dependent on
config_accessor and if it is documented somewhere.

Member

jonleighton commented May 2, 2012

@josevalim there is some documentation here

The point was to create something similar to class_attribute that can be used whether or not a module is included in a class or a module. I.e. the following wouldn't work:

module Foo
  included do
    class_attribute :foo
  end
end

module ActiveRecord::Model
  include Foo # we're not in a class, so class_attribute isn't defined
end

The reason that it's in AM is because some AM modules use it.

I'm happy to hear idea for other, nicer solutions :)

Contributor

josevalim commented May 2, 2012

@jonleighton If you extend ActiveSupport::Concern in ActiveRecord::Model, the modules should be lazily included. This means class_attribute should work fine. At least this is how it was designed to work. /cc @wycats

Member

jonleighton commented May 2, 2012

Hmm good point. I will check that. IIRC there was some issue with relying on that behaviour but I can't remember the details so it's worth revisiting.

Member

jonleighton commented May 5, 2012

@josevalim I think part of it was that I wanted to make it possible to set config stuff on AR::Model and have it inherited down to classes that include it, and to AR::Base. But it's on my list to revisit this so I'll think about it some more then.

@tilsammans your proposed pull request is ok to be merged in, but it'd need minor changes before. Could you please:

  • Change from config_attribute to class_attribute, and make sure you add a require to it.
  • Add a Changelog entry.
  • Squash the commits into one.

Thanks!

carlosantoniodasilva and others added some commits May 15, 2012

Merge pull request #6327 from nashby/string-interpolation
remove backported string interpolation
Remember the stored attributes in a config attribute.
This allows you to retrieve the list of attributes you've defined.
Usable for e.g. selects in the view, or interators based on the
attributes you wish to store in the serialized column.
Use class_attribute and not config_attribute.
Also require the needed files.
Contributor

tilsammans commented May 15, 2012

ActiveRecord::Store is a Module, not a Class, so class_attribute will not work. What to do?

@tilsammans well, ok for config_attribute for now, and thanks for the changelog entry.

Your pull request is not applying cleanly anymore, can you please rebase from current master, and squash your commits into one, so we can merge? Thanks.

carlosantoniodasilva added a commit that referenced this pull request Jun 19, 2012

Merge pull request #5412 from tilsammans/stored_attributes
Added `stored_attributes` hash which contains the attributes stored using
ActiveRecord::Store. This allows you to retrieve the list of attributes
you've defined.

    class User < ActiveRecord::Base
      store :settings, accessors: [:color, :homepage]
    end

    User.stored_attributes[:settings] # [:color, :homepage]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment