search

code4lib / ruby-oai

a Ruby library for building OAI-PMH clients and servers
MIT License
62 stars 43 forks source link

model.scoped deprecated and removed in activerecord 4+ #49

Closed visualist closed 5 years ago

visualist commented 9 years ago

Since ActiveRecord 4.2 (Rails4) has gone and removed the scoped class method, I added a conditional return value using the recommended replacement for model.scoped, which is model.all. I made this conditional on version 4 (major # only) since I didn't want to break any legacy code that still depends on older versions.

jrochkind commented 5 years ago

Hi, this is from 2015, and unmerged with no comments.

I think it is still applicable, to getting the ActiveRecordWrapper to work under any rails 4.2+.

Can anyone who may see this tell me if this gem is still maintained? I'm not sure if I should fork it, or if there are maintainers here to review/merge PRs, or if they are open to adding new maintainers (perhaps @visualist if 4 years later he's still working in this area!), or what.

Thanks for any input.

visualist commented 5 years ago

Oh my god this is so old! What a blast from the past. OAI is something I haven't even thought about in the past 3+ years, although I'm still working in museum circles.

As far as maintained - I got the feeling this gem wasn't even maintained back in 2015.

I'm not doing any OAI work, nothing related to it, presently. We do have an API-driven website now, so if it made sense to "rejoin" the open linked data crowd with our collections data, then I guess it could be done through Rails but not in an ActiveRecord-proper way, unless it now consumes APIs and not just databases. I'm a bit out of touch with the state-of-the-art Rails world, having re-architected and built our new site (Walker Art Center) with other technologies. Would be nice to get back into Rails, tho, I miss it.

jrochkind commented 5 years ago

Cool, I expect visualist to unsubscribe, but I'm working on getting this gem into shape and fixing this.

This is clearly a bug, but there is no existing test that exersizes it, and having trouble figuring out how to make that does, working on it.

visualist commented 5 years ago

@jrochkind - I don't mind following, unless it gets crazy busy, which I don't expect.

jrochkind commented 5 years ago

OK, still not sure how to write a test to excersize it. But the comment says "Default to empty set, as we've tried everything else".

In modern Rails, there's an easier and better way to return a Relation for the empty set -- model.none. So that's probably the right solution.

Still wanna try to figure out a test to exersize it, but if I can't, will just commit that anyway.

visualist commented 5 years ago

I could possibly put some mental energy into this later in the week or next week, thinking about how it might be tested. But right now I'm too busy, and this stuff is way too old for me to just remember it.

jrochkind commented 5 years ago

Eh, I'm doing good with it, but thanks @visualist ! I have a couple days, and use ruby and Rails on the regular.

I'm thinking that this behavior may not make sense. If it can't figure out how to retrieve a set, should it really just return an empty set as it's doing presently, or should it raise?

Have to figure out how to test the scenario either way. For now I guess I'll make it do what it was trying to do before (but hasn't worked in many years!).

visualist commented 5 years ago

ok @jrochkind - good luck!