refinery / refinerycms-news

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

Use translated scope in Archive because we should #121

Closed harley closed 11 years ago

harley commented 11 years ago

I came across this because I get an exception "ActionView::Template::Error: undefined method 'html_safe' for nil:NilClass" which is caused by truncate(...) returning nil on line https://github.com/refinery/refinerycms-news/blob/master/app/helpers/refinery/news/items_helper.rb#L24

Easily reproducable by having 2 locales and create a news item in one language but not the 2nd, then switching to the 2nd locale to view the Archive page, it would crash because it loads some news items with 'item.body == nil' (causing truncate(item.body) to be nil).

There are validations for News::Item so we just need to make sure to load only valid records. This is also consistent with the rest of the refinery/news/items_controller.rb file

The commit in this pull can also be cherry-picked into the 2-0-stable branch

ugisozols commented 11 years ago

Would you be keen to write specs for this so that we don't break it in the future?

harley commented 11 years ago

sure! hold on

harley commented 11 years ago

hmm i can't seem to run any test because of the following error

~/code/own/refinerycms-news use_translated_scope ✔ ➤ bundle exec rspec spec/helpers/refinery/news/items_helper_spec.rb
/Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration.rb:991:in `require': cannot load such file -- nc (LoadError)
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration.rb:991:in `rescue in custom_formatter'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration.rb:988:in `custom_formatter'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration.rb:490:in `add_formatter'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration_options.rb:30:in `block in configure'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration_options.rb:30:in `each'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/configuration_options.rb:30:in `configure'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/command_line.rb:21:in `run'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/runner.rb:80:in `run'
        from /Users/username/.rvm/gems/ruby-1.9.3-p194/gems/rspec-core-2.12.2/lib/rspec/core/runner.rb:17:in `block in autorun'

haven't seen that "cannot load such file -- nc (LoadError)" and googling didn't seem to help. any ideas?

i'll push the spec update anyway so you can run it if all tests pass on your machine. thanks!

ugisozols commented 11 years ago

Haven't seen that nc error either... did you generate dummy app?

ugisozols commented 11 years ago

Specs do pass for me locally so could you do one more thing - squash commits and force push. Thanks! :)

harley commented 11 years ago

squashed. good to merge!

PS: i did generate a dummy app with bundle exec rake refinery:testing:dummy_app -- tried it without the dummy app as well and got the same error

harley commented 11 years ago

oh nevermind i had a ~/.rspec file which contains --format=Nc which i suppose isn't valid for this version of rspec. removed that and all tests pass now. sorry :)

ugisozols commented 11 years ago

Thanks @harleyttd!

harley commented 11 years ago

thanks for merging. can you also cherry-pick into 2-0-stable? or is it better if i make a pull against 2-0-stable? (didnt like the idea of 2 pulls for the same thing)

ugisozols commented 11 years ago

I cherry-picked and pushed to 2-0-stable in 3431029.

harley commented 11 years ago

:heartbeat: