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

Reevaluate dependency upon ActiveFedora #172

Open
jrgriffiniii opened this issue Jul 25, 2019 · 6 comments
Open

Reevaluate dependency upon ActiveFedora #172

jrgriffiniii opened this issue Jul 25, 2019 · 6 comments

Comments

@jrgriffiniii
Copy link
Contributor

This was proposed by @tpendragon, and perhaps calls to the Gem (such as what is found on https://github.com/samvera/hydra-editor/blob/master/lib/hydra_editor/controller_resource.rb#L3) could be removed for a future minor release.

@jrgriffiniii jrgriffiniii changed the title Reevaluate dependencies for ActiveFedora Reevaluate dependency upon ActiveFedora Jul 25, 2019
@jrgriffiniii jrgriffiniii added this to the 5.0.x series milestone Jul 26, 2019
@jrgriffiniii jrgriffiniii self-assigned this Jul 29, 2019
@jrgriffiniii
Copy link
Contributor Author

032f9ea should remove ActiveFedora as a dependency, and 4b10480 hard-codes the three supported ORMs. This is far from an ideal approach, but my sense is that one shouldn't encounter compatibility issues for all three cases handled.

@no-reply
Copy link

What's the contract for a supported ORM?

Instead of hard coding, could we make orm_class configurable?

I suspect 4b10480 will break Hyrax, since both Valkyrie and ActiveFedora are defined.

@jrgriffiniii
Copy link
Contributor Author

I'm definitely in support of making orm_class configurable, but first I'll need to address some outstanding compatibility issues for ActiveRecord Models (if, indeed, these should be supported - I don't know if that's going to be considered necessary for a future release).

I needed to squash a number of commits to get stubbing properly working for 8a965f1 and 0f9b2ad.

Regarding the contract, my suspicion is that it is probably going to require either .reflect_on_association or .schema on the Class (ActiveRecord does not appear to offer either in https://api.rubyonrails.org/classes/ActiveRecord/Reflection/ClassMethods.html, but perhaps this is the wrong section in the documentation).

@tpendragon
Copy link
Contributor

@no-reply Does Hyrax use the features that hydra-editor uses the ORM for? I vaguely remember just using the partial lookup bit, but I might be mixed up.

@tpendragon
Copy link
Contributor

For instance, Hyrax seems to have its own ControllerResource: https://github.com/samvera/hyrax/blob/5a9d1be16ee1a9150646384471992b03aab527a5/lib/hyrax/controller_resource.rb

@jrgriffiniii
Copy link
Contributor Author

Maybe samvera/hydra-works#352 provides some guidance here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Status: Backlog
Development

No branches or pull requests

3 participants