A possible fix for issue 340 #348

Closed
wants to merge 4 commits into
from

Projects

None yet

4 participants

@eitoball

Historically, #page_entries_info uses entry when collection is empty. But, I agree with @znz in amatsuda/kaminari#340. #page_entries_info should use model name even if the given collection is empty.

@yuki24 yuki24 was assigned Jun 21, 2013
@zzak
Collaborator
zzak commented Aug 7, 2013

I will merge this if rebased, thank you!

@yuki24
Collaborator
yuki24 commented Aug 7, 2013

👍

@eitoball
eitoball commented Aug 7, 2013

Rebased and pushed.

Some builds on Travis CI are failing, but I guess that they are not caused by changes I made.
Anyway, I have fixed some of those errors. I am working on error with sinatra, but I don't think that I can fix errors with data_mapper.
Please merge this PR if you think good enough.

Thank you.

@eitoball eitoball fix build error in spec with sinatra
The error was 'NameError: uninitialized constant Kaminari::ActionViewExtension'.
6003b84
@eitoball
eitoball commented Aug 7, 2013

For data_mapper issue, we need to wait DatabaseCleaner/database_cleaner#217 to be merged.

@eitoball
eitoball commented Aug 7, 2013

Even though this is database_cleaner issue, I wanted all specs to be green and cannot wait fix to be merged. So, I added the following code (in my local repo) at the beginning of spec/support/database_cleaner.rb to run specs with data_mapper. All green.

if defined? DataMapper
  unless DataMapper::Adapters::DataObjectsAdapter.instance_methods.include?(:use_sequence)
    uses_sequence_mod = Module.new do
      def uses_sequence
        select("SELECT name FROM sqlite_master WHERE type='table' AND name='sqlite_sequence';").include?("sqlite_sequence")
      end
    end
    %w{Sqlite3Adapter SqliteAdapter}.each do |adapter|
      begin
        DataMapper::Adapters.const_get(adapter, false).class_eval do
          include uses_sequence_mod
        end
      rescue NameError
        # Do nothing
      end
    end
  end
end
@amatsuda
Owner
amatsuda commented Aug 7, 2013

⚡️⚡️

@zzak zzak closed this in c7c88c5 Aug 8, 2013
@zzak
Collaborator
zzak commented Aug 8, 2013

@eitoball Thank you for the patch, we didn't merge the database_cleaner stuff because we are working on this separately.

@eitoball
eitoball commented Aug 8, 2013

@zzak Thank you, but I didn't like the way it was merged I didn't get any credits in commit log. 😢

@zzak
Collaborator
zzak commented Aug 8, 2013

@eitoball Sorry, we were trying out new pair setup.

If you want to reset your local HEAD and merge without the database_cleaner stuff I will revert and merge with your credit.

@eitoball
eitoball commented Aug 8, 2013

Thanks.
Since this PR is closed, I open another PR #430.

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