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

Tests break when upgrading from 0.7.2 to 0.7.4 #228

Closed
jmuheim opened this issue Sep 8, 2016 · 10 comments
Closed

Tests break when upgrading from 0.7.2 to 0.7.4 #228

jmuheim opened this issue Sep 8, 2016 · 10 comments

Comments

@jmuheim
Copy link

jmuheim commented Sep 8, 2016

I have the following model:

class SuccessCriterion < ActiveRecord::Base
  acts_as_tree order: :position, dependent: :restrict_with_error
  acts_as_list scope: [:parent_id]
end

With the following test:

it 'scopes by parent_id' do
  root = create :success_criterion
  child_1 = create :success_criterion, name: 'child 1'
  child_2 = create :success_criterion, name: 'child 2'

  root.children = [child_1, child_2]

  expect(root.position).to be 1
  expect(child_1.position).to be 1
  expect(child_2.position).to be 2
end

end

With acts_as_list v0.7.2, it passes:

josh@Macbuech:~/Documents/Work/Access4All/projects/a4aa2/src$ rspec spec/models/success_criterion_spec.rb:131
Run options: include {:focus=>true, :locations=>{"./spec/models/success_criterion_spec.rb"=>[131]}}

Randomized with seed 54471

SuccessCriterion
  acts as list
    scopes by parent_id

Top 0 slowest examples (0 seconds, 0.0% of total time):

Finished in 0.19168 seconds (files took 5.46 seconds to load)
1 example, 0 failures

Randomized with seed 54471

With v0.7.4, it doesn't:

josh@Macbuech:~/Documents/Work/Access4All/projects/a4aa2/src$ rspec spec/models/success_criterion_spec.rb:131
Run options: include {:focus=>true, :locations=>{"./spec/models/success_criterion_spec.rb"=>[131]}}

Randomized with seed 41970

SuccessCriterion
  acts as list
    scopes by parent_id (FAILED - 1)

Failures:

  1) SuccessCriterion acts as list scopes by parent_id
     Failure/Error: expect(child_1.position).to be 1

       expected #<Fixnum:3> => 1
            got #<Fixnum:5> => 2

       Compared using equal?, which compares object identity,
       but expected and actual are not the same object. Use
       `expect(actual).to eq(expected)` if you don't care about
       object identity in this example.
     # ./spec/models/success_criterion_spec.rb:131:in `block (3 levels) in <top (required)>'

Top 0 slowest examples (0 seconds, 0.0% of total time):

Finished in 0.20221 seconds (files took 4.54 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/models/success_criterion_spec.rb:123 # SuccessCriterion acts as list scopes by parent_id

Randomized with seed 41970

Any idea what's going on here?

@brendon
Copy link
Owner

brendon commented Sep 8, 2016

Hi @jmuheim, can you upgrade to the latest 0.8.x version and see if you're experiencing the same failure?

@jmuheim
Copy link
Author

jmuheim commented Sep 8, 2016

Yes I do.

@brendon
Copy link
Owner

brendon commented Sep 8, 2016

Ok, thanks for that. Are you able to isolate this failure so that you don't rely on acts_as_tree? That gem might be calling another callback that modify's the position again. Take a look at the SQL executed and that might also give us a clue as to what's going on.

@brendon
Copy link
Owner

brendon commented Nov 26, 2016

@jmuheim, can you let us know where you're at, or close the issue if you've managed to sort it out :)

@brendon
Copy link
Owner

brendon commented Nov 27, 2016

@jmuheim, I'm closing this for now. Let me know if you want to reopen it. Also, try master and see if that fixes the problem.

@brendon brendon closed this as completed Nov 27, 2016
@jmuheim
Copy link
Author

jmuheim commented Feb 17, 2017

I'm sorry for not responding. The problem has disappeared with the newest version.

Thank you.

@brendon
Copy link
Owner

brendon commented Feb 18, 2017

Great to hear! :)

@calleo
Copy link

calleo commented Dec 30, 2018

Just in case anyone else ends up here.

I was upgrading from 0.6 and tests started to fail when going from 0.7.2 to 0.7.4. All list items had position "1".

We are using Factory Girl 4.5 and Rails 4.2. This is how the code looks that was creating the list items list.items << create(:item)

Once I changed it to create(:item, list: list) things started to work again.

Note that we are using acts_as_list with a scope.

Looks like the position is first calculated when the list item is created (with scope "list_id = nil") which gives position "1". Appending the item to a list (using "<<") does not re-compute the position. So, supplying a list when the item is created fixes the problem.

Could be an issue with FactoryGirl that has been corrected long time ago.

@brendon
Copy link
Owner

brendon commented Jan 2, 2019

That's a strange one. Changing the list and re-saving the item should change the position to the next available position in the list. But you're right, supplying the list on create will give you correct positions :)

@brendon
Copy link
Owner

brendon commented Feb 13, 2019

@calleo, looking at this further, I'm wondering if << runs any callbacks on the item being appended to the list? It needs to run the callbacks to pick up the change in scope, otherwise its position will remain as it is.

If you wanted to look into this further, feel free :) I'm happy to provide some guidance if necessary :)

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

No branches or pull requests

3 participants