Rails4 Model.find() fix #384

Merged
merged 3 commits into from Apr 17, 2013

Conversation

Projects
None yet
7 participants
@behrendtio

This pull request fixes a few deprecation warnings during the tests plus re-enables Model.find() behaviour for rails4:

irb(main):003:0> Job.find 'ad'
  Job Load (0.4ms)  SELECT `jobs`.* FROM `jobs` WHERE `jobs`.`slug` = 'ad' ORDER BY `jobs`.`id` ASC LIMIT 1
# => <Job id: 16 ... >

irb(main):004:0> Job.find 'not-there'
  Job Load (0.4ms)  SELECT `jobs`.* FROM `jobs` WHERE `jobs`.`slug` = 'not-there' ORDER BY `jobs`.`id` ASC LIMIT 1
  Job Load (0.3ms)  SELECT `jobs`.* FROM `jobs` WHERE `jobs`.`id` = 0 LIMIT 1
ActiveRecord::RecordNotFound: Couldn't find Job with id=not-there
@behrendtio

This comment has been minimized.

Show comment Hide comment
@behrendtio

behrendtio Mar 30, 2013

First attempt to get #357 working.

Note: This doesn't fix the whole test suite, there a still two things open:

  1. Some tests fail with Stack level too deep
  2. History and translation related tests fail

First attempt to get #357 working.

Note: This doesn't fix the whole test suite, there a still two things open:

  1. Some tests fail with Stack level too deep
  2. History and translation related tests fail
@josephers

This comment has been minimized.

Show comment Hide comment
@josephers

josephers Apr 2, 2013

Hooray!

Hooray!

@norman

This comment has been minimized.

Show comment Hide comment
@norman

norman Apr 16, 2013

Owner

Thanks very much for the work on this. I'll be happy to merge this once the tests are passing. I'll have some time to work on this later this week, but if anybody else wants to work on it too feel free.

Owner

norman commented Apr 16, 2013

Thanks very much for the work on this. I'll be happy to merge this once the tests are passing. I'll have some time to work on this later this week, but if anybody else wants to work on it too feel free.

@memoxmrdl

This comment has been minimized.

Show comment Hide comment
@memoxmrdl

memoxmrdl Apr 16, 2013

hello!! what is the solution, have problems about find()

hello!! what is the solution, have problems about find()

@norman

This comment has been minimized.

Show comment Hide comment
@norman

norman Apr 16, 2013

Owner

what is the solution

There isn't one yet. That's why this is still open, and not merged.

Owner

norman commented Apr 16, 2013

what is the solution

There isn't one yet. That's why this is still open, and not merged.

@behrendtio

This comment has been minimized.

Show comment Hide comment
@behrendtio

behrendtio Apr 17, 2013

@memoxmrdl what problems do you have exactly? Did you use my commit in your Gemfile or the current rails4 branch of friendly_id?

@memoxmrdl what problems do you have exactly? Did you use my commit in your Gemfile or the current rails4 branch of friendly_id?

@memoxmrdl

This comment has been minimized.

Show comment Hide comment
@memoxmrdl

memoxmrdl Apr 17, 2013

I am installed all yes i use rails 4 of branch of friendly_id, my problem is when use find and finds nothing

I am installed all yes i use rails 4 of branch of friendly_id, my problem is when use find and finds nothing

@behrendtio

This comment has been minimized.

Show comment Hide comment
@behrendtio

behrendtio Apr 17, 2013

As mentioned above my pull request isn't merged yet since it's not fixing all issues, especially stuff that is related to globalize, which I couldn't figure out yet. If you need the find magic specifically, you can use my branch if you want:

gem 'friendly_id', github: 'mbehrendt/friendly_id', branch: :rails4

As mentioned above my pull request isn't merged yet since it's not fixing all issues, especially stuff that is related to globalize, which I couldn't figure out yet. If you need the find magic specifically, you can use my branch if you want:

gem 'friendly_id', github: 'mbehrendt/friendly_id', branch: :rails4
@memoxmrdl

This comment has been minimized.

Show comment Hide comment
@memoxmrdl

