refinery / refinerycms-news

News Plugin for Refinery CMS
http://www.refinerycms.com
MIT License
121 stars 120 forks source link

How about support refinerycms edge vesion #140

Closed xiaods closed 10 years ago

xiaods commented 10 years ago

I found the master branch spec add this limitation: s.add_dependency 'refinerycms-core', '~> 2.1.1' I am always use a edge version, How about open it and let me experience it.

parndt commented 10 years ago

@xiaods I've gotten it part of the way but the tests are currently failing. It's on the rails4 branch and I'd appreciate your help getting it ready. :)

xiaods commented 10 years ago

@parndt i have bundle install the testing environment, ran into this issue, could you do me a favor?

dxiao at localhost in ~/Documents/Code/refinery/refinerycms-news on rails4
$ bundle install
    refinerycms-authentication (>= 0) ruby depends on
      mime-types (~> 1.16) ruby

    refinerycms-testing (>= 0) ruby depends on
      mime-types (2.0)

Bundler could not find compatible versions for gem "activesupport":
  In Gemfile:
    refinerycms-authentication (>= 0) ruby depends on
      activesupport (= 4.0.0) ruby

    refinerycms-news (>= 0) ruby depends on
      activesupport (4.0.0.beta1)
parndt commented 10 years ago

I got this, too, but fixed it eventually on my system.

Try putting in your Gemfile:

gem 'mime-types', '= 1.25.1'

Now, bundle install or bundle update or whatever needs to happen. It's because the Mail gem depends on mime-types at ~> 1.16 and other gems have moved on to 2.0. Relates to mikel/mail#641

xiaods commented 10 years ago

@parndt thansk, it works like a charm.

xiaods commented 10 years ago
Failures:

  1) Refinery::News::Item#archive should show 5 news items with publish dates in same month
     Failure/Error: 5.times { FactoryGirl.create(:news_item, :publish_date => publish_date) }
     ActiveRecord::RecordNotSaved:
       ActiveRecord::RecordNotSaved
     # ./spec/models/refinery/news/item_spec.rb:16:in `block (4 levels) in <module:News>'
     # ./spec/models/refinery/news/item_spec.rb:16:in `times'
     # ./spec/models/refinery/news/item_spec.rb:16:in `block (3 levels) in <module:News>'

Finished in 0.57248 seconds
1 example, 1 failure

Failed examples:

rspec ./spec/models/refinery/news/item_spec.rb:15 # Refinery::News::Item#archive should show 5 news items with publish dates in same month

no clue... need some times..

parndt commented 10 years ago

yeah, I think @shioyama could probably explain the failure? Maybe?

xiaods commented 10 years ago

through my inspect, the reason code is here: in app/models/refinery/news/item.rb

9   ¦ ¦ translates :title, :body, :slug                                                            │
 10                                                                                                  │Finished in 0.57407 seconds
 11   ¦ ¦ before_save do |m|                                                                         │1 example, 1 failure
 12   ¦ ¦ ¦ m.translation.globalized_model = self                                                    │
 13   ¦ ¦ ¦ m.translation.save if m.translation.new_record?                                          │Failed examples:
 14   ¦ ¦ end

if i comments this, all test pass.

dxiao at Deshis-MacBook-Pro in ~/Documents/Code/refinery/refinerycms-news on rails4*
$ bundle exec rspec  --color  spec/models/refinery/news/item_spec.rb
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
................

Finished in 0.39863 seconds
16 examples, 0 failures
xiaods commented 10 years ago

found the final reason: WARNING: Can't mass-assign protected attributes for Refinery::News::Item::Translation: locale

i feel this is regression bug.

shioyama commented 10 years ago

The problem is that the edge (rails4) branch of refinerycms-news is using the protected_attributes gem, which globalize does not support in versions > 4.0 (ActiveRecord 4 / Rails 4). This is not a regression bug. The solution is to get that gem off using protected attributes, and this error will disappear.

Hope that helps.

xiaods commented 10 years ago

@shioyama see my patch. it's ok.

shioyama commented 10 years ago

That patch looks good to me. @parndt that code for saving translations that you added in 06c3755 was needed in the AR3 version of globalize only, if I recall correctly.

parndt commented 10 years ago

@shioyama sweet. :smile:

xiaods commented 10 years ago

branch rails4 also support refinerycms edge version. so close this issue.