From 11b082045552d616dad01df7f7ee5a387b409863 Mon Sep 17 00:00:00 2001 From: Cyril Mougel Date: Mon, 15 Jul 2013 11:40:23 +0200 Subject: [PATCH] Little refactoring of action in ProblemController --- app/controllers/problems_controller.rb | 60 ++++++++++++++++++++++++++++++++++++++---------------------- config/locales/en.yml | 9 ++++++++- spec/controllers/problems_controller_spec.rb | 38 +++++++++++++++++++++++--------------- spec/models/problem_spec.rb | 1 - 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index 404a693..6e71821 100644 --- a/app/controllers/problems_controller.rb +++ b/app/controllers/problems_controller.rb @@ -6,10 +6,13 @@ # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search class ProblemsController < ApplicationController - 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] + before_filter :need_selected_problem, :only => [ + :resolve_several, :unresolve_several, :unmerge_several + ] + expose(:app) { if current_user.admin? App.find(params[:app_id]) @@ -17,15 +20,23 @@ class ProblemsController < ApplicationController current_user.apps.find(params[:app_id]) end } + expose(:problem) { app.problems.find(params[:id]) } + expose(:err_ids) { + (params[:problems] || []).compact + } + + expose(:selected_problems) { + Array(Problem.find(err_ids)) + } + def index app_scope = current_user.admin? ? App.all : current_user.apps @all_errs = params[:all_errs] @problems = Problem.for_apps(app_scope).in_env(params[:environment]).all_else_unresolved(@all_errs).ordered_by(@sort, @order) - @selected_problems = params[:problems] || [] respond_to do |format| format.html do @problems = @problems.page(params[:page]).per(current_user.per_page) @@ -64,35 +75,40 @@ class ProblemsController < ApplicationController end def resolve_several - @selected_problems.each(&:resolve!) - flash[:success] = "Great news everyone! #{I18n.t(:n_errs_have, :count => @selected_problems.count)} been resolved." + selected_problems.each(&:resolve!) + flash[:success] = "Great news everyone! #{I18n.t(:n_errs_have, :count => selected_problems.count)} been resolved." redirect_to :back end def unresolve_several - @selected_problems.each(&:unresolve!) - flash[:success] = "#{I18n.t(:n_errs_have, :count => @selected_problems.count)} been unresolved." + selected_problems.each(&:unresolve!) + flash[:success] = "#{I18n.t(:n_errs_have, :count => selected_problems.count)} been unresolved." redirect_to :back end + ## + # Action to merge several Problem in One problem + # + # @param [ Array ] :problems the list of problem ids + # def merge_several - if @selected_problems.length < 2 - flash[:notice] = "You must select at least two errors to merge" + if selected_problems.length < 2 + flash[:notice] = I18n.t('controllers.problems.flash.need_two_errors_merge') else - @merged_problem = Problem.merge!(@selected_problems) - flash[:notice] = "#{@selected_problems.count} errors have been merged." + ProblemMerge.new(selected_problems).merge + flash[:notice] = I18n.t('controllers.problems.flash.merge_several.success', :nb => selected_problems.count) end redirect_to :back end def unmerge_several - all = @selected_problems.map(&:unmerge!).flatten + all = selected_problems.map(&:unmerge!).flatten flash[:success] = "#{I18n.t(:n_errs_have, :count => all.length)} been unmerged." redirect_to :back end def destroy_several - nb_problem_destroy = ProblemDestroy.execute(@selected_problems) + nb_problem_destroy = ProblemDestroy.execute(selected_problems) flash[:notice] = "#{I18n.t(:n_errs_have, :count => nb_problem_destroy)} been deleted." redirect_to :back end @@ -117,20 +133,20 @@ class ProblemsController < ApplicationController IssueTracker.default_url_options[:protocol] = request.scheme end - def find_selected_problems - err_ids = (params[:problems] || []).compact - if err_ids.empty? - flash[:notice] = "You have not selected any errors" - redirect_to :back - else - @selected_problems = Array(Problem.find(err_ids)) - end - end - def set_sorting_params @sort = params[:sort] @sort = "last_notice_at" unless %w{app message last_notice_at last_deploy_at count}.member?(@sort) @order = params[:order] || "desc" end + + ## + # Redirect :back if no errors selected + # + def need_selected_problem + if err_ids.empty? + flash[:notice] = I18n.t('controllers.problems.flash.no_select_problem') + redirect_to :back + end + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 7220718..e0174b4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2,7 +2,7 @@ # See http://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points. en: - hello: "Hello world" + flash: apps: create: @@ -11,6 +11,7 @@ en: success: "Good news everyone! '%{app_name}' was successfully updated." destroy: success: "'%{app_name}' was successfully destroyed." + n_errs_have: one: "%{count} err has" other: "%{count} errs have" @@ -38,6 +39,12 @@ en: error: "You can't delete yourself" update: success: "%{name}'s information was successfully updated." + problems: + flash: + no_select_problem: "You have not selected any errors" + need_two_errors_merge: "You must select at least two errors to merge" + merge_several: + success: "%{nb} errors have been merged." devise: registrations: diff --git a/spec/controllers/problems_controller_spec.rb b/spec/controllers/problems_controller_spec.rb index 6e3f4b1..b86a467 100644 --- a/spec/controllers/problems_controller_spec.rb +++ b/spec/controllers/problems_controller_spec.rb @@ -341,31 +341,25 @@ describe ProblemsController do @problem2 = Fabricate(:err, :problem => Fabricate(:problem, :resolved => false)).problem end - it "should apply to multiple problems" do - post :resolve_several, :problems => [@problem1.id.to_s, @problem2.id.to_s] - assigns(:selected_problems).should == [@problem1, @problem2] - end - - it "should require at least one problem" do - post :resolve_several, :problems => [] - request.flash[:notice].should match(/You have not selected any/) - end - context "POST /problems/merge_several" do it "should require at least two problems" do post :merge_several, :problems => [@problem1.id.to_s] - request.flash[:notice].should match(/You must select at least two/) + request.flash[:notice].should eql I18n.t('controllers.problems.flash.need_two_errors_merge') end it "should merge the problems" do - lambda { - post :merge_several, :problems => [@problem1.id.to_s, @problem2.id.to_s] - assigns(:merged_problem).reload.errs.length.should == 2 - }.should change(Problem, :count).by(-1) + ProblemMerge.should_receive(:new).and_return(double(:merge => true)) + post :merge_several, :problems => [@problem1.id.to_s, @problem2.id.to_s] end end context "POST /problems/unmerge_several" do + + it "should require at least one problem" do + post :unmerge_several, :problems => [] + request.flash[:notice].should eql I18n.t('controllers.problems.flash.no_select_problem') + end + it "should unmerge a merged problem" do merged_problem = Problem.merge!(@problem1, @problem2) merged_problem.errs.length.should == 2 @@ -374,9 +368,16 @@ describe ProblemsController do merged_problem.reload.errs.length.should == 1 }.should change(Problem, :count).by(1) end + end context "POST /problems/resolve_several" do + + it "should require at least one problem" do + post :resolve_several, :problems => [] + request.flash[:notice].should eql I18n.t('controllers.problems.flash.no_select_problem') + end + it "should resolve the issue" do post :resolve_several, :problems => [@problem2.id.to_s] @problem2.reload.resolved?.should == true @@ -390,10 +391,17 @@ describe ProblemsController do it "should display a message about 2 errs" do post :resolve_several, :problems => [@problem1.id.to_s, @problem2.id.to_s] flash[:success].should match(/2 errs have been resolved/) + controller.selected_problems.should == [@problem1, @problem2] end end context "POST /problems/unresolve_several" do + + it "should require at least one problem" do + post :unresolve_several, :problems => [] + request.flash[:notice].should eql I18n.t('controllers.problems.flash.no_select_problem') + end + it "should unresolve the issue" do post :unresolve_several, :problems => [@problem1.id.to_s] @problem1.reload.resolved?.should == false diff --git a/spec/models/problem_spec.rb b/spec/models/problem_spec.rb index cd483f6..1b6cbab 100644 --- a/spec/models/problem_spec.rb +++ b/spec/models/problem_spec.rb @@ -152,7 +152,6 @@ describe Problem do end end - context "Scopes" do context "resolved" do it 'only finds resolved Problems' do -- libgit2 0.21.2