[WIP] refactor the problems_controller

Extract the filter `find_problem` in decent_exposure to get this problem

[WIP] refactor the problems_controller
Extract the filter `find_problem` in decent_exposure to get this problem
Cyril Mougel
1 parent d0894cbe
Showing 7 changed files with 65 additions and 50 deletions Show diff stats
app/controllers/problems_controller.rb
app/views/problems/_issue_tracker_links.html.haml
app/views/problems/show.html.haml
app/views/problems/show.ics.haml
spec/controllers/problems_controller_spec.rb
spec/views/problems/show.html.haml_spec.rb
spec/views/problems/show.ics.haml_spec.rb
@@ -6,7 +6,6 @@
 # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search
 class ProblemsController < ApplicationController
-  before_filter :find_problem, :except => [:index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search]
   before_filter :find_selected_problems, :only => [:destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several]
   before_filter :set_sorting_params, :only => [:index, :all, :search]
   before_filter :set_tracker_params, :only => [:create_issue]
@@ -18,6 +17,9 @@ class ProblemsController &lt; ApplicationController
       current_user.apps.find(params[:app_id])
     end
   }
+  expose(:problem) {
+    app.problems.find(params[:id])
+  }
   def index
     app_scope = current_user.admin? ? App.all : current_user.apps
@@ -33,28 +35,28 @@ class ProblemsController &lt; ApplicationController
   end
   def show
-    @notices  = @problem.notices.reverse_ordered.page(params[:notice]).per(1)
+    @notices  = problem.notices.reverse_ordered.page(params[:notice]).per(1)
     @notice   = @notices.first
     @comment = Comment.new
   end
   def create_issue
-    issue_creation = IssueCreation.new(@problem, current_user, params[:tracker])
+    issue_creation = IssueCreation.new(problem, current_user, params[:tracker])
     unless issue_creation.execute
       flash[:error] = issue_creation.errors[:base].first
     end
-    redirect_to app_problem_path(app, @problem)
+    redirect_to app_problem_path(app, problem)
   end
   def unlink_issue
-    @problem.update_attribute :issue_link, nil
-    redirect_to app_problem_path(app, @problem)
+    problem.update_attribute :issue_link, nil
+    redirect_to app_problem_path(app, problem)
   end
   def resolve
-    @problem.resolve!
+    problem.resolve!
     flash[:success] = 'Great news everyone! The err has been resolved.'
     redirect_to :back
   rescue ActionController::RedirectBackError
@@ -109,10 +111,6 @@ class ProblemsController &lt; ApplicationController
   protected
-    def find_problem
-      @problem = app.problems.find(params[:id])
-    end
-
     def set_tracker_params
       IssueTracker.default_url_options[:host] = request.host
       IssueTracker.default_url_options[:port] = request.port
 - if app.issue_tracker_configured? || current_user.github_account?
