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

How about support refinerycms edge vesion #140

Closed
xiaods opened this issue Dec 8, 2013 · 13 comments
Closed

How about support refinerycms edge vesion #140

xiaods opened this issue Dec 8, 2013 · 13 comments

Comments

@xiaods
Copy link

xiaods commented Dec 8, 2013

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
Copy link
Member

parndt commented Dec 8, 2013

@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
Copy link
Author

xiaods commented Dec 9, 2013

@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
Copy link
Member

parndt commented Dec 9, 2013

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
Copy link
Author

xiaods commented Dec 9, 2013

@parndt thansk, it works like a charm.

@xiaods
Copy link
Author

xiaods commented Dec 9, 2013

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
Copy link
Member

parndt commented Dec 9, 2013

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

@xiaods
Copy link
Author

xiaods commented Dec 9, 2013

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
Copy link
Author

xiaods commented Dec 11, 2013

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

i feel this is regression bug.

xiaods pushed a commit to xiaods/refinerycms-news that referenced this issue Dec 12, 2013
globalize gem add a validation for locale field in translation table
we should be explicit expose locale

more details see refinery#140
@shioyama
Copy link

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
Copy link
Author

xiaods commented Dec 12, 2013

@shioyama see my patch. it's ok.

@shioyama
Copy link

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
Copy link
Member

parndt commented Dec 12, 2013

@shioyama sweet. 😄

@xiaods
Copy link
Author

xiaods commented Jan 25, 2014

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

@xiaods xiaods closed this as completed Jan 25, 2014
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