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

Question about "sets" for ActiveRecordWrapper #9

Closed
seandmccarthy opened this issue Jun 7, 2012 · 2 comments
Closed

Question about "sets" for ActiveRecordWrapper #9

seandmccarthy opened this issue Jun 7, 2012 · 2 comments

Comments

@seandmccarthy
Copy link
Contributor

ActiveRecordWrapper is a wrapper for model.

The documentation states that your ActiveRecord model should implement a class method "sets":

# A model class is expected to provide a method Model.sets that
# returns all the sets the model supports.  See the 
# activerecord_provider tests for an example.

However, the find method basically composes a query condition that, if the OAI request contains a "set" parameter, looks like:

AND set = '<set_name>'

So it is assuming your ActiveRecord model has an attribute (and hence a database column) "set".

Is this what we want? This amounts to per-record granularity- you need to have the "set" value set per record that you want returned. Conceptually I think of a model as being a set, and hence all it's record belong.

I think we should remove this. I'm happy to supply a pull request if no one has an argument in favour of keeping it.

@tjdett
Copy link
Contributor

tjdett commented Aug 16, 2012

@seandmccarthy I'm looking at this issue right now, as I have I case where records could conceptually belong to more than one set.

My intention is provide an additional method for defining sets that uses ActiveRecord associations. Something along the lines of:

class Record < ActiveRecord::Base
  has_many :set_memberships, :class_name => 'RecordSetMembership'
  has_many :sets, :through => :set_memberships

  def self.sets
    Set.all
  end

end

class RecordSetMembership < ActiveRecord::Base
  belongs_to :record
  belongs_to :set
end

class Set < ActiveRecord::Base
  has_many :record_memberships, :class_name => 'RecordSetMembership'
  has_many :records, :through => :record_memberships
end

It shouldn't be too hard to check if the results returned by model.sets are ActiveRecord models, use reflection to find the association back to model (eg. set.records) and then use it to perform find.

@tjdett
Copy link
Contributor

tjdett commented Aug 16, 2012

Pull-request #18 splits out this behaviour into two variants:

  • Model-based sets
  • Attribute-based sets

In model-based sets, the sets are ActiveRecord models which have an association back to records. They preserve the example DCSet behaviour.

In attribute-based sets, the set attribute to determine set membership (and the class method simply needs to provide a list of those sets somehow).

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

No branches or pull requests

2 participants