Skip to content

Update for ActiveRecord 4 API#5

Merged
nuclearsandwich merged 7 commits intotechnoweenie:masterfrom
nuclearsandwich:where-4.0-art-thou-iteratio
Mar 30, 2016
Merged

Update for ActiveRecord 4 API#5
nuclearsandwich merged 7 commits intotechnoweenie:masterfrom
nuclearsandwich:where-4.0-art-thou-iteratio

Conversation

@nuclearsandwich
Copy link
Collaborator

This removes the use of ActiveRecord 3.x all with options and uses the specific query building methods followed by ActiveRecord::Relation.to_a instead.

As far as I can tell, the tests do not test use fo a select or joins option so that stuff could be wrong.

  • Restrict dependencies to ActiveRecord >= 4.0?
  • Consider whether there are refactoring possibilities to avoid wrapping up options into a hash only to unwrap them again buthonestlyitssundayandIimnotsuperfussed
  • Add tests for :select and :joins options?

cc #4

This removes the use of ActiveRecord 3.x `all` with options and uses
the specific query building methods followed by
`ActiveRecord::Relation.to_a` instead.

As far as I can tell, the tests do not test use fo a select or joins
option so that stuff could be wrong.
It turns out, although this was documented there were no tests for it
and it was never set by the initializer nor did it have any mutators.
So out it goes.
Select probably was working pre-nuclearsandwich but inconsistencies in
Ruby made it hard to see that. Restored that and wrote a very basic test
for it.
This doesn't _really_ test the join option but off the top of my head
I'm not sure what the best way to do so is. If there's a method that
stops ActiveRecord from fetching association info from the database that
would probably work but I don't know it. This at least test that
specifying a join doesn't break anything else.
@nuclearsandwich
Copy link
Collaborator Author

/cc @mislav I dunno if you actually needed this anywhere but I'd love to get another pair of eyes reviewing these changes. also @lachlanhardy @lynnwallenstein for 👀 If there's nothing glaring here I'm probably going merge this and send another PR cutting ✂️ this as 2.0.

test/helper.rb Outdated
end
end

puts AssociatedModel.table_name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose does puts serve here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose does puts serve here?

None now that I've sussed model creation out 😁

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right then 🍤

@nuclearsandwich nuclearsandwich merged commit facba7e into technoweenie:master Mar 30, 2016
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

Successfully merging this pull request may close these issues.

2 participants