-
Notifications
You must be signed in to change notification settings - Fork 43
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
Redo "sets" in ActiveRecord::Wrapper #67
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ActiveRecord::Wrapper has some kind of built-in implementation for providing oai-pmh "sets".
But it is non-documented, and very confusing. It seems to be based on assuming that you will organize your records into sets using an actual rdbms fk/pk association (and corresponding ActiveRecord association), and/or with a String "set" column/attribute in your record table.
This seems not flexible enough, and making too many assumptions. The code is also somewhat convoluted, making lots of assumptions about your Models and making guesses. I was unable to figure out how to make tests even exersizing some paths in the code.
See eg https://github.com/code4lib/ruby-oai/blob/master/lib/oai/provider/model/activerecord_wrapper.rb#L91-L118
We should deprecate all the existing "set" stuff. And provide a new set implementation where the sets are defined on the Wrapper, rather than making assumptions about the model. They can probably conveniently make use of ActiveRecord "scopes" (methods that return relations) for the simple cases.
It should not require any additional methods (or db attributes/relationships) be added to your AR models, you should do the configuration in the wrapper class (and/or a custom sub-class of the wrapper class?)
This will probably not be done before 1.0.
See #9, #49, #66.
The text was updated successfully, but these errors were encountered: