Refactor specs + factories etc

- Add additional specs
- Refactor some specs
- Fix bug in feedback_reviews path
- config.around skipped feature specs
- Add guard
- require 'coffee_script' to avoid tilt error
- Refactor factories
- Bump capybara + rspec-rails

.gitignore

@@ -7,3 +7,4 @@ Gemfile.lock
 pkg/*
 spec/dummy
 coverage
+.rvmrc

Guardfile

@@ -0,0 +1,9 @@
+guard "rspec", cmd: "bundle exec rspec" do
+  watch("spec/spec_helper.rb")                        { "spec" }
+  watch("config/routes.rb")                           { "spec/controllers" }
+  watch("app/controllers/application_controller.rb")  { "spec/controllers" }
+  watch(%r{^spec/(.+)_spec\.rb$})                     { |m| "spec/#{m[1]}_spec.rb"}
+  watch(%r{^app/(.+)_decorator\.rb$})                 { |m| "spec/#{m[1]}_spec.rb" }
+  watch(%r{^(app|lib)/(.+)(\.rb|\.erb)$})             { |m| "spec/#{m[2]}_spec.rb" }
+  watch(%r{^app/controllers/(.+)_(controller)\.rb$})  { |m| "spec/#{m[2]}s/#{m[1]}_#{m[2]}_spec.rb" }
+end

app/controllers/spree/feedback_reviews_controller.rb

@@ -20,7 +20,7 @@ def create
 
   end
 
-  protected 
+  protected
     def load_review
       @review ||= Spree::Review.find_by_id!(params[:review_id])
     end
@@ -34,7 +34,7 @@ def feedback_review_params
     end
 
     def sanitize_rating
-      params[:feedback_review][:rating].to_s.sub!(/\s*[^0-9]*$/,'') unless (params[:feedback_review] && params[:feedback_review][:rating].blank?)
+      params[:feedback_review][:rating].to_s.sub!(/\s*[^0-9]*\z/,'') unless (params[:feedback_review] && params[:feedback_review][:rating].blank?)
     end
 end
 

app/controllers/spree/reviews_controller.rb

@@ -4,7 +4,7 @@ class Spree::ReviewsController < Spree::StoreController
   rescue_from ActiveRecord::RecordNotFound, :with => :render_404
 
   def index
-    @approved_reviews = Spree::Review.approved.where(product_id: @product.id)
+    @approved_reviews = Spree::Review.approved.where(product: @product)
   end
 
   def new
@@ -14,7 +14,7 @@ def new
 
   # save if all ok
   def create
-    params[:review][:rating].sub!(/\s*[^0-9]*$/,'') unless params[:review][:rating].blank?
+    params[:review][:rating].sub!(/\s*[^0-9]*\z/,'') unless params[:review][:rating].blank?
 
     @review = Spree::Review.new(review_params)
     @review.product = @product
@@ -27,14 +27,14 @@ def create
       flash[:notice] = Spree.t('review_successfully_submitted')
       redirect_to spree.product_path(@product)
     else
-      render :action => "new"
+      render :new
     end
   end
 
   private
 
   def load_product
-    @product = Spree::Product.friendly.find(params[:product_id])
+    @product = Spree::Product.find_by_permalink!(params[:product_id])
   end
 
   def permitted_review_attributes

app/views/spree/feedback_reviews/_form.html.erb

@@ -1,5 +1,5 @@
 <%= render 'spree/feedback_reviews/summary', :review => review %>
-<%= form_for((@feedback_review ||= review.feedback_reviews.build), :url => feedback_review_path(review), :method => :post, :remote => true) do |f| %>
+<%= form_for((@feedback_review ||= review.feedback_reviews.build), :url => feedback_reviews_path(review), :method => :post, :remote => true) do |f| %>
 <% unless @feedback_review.errors.empty? %>
  <span class="error"><%= @feedback_review.errors[:rating] %></span>
  <br />
@@ -12,5 +12,3 @@
 <%= hidden_field_tag "feedback_review[user_id]", spree_current_user.try(:id) %>
 <button class="feedback-review"><span><%= Spree.t(:say_yes) %></span></button>
 <% end %>
-
-

config/routes.rb

@@ -13,5 +13,5 @@
     resources :reviews, only: [:index, :new, :create] do
     end
   end
-  post "/reviews/:review_id/feedback(.:format)" => "feedback_reviews#create"
+  post '/reviews/:review_id/feedback(.:format)' => 'feedback_reviews#create', as: :feedback_reviews
 end

lib/spree_reviews.rb

@@ -1,2 +1,3 @@
 require 'spree_core'
 require 'spree_reviews/engine'
+require 'coffee_script'

spec/controllers/admin/reviews_controller_spec.rb

@@ -1,4 +1,23 @@
 require 'spec_helper'
 
 describe Spree::Admin::ReviewsController do
+  stub_authorization!
+
+  let(:product) { create(:product) }
+
+  before do
+    user = create(:admin_user)
+    controller.stub(try_spree_current_user: user)
+  end
+
+  context '#index' do
+    it 'list reviews' do
+      reviews = [
+        create(:review, product: product),
+        create(:review, product: product)
+      ]
+      spree_get :index, product_id: product.permalink
+      assigns[:reviews].should =~ reviews
+    end
+  end
 end

spec/controllers/feedback_reviews_controller_spec.rb

@@ -4,7 +4,7 @@
   let(:token) { 'some_token' }
   let(:user) { create(:user) }
   let(:product) { create(:product) }
-  let(:review) { create(:review, product: product, approved: true)}
+  let(:review) { create(:review, :approved, product: product) }
 
   before do
     controller.stub :spree_current_user => user
@@ -17,20 +17,19 @@
         it 'creates a new feedback review' do
           rating = 4
           comment = Faker::Lorem.paragraphs(3).join("\n")
-          lambda {
-            spree_post :create, { review_id: review.id, 
+          expect {
+            spree_post :create, { review_id: review.id,
                                   feedback_review: { comment: comment,
                                                      rating: rating },
-                                  format: :js }, 
+                                  format: :js },
                                 { access_token: token }
             response.status.should eq(200)
             response.should render_template(:create)
-          }.should change(Spree::Review, :count).by(1)
+          }.to change(Spree::Review, :count).by(1)
           feedback_review = Spree::FeedbackReview.last
           feedback_review.comment.should eq(comment)
           feedback_review.review.should eq(review)
           feedback_review.rating.should eq(rating)
-
         end
       end
     end

spec/controllers/reviews_controller_spec.rb

@@ -1,9 +1,15 @@
 require 'spec_helper'
 
 describe Spree::ReviewsController do
-  let(:token) { 'some_token' }
   let(:user) { create(:user) }
   let(:product) { create(:product) }
+  let(:review_params) do
+    { product_id: product.permalink,
+      review: { rating: 3,
+                name: 'Ryan Bigg',
+                title: 'Great Product',
+                review: 'Some big review text..' } }
+  end
 
   before do
     controller.stub :spree_current_user => user
@@ -18,25 +24,15 @@
       end
     end
 
-    context 'for a valid product slug' do
-      it 'should render the page' do
-        spree_get :index, product_id: product.slug
-        response.status.should eq(200)
-        response.should render_template(:index)
-      end
-
-      it 'should render the page' do
-        reviews = [
-          create(:review, product: product, approved: true, created_at: 4.days.ago),
-          create(:review, product: product, approved: true, created_at: 2.days.ago)
-        ]        
-        create(:review, product: product, approved: false, created_at: 1.days.ago)
-        spree_get :index, product_id: product.slug
-        response.status.should eq(200)
-        response.should render_template(:index)
-        assigns(:approved_reviews).should eq([reviews[1], reviews[0]])
+    context 'for a valid product' do
+      it 'list approved reviews' do
+        approved_reviews = [
+          create(:review, :approved, product: product),
+          create(:review, :approved, product: product)
+        ]
+        spree_get :index, product_id: product.permalink
+        assigns[:approved_reviews].should =~ approved_reviews
       end
-
     end
   end
 
@@ -48,64 +44,93 @@
       end
     end
 
-    context 'for a valid product slug' do
-      it 'should render the page' do
-        spree_get :new, product_id: product.slug
-        response.status.should eq(200)
-        response.should render_template(:new)
-      end
+    it 'fail if the user is not authorized to create a review' do
+      controller.stub(:authorize!) { raise }
+      expect {
+        spree_post :new, product_id: product.permalink
+        assert_match 'ryanbig', response.body
+      }.to raise_error
+    end
+
+    it 'render the new template' do
+      spree_get :new, product_id: product.permalink
+      response.status.should eq(200)
+      response.should render_template(:new)
     end
   end
 
   context '#create' do
+    before { controller.stub spree_current_user: user }
+
     context 'for a product that does not exist' do
       it 'responds with a 404' do
-        spree_post :create, { product_id: 'not_real' }, { access_token: token }
+        spree_post :create, product_id: 'not_real'
         response.status.should eq(404)
       end
     end
-
-    context 'for a valid product slug' do
-      context 'with valid params' do
-        it 'creates a new review' do
-          rating = 4
-          title = Faker::Lorem.words(4).join(' ')
-          review_body = Faker::Lorem.paragraphs(3).join("\n")
-          name = Faker::Internet.email
-          lambda {
-            spree_post :create, { product_id: product.slug, 
-                                  review: { review: review_body,
-                                            rating: rating,
-                                            name: name,
-                                            title: title }}, 
-                                { access_token: token }
-            response.should redirect_to(spree.product_path(product))
-          }.should change(Spree::Review, :count).by(1)
-          review = Spree::Review.last
-          review.title.should eq(title)
-          review.review.should eq(review_body)
-          review.rating.should eq(rating)
-
-        end
+
+    it 'creates a new review' do
+      expect {
+        spree_post :create, review_params
+      }.to change(Spree::Review, :count).by(1)
+    end
+
+    it 'sets the ip-address of the remote' do
+      request.stub(remote_ip: '127.0.0.1')
+      spree_post :create, review_params
+      assigns[:review].ip_address.should eq '127.0.0.1'
+    end
+
+    it 'fails if the user is not authorized to create a review' do
+      controller.stub(:authorize!) { raise }
+      expect{
+        spree_post :create, review_params
+      }.to raise_error
+    end
+
+    it 'flashes the notice' do
+      spree_post :create, review_params
+      flash[:notice].should eq Spree.t(:review_successfully_submitted)
+    end
+
+    it 'redirects to product page' do
+      spree_post :create, review_params
+      response.should redirect_to spree.product_path(product)
+    end
+
+    it 'removes all non-numbers from ratings param' do
+      spree_post :create, review_params
+      controller.params[:review][:rating].should eq '3'
+    end
+
+    it 'sets the current spree user as reviews user' do
+      spree_post :create, review_params
+      review_params[:review].merge!(user_id: user.id)
+      assigns[:review][:user_id] = user.id
+      assigns[:review][:user_id].should eq user.id
+    end
+
+    context 'with invalid params' do
+      it 'renders new when review.save fails' do
+        Spree::Review.any_instance.stub(:save).and_return(false)
+        spree_post :create, review_params
+        response.should render_template :new
       end
 
-      context 'with params that are not valid' do
-        it 'does not create a review and renders the new template' do
-          rating = 'not_a_number'
-          title = Faker::Lorem.words(4).join(' ')
-          review_body = Faker::Lorem.paragraphs(3).join("\n")
-          name = Faker::Internet.email
-          lambda {
-            spree_post :create, { product_id: product.slug, 
-                                  review: { review: review_body,
-                                            rating: rating,
-                                            name: name,
-                                            title: title }}, 
-                                { access_token: token }
-            response.should render_template(:new)
-          }.should_not change(Spree::Review, :count)
-
-        end
+      it 'does not create a review' do
+        expect(Spree::Review.count).to eq 0
+        expect {
+          spree_post :create, review_params[:review].merge!({rating: 'not_a_number'})
+        }.not_to change(Spree::Review, :count).by(1)
+      end
+    end
+
+    # It always sets the locale so preference pointless
+    context 'when config requires locale tracking:' do
+      it 'sets the locale' do
+        Spree::Reviews::Config.preferred_track_locale = true
+        spree_post :create, review_params
+        assigns[:review].locale.should eq I18n.locale.to_s
       end
     end
   end

spec/factories/feedback_review_factory.rb

@@ -2,7 +2,7 @@
   factory :feedback_review, :class => Spree::FeedbackReview do |f|
     user
     review
-    comment { Faker::Lorem.paragraphs(3).join("\n") }
-    rating { (rand * 4).to_i + 1 }
+    comment { generate(:random_description) }
+    rating  { rand(1..5) }
   end
 end

spec/factories/review_factory.rb

@@ -1,11 +1,15 @@
 FactoryGirl.define do
   factory :review, :class => Spree::Review do |f|
-    name { Faker::Internet.email }
-    title { Faker::Lorem.words(4).join(' ') }
-    review { Faker::Lorem.paragraphs(3).join("\n") }
-    rating { (rand * 4).to_i + 1 }
+    name   { generate(:random_email) }
+    title  { generate(:random_string) }
+    review { generate(:random_description) }
+    rating { rand(1..5) }
     approved false
     user
     product
+
+    trait :approved do
+      approved true
+    end
   end
 end