-  - if @problem.issue_link.present?
-    %span= link_to 'go to issue', @problem.issue_link, :class => "#{@problem.issue_type}_goto goto-issue"
-    = link_to 'unlink issue', unlink_issue_app_problem_path(app, @problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue"
-  - elsif @problem.issue_link == "pending"
-    %span.disabled= link_to 'creating...', '#', :class => "#{@problem.issue_type}_inactive create-issue"
-    = link_to 'retry', create_issue_app_problem_path(app, @problem), :method => :post
+  - if problem.issue_link.present?
+    %span= link_to 'go to issue', problem.issue_link, :class => "#{problem.issue_type}_goto goto-issue"
+    = link_to 'unlink issue', unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue"
+  - elsif problem.issue_link == "pending"
+    %span.disabled= link_to 'creating...', '#', :class => "#{problem.issue_type}_inactive create-issue"
+    = link_to 'retry', create_issue_app_problem_path(app, problem), :method => :post
   - else
     - if app.github_repo?
       - if current_user.can_create_github_issues?
-        %span= link_to 'create issue', create_issue_app_problem_path(app, @problem, :tracker => 'user_github'), :method => :post, :class => "github_create create-issue"
+        %span= link_to 'create issue', create_issue_app_problem_path(app, problem, :tracker => 'user_github'), :method => :post, :class => "github_create create-issue"
       - elsif app.issue_tracker_configured? && app.issue_tracker.label.eql?('github')
-        %span= link_to 'create issue', create_issue_app_problem_path(app, @problem), :method => :post, :class => "github_create create-issue"
+        %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "github_create create-issue"
     - if app.issue_tracker_configured? && !app.issue_tracker.label.eql?('github')
-      %span= link_to 'create issue', create_issue_app_problem_path(app, @problem), :method => :post, :class => "#{app.issue_tracker.label}_create create-issue"
+      %span= link_to 'create issue', create_issue_app_problem_path(app, problem), :method => :post, :class => "#{app.issue_tracker.label}_create create-issue"
-- content_for :page_title, @problem.message
+- content_for :page_title, problem.message
 - content_for :title_css_class, 'err_show'
-- content_for :title, @problem.error_class || truncate(@problem.message, :length => 32)
+- content_for :title, problem.error_class || truncate(problem.message, :length => 32)
 - content_for :meta do
   %strong App:
   = link_to app.name, app
   %strong Where:
-  = @problem.where
+  = problem.where
   %br
   %strong Environment:
-  = @problem.environment
+  = problem.environment
   %strong Last Notice:
-  = @problem.last_notice_at.to_s(:precise)
+  = problem.last_notice_at.to_s(:precise)
 - content_for :action_bar do
-  - if @problem.unresolved?
-    %span= link_to 'resolve', [:resolve, app, @problem], :method => :put, :data => { :confirm => problem_confirm }, :class => 'resolve'
+  - if problem.unresolved?
+    %span= link_to 'resolve', [:resolve, app, problem], :method => :put, :data => { :confirm => problem_confirm }, :class => 'resolve'
   - if current_user.authentication_token
-    %span= link_to 'iCal', polymorphic_path([app, @problem], :format => "ics", :auth_token => current_user.authentication_token), :class => "calendar_link"
+    %span= link_to 'iCal', polymorphic_path([app, problem], :format => "ics", :auth_token => current_user.authentication_token), :class => "calendar_link"
   %span>= link_to 'up', (request.env['HTTP_REFERER'] ? :back : app_problems_path(app)), :class => 'up'
   %br
   = render "issue_tracker_links"
-- if @problem.comments_allowed? || @problem.comments.any?
+- if problem.comments_allowed? || problem.comments.any?
   - content_for :comments do
     %h3 Comments
-    - @problem.comments.each do |comment|
+    - problem.comments.each do |comment|
       .window
         %table.comment
           %tr
@@ -37,11 +37,11 @@
               - else
                 %span.comment-info
                   = time_ago_in_words(comment.created_at, true) << " ago by [Unknown User]"
-              %span.delete= link_to '&#10008;'.html_safe, [app, @problem, comment], :method => :delete, :data => { :confirm => "Are you sure you don't need this comment?" }, :class => "destroy-comment"
+              %span.delete= link_to '&#10008;'.html_safe, [app, problem, comment], :method => :delete, :data => { :confirm => "Are you sure you don't need this comment?" }, :class => "destroy-comment"
           %tr
             %td= simple_format comment.body
-    - if @problem.comments_allowed?
-      = form_for [app, @problem, @comment] do |comment_form|
+    - if problem.comments_allowed?
+      = form_for [app, problem, @comment] do |comment_form|
         %p Add a comment
         = comment_form.text_area :body
         = comment_form.submit "Save Comment"
@@ -63,7 +63,7 @@
 - if @notice
   #summary
     %h3 Summary
-    = render 'notices/summary', :notice => @notice, :problem => @problem
+    = render 'notices/summary', :notice => @notice
   #backtrace
     %h3 Backtrace
-= generate_problem_ical(@problem.notices.order_by(:created_at.asc))
+= generate_problem_ical(problem.notices.order_by(:created_at.asc))
@@ -138,7 +138,7 @@ describe ProblemsController do
       it "finds the problem" do
         get :show, :app_id => app.id, :id => err.problem.id
-        assigns(:problem).should == err.problem
+        controller.problem.should == err.problem
       end
       it "successfully render page" do
@@ -179,7 +179,7 @@ describe ProblemsController do
       it 'finds the problem if the user is watching the app' do
         get :show, :app_id => @watched_app.to_param, :id => @watched_err.problem.id
-        assigns(:problem).should == @watched_err.problem
+        controller.problem.should == @watched_err.problem
       end
       it 'raises a DocumentNotFound error if the user is not watching the app' do
@@ -205,7 +205,7 @@ describe ProblemsController do
       @problem.app.problems.should_receive(:find).and_return(@problem.problem)
       put :resolve, :app_id => @problem.app.id, :id => @problem.problem.id
       controller.app.should == @problem.app
-      assigns(:problem).should == @problem.problem
+      controller.problem.should == @problem.problem
     end
     it "should resolve the issue" do
 require 'spec_helper'
 describe "problems/show.html.haml" do
+  let(:problem) { Fabricate(:problem) }
+  let(:comment) { Fabricate(:comment) }
+
   before do
-    problem = Fabricate(:problem)
-    comment = Fabricate(:comment)
-    assign :problem, problem
-    assign :comment, comment
     view.stub(:app).and_return(problem.app)
+    view.stub(:problem).and_return(problem)
+
+    assign :comment, comment
     assign :notices, problem.notices.page(1).per(1)
     assign :notice, problem.notices.first
+
     controller.stub(:current_user) { Fabricate(:user) }
   end
   def with_issue_tracker(tracker, problem)
     problem.app.issue_tracker = tracker.new :api_token => "token token token", :project_id => "1234"
-    assign :problem, problem
+    view.stub(:problem).and_return(problem)
     view.stub(:app).and_return(problem.app)
   end
@@ -54,7 +57,7 @@ describe &quot;problems/show.html.haml&quot; do
     it "should link 'up' to app_problems_path if HTTP_REFERER isn't set'" do
       controller.request.env['HTTP_REFERER'] = nil
       problem = Fabricate(:problem_with_comments)
-      assign :problem, problem
+      view.stub(:problem).and_return(problem)
       view.stub(:app).and_return(problem.app)
       render
@@ -67,7 +70,7 @@ describe &quot;problems/show.html.haml&quot; do
         controller.stub(:current_user) { user }
         problem = Fabricate(:problem_with_comments, :app => Fabricate(:app, :github_repo => "test_user/test_repo"))
-        assign :problem, problem
+        view.stub(:problem).and_return(problem)
         view.stub(:app).and_return(problem.app)
         render
@@ -77,7 +80,7 @@ describe &quot;problems/show.html.haml&quot; do
       it 'should allow creating issue for github if application has a github tracker' do
         problem = Fabricate(:problem_with_comments, :app => Fabricate(:app, :github_repo => "test_user/test_repo"))
         with_issue_tracker(GithubIssuesTracker, problem)
-        assign :problem, problem
+        view.stub(:problem).and_return(problem)
         view.stub(:app).and_return(problem.app)
         render
@@ -89,7 +92,7 @@ describe &quot;problems/show.html.haml&quot; do
         let(:app) { App.new(:new_record => false) }
         it 'not see link to create issue' do
-          assign :problem, problem
+          view.stub(:problem).and_return(problem)
           view.stub(:app).and_return(problem.app)
           render
           expect(view.content_for(:action_bar)).to_not match(/create issue/)
@@ -105,7 +108,7 @@ describe &quot;problems/show.html.haml&quot; do
         context "with problem without issue link" do
           let(:problem){ Problem.new(:new_record => false, :app => app) }
           it 'not see link if no issue tracker' do
-            assign :problem, problem
+            view.stub(:problem).and_return(problem)
             view.stub(:app).and_return(problem.app)
             render
             expect(view.content_for(:action_bar)).to match(/create issue/)
@@ -117,7 +120,7 @@ describe &quot;problems/show.html.haml&quot; do
           let(:problem){ Problem.new(:new_record => false, :app => app, :issue_link => 'http://foo') }
           it 'not see link if no issue tracker' do
-            assign :problem, problem
+            view.stub(:problem).and_return(problem)
             view.stub(:app).and_return(problem.app)
             render
             expect(view.content_for(:action_bar)).to_not match(/create issue/)
@@ -136,7 +139,7 @@ describe &quot;problems/show.html.haml&quot; do
     it 'should display comments and new comment form when no issue tracker' do
       problem = Fabricate(:problem_with_comments)
-      assign :problem, problem
+      view.stub(:problem).and_return(problem)
       view.stub(:app).and_return(problem.app)
       render
@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+describe "problems/show.html.ics" do
+  let(:problem) { Fabricate(:problem) }
+  before do
+    view.stub(:problem).and_return(problem)
+  end
+
+  it 'should work' do
+    render :template => 'problems/show.ics.haml'
+  end
+
+
+end
	@@ -138,7 +138,7 @@ describe ProblemsController do		@@ -138,7 +138,7 @@ describe ProblemsController do
138		138
139	it "finds the problem" do	139	it "finds the problem" do
140	get :show, :app_id => app.id, :id => err.problem.id	140	get :show, :app_id => app.id, :id => err.problem.id
141	- assigns(:problem).should == err.problem	141	+ controller.problem.should == err.problem
142	end	142	end
143		143
144	it "successfully render page" do	144	it "successfully render page" do
	@@ -179,7 +179,7 @@ describe ProblemsController do		@@ -179,7 +179,7 @@ describe ProblemsController do
179		179
180	it 'finds the problem if the user is watching the app' do	180	it 'finds the problem if the user is watching the app' do
181	get :show, :app_id => @watched_app.to_param, :id => @watched_err.problem.id	181	get :show, :app_id => @watched_app.to_param, :id => @watched_err.problem.id
182	- assigns(:problem).should == @watched_err.problem	182	+ controller.problem.should == @watched_err.problem
183	end	183	end
184		184
185	it 'raises a DocumentNotFound error if the user is not watching the app' do	185	it 'raises a DocumentNotFound error if the user is not watching the app' do
	@@ -205,7 +205,7 @@ describe ProblemsController do		@@ -205,7 +205,7 @@ describe ProblemsController do
205	@problem.app.problems.should_receive(:find).and_return(@problem.problem)	205	@problem.app.problems.should_receive(:find).and_return(@problem.problem)
206	put :resolve, :app_id => @problem.app.id, :id => @problem.problem.id	206	put :resolve, :app_id => @problem.app.id, :id => @problem.problem.id
207	controller.app.should == @problem.app	207	controller.app.should == @problem.app
208	- assigns(:problem).should == @problem.problem	208	+ controller.problem.should == @problem.problem
209	end	209	end
210		210
211	it "should resolve the issue" do	211	it "should resolve the issue" do
@@ -0,0 +1,14 @@		@@ -0,0 +1,14 @@
	1	+require 'spec_helper'
	2	+
	3	+describe "problems/show.html.ics" do
	4	+ let(:problem) { Fabricate(:problem) }
	5	+ before do
	6	+ view.stub(:problem).and_return(problem)
	7	+ end
	8	+
	9	+ it 'should work' do
	10	+ render :template => 'problems/show.ics.haml'
	11	+ end
	12	+
	13	+
	14	+end