memoxmrdl Apr 17, 2013

@mbehrendt OK tanks i notice you if not have problems!

@mbehrendt OK tanks i notice you if not have problems!

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 17, 2013

Collaborator

@memoxmrdl you're trying to use a beta version of Rails with an unreleased version of friendly_id.. things are bound to not work. Having said that, the issue seems unrelated to this pull request so could you please file it as an issue separately? 😄

Collaborator

parndt commented Apr 17, 2013

@memoxmrdl you're trying to use a beta version of Rails with an unreleased version of friendly_id.. things are bound to not work. Having said that, the issue seems unrelated to this pull request so could you please file it as an issue separately? 😄

@behrendtio

This comment has been minimized.

Show comment Hide comment
@behrendtio

behrendtio Apr 17, 2013

Indeed, this was broken in the original rails4 branch as well, just checked it, so not related to this PR. Seems like there's even more to fix for rails 4 :)

Indeed, this was broken in the original rails4 branch as well, just checked it, so not related to this PR. Seems like there's even more to fix for rails 4 :)

@memoxmrdl

This comment has been minimized.

Show comment Hide comment
@memoxmrdl

memoxmrdl Apr 17, 2013

ok sorry 😄

ok sorry 😄

norman added a commit that referenced this pull request Apr 17, 2013

@norman norman merged commit 903f2fa into norman:rails4 Apr 17, 2013

@norman

This comment has been minimized.

Show comment Hide comment
@norman

norman Apr 17, 2013

Owner

Ok, actually I'm just going to merge this into the Rails 4 branch directly and we can go with it from there. I'll wait to merge it into master until all tests are passing.

Owner

norman commented Apr 17, 2013

Ok, actually I'm just going to merge this into the Rails 4 branch directly and we can go with it from there. I'll wait to merge it into master until all tests are passing.

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 17, 2013

Collaborator

👍 I was looking at this earlier this morning and tempted to do the same.

Collaborator

parndt commented Apr 17, 2013

👍 I was looking at this earlier this morning and tempted to do the same.

@norman

This comment has been minimized.

Show comment Hide comment
@norman

norman Apr 17, 2013

Owner

@parndt what would you think about putting friendly_id's globalize3 support into its own gem? It might make it a tad easier to proceed in parallel.

Owner

norman commented Apr 17, 2013

@parndt what would you think about putting friendly_id's globalize3 support into its own gem? It might make it a tad easier to proceed in parallel.

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 17, 2013

Collaborator

Yes, I think so. Shall we call it friendly_id_globalize ? (I think I'm going to rename globalize3 to globalize because the constant 2/3/4/5 will get a bit tired)
Do you want to make the repository or shall I?

Collaborator

parndt commented Apr 17, 2013

Yes, I think so. Shall we call it friendly_id_globalize ? (I think I'm going to rename globalize3 to globalize because the constant 2/3/4/5 will get a bit tired)
Do you want to make the repository or shall I?

@norman

This comment has been minimized.

Show comment Hide comment
@norman

norman Apr 17, 2013

Owner

@parndt since we're going to reorganize things a bit, how about if we make a friendly_id org, move this repo there, and keep the friendly_id_globalize gem there too?

Owner

norman commented Apr 17, 2013

@parndt since we're going to reorganize things a bit, how about if we make a friendly_id org, move this repo there, and keep the friendly_id_globalize gem there too?

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 17, 2013

Collaborator

@norman that sounds fine with me :-)

Collaborator

parndt commented Apr 17, 2013

@norman that sounds fine with me :-)

@akashkamboj

This comment has been minimized.

Show comment Hide comment
@akashkamboj

akashkamboj Apr 18, 2013

@norman find fix isn't working fine with STI.

@norman find fix isn't working fine with STI.

@terracatta

This comment has been minimized.

Show comment Hide comment
@terracatta

terracatta Jul 26, 2013

Are there any plans to complete / re-introduce find override functionality back into master? Or has the project as a whole decided to move away from overriding find?

Are there any plans to complete / re-introduce find override functionality back into master? Or has the project as a whole decided to move away from overriding find?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment