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

#update_all is not working correctly on ActiveRecord::Relation if #offset is added #10849

Closed
maciejkowalski opened this issue Jun 5, 2013 · 7 comments

Comments

@maciejkowalski
Copy link

I am working with Rails 3.2.13 on ruby 2.0.0p195 (2013-05-14 revision 40734) [x86_64-linux].

Here goes the gemfile:
https://gist.github.com/maciejkowalski/2ba3387d20a7eb750f50

Sample code:

class News << ActiveRecord::Base
  # ...
  after_save :limit_frontpage_news
  def limit_frontpage_news
    if (News.where(frontpage: true).count > 30)
      puts "before => #{News.where(frontpage: true).count}"
      puts "offset count -> #{News.where(frontpage: true).offset(30).count}"
      News.where(frontpage: true).offset(30).update_all(frontpage: false)
      # News.where(frontpage: true).offset(30).each do |news|
      #   news.update_column(:frontpage, false)
      # end
      puts "after => #{News.where(frontpage: true).count}"
    end
  end
end

And Spec code:

require 'spec_helper'

describe News do

  context 'frontpage callbacks' do
    msg = 'should be 30 frontpage news'

    context 'changing news to fronpage news' do

      context 'when there is < 30 frontpage news' do
        it msg do
          create_list(:news, 2, :published, :frontpage)
          news = create(:news, :published)
          news.should_receive(:limit_frontpage_news).and_call_original
          news.update_attributes(:frontpage => true)
        end
      end

      context 'when there is > 30 frontpage news' do
        it msg do
          create_list(:news, 30, :published, :frontpage)
          news = create(:news, :published)
          news.should_receive(:limit_frontpage_news).and_call_original
          news.update_attributes(:frontpage => true)
          News.where(frontpage: true).count.should eq(30)
        end
      end

    end

    context 'creating new fronpage news' do

      context 'when there is < 30 frontpage news' do
        it msg do
          News.any_instance.should_receive(:limit_frontpage_news).and_call_original
          create(:news, :published, :frontpage)
        end
      end

      context 'when there is > 30 frontpage news' do
        it msg do
          create_list(:news, 30, :published, :frontpage)
          News.any_instance.should_receive(:limit_frontpage_news).and_call_original
          create(:news, :published, :frontpage)
          News.where(frontpage: true).count.should eq(30)
        end
      end

    end

  end
end

When I run spec with Rails way code

News.where(frontpage: true).offset(30).update_all(frontpage: false)

I get errors, spec doesn't pass.
https://gist.github.com/maciejkowalski/ed9a9096f38c20cead6f

But when I do this via simple each everything is OK

      News.where(frontpage: true).offset(30).each do |news|
        news.update_column(:frontpage, false)
      end

Output:
https://gist.github.com/maciejkowalski/99af7eb498a470000492

@adamgamble
Copy link
Contributor

This is also an issue in 4.0

@acapilleri
Copy link
Contributor

@rafaelfranca I debugged this issue. Arel::UpdateManagerdoes not implement skipalias offset, do you think that we should implement this method in UpdateManager of Arel to fix this issue?
thanks in advance

@acapilleri
Copy link
Contributor

cc/ @carlosantoniodasilva

@arthurnn
Copy link
Member

arthurnn commented Apr 9, 2014

as @laurocaetano mentioned on his PR, this seems to be a AREL issue, @maciejkowalski do you mind opening a issue on the arel repo, reference this one, and close it in here?
Thanks

laurocaetano added a commit to laurocaetano/arel that referenced this issue Apr 27, 2014
@rails-bot
Copy link

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rails-bot
Copy link

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rvfx
Copy link

rvfx commented Sep 24, 2017

I just encountered this bug using Rails 5.0.4.

@rails-bot rails-bot bot removed the stale label Sep 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants