diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index ffbadbe..b68b4a9 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -1,4 +1,7 @@ class AppsController < InheritedResources::Base + + include ProblemsSearcher + before_filter :require_admin!, :except => [:index, :show] before_filter :parse_email_at_notices_or_set_default, :only => [:create, :update] before_filter :parse_notice_at_notices_or_set_default, :only => [:create, :update] @@ -9,16 +12,10 @@ class AppsController < InheritedResources::Base format.html do @all_errs = !!params[:all_errs] - @sort = params[:sort] - @order = params[:order] - @sort = "last_notice_at" unless %w{message app last_deploy_at last_notice_at count}.member?(@sort) - @order = "desc" unless %w{asc desc}.member?(@order) - @problems = resource.problems @problems = @problems.unresolved unless @all_errs - @problems = @problems.in_env(params[:environment]).ordered_by(@sort, @order).page(params[:page]).per(current_user.per_page) + @problems = @problems.in_env(params[:environment]).ordered_by(params_sort, params_order).page(params[:page]).per(current_user.per_page) - @selected_problems = params[:problems] || [] @deploys = @app.deploys.order_by(:created_at.desc).limit(5) end format.atom do diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index 6e71821..ae76d05 100644 --- a/app/controllers/problems_controller.rb +++ b/app/controllers/problems_controller.rb @@ -6,8 +6,8 @@ # COLLECTION => :index, :all, :destroy_several, :resolve_several, :unresolve_several, :merge_several, :unmerge_several, :search class ProblemsController < ApplicationController - before_filter :set_sorting_params, :only => [:index, :all, :search] - before_filter :set_tracker_params, :only => [:create_issue] + + include ProblemsSearcher before_filter :need_selected_problem, :only => [ :resolve_several, :unresolve_several, :unmerge_several @@ -25,25 +25,35 @@ class ProblemsController < ApplicationController app.problems.find(params[:id]) } - expose(:err_ids) { - (params[:problems] || []).compact + + expose(:all_errs) { + params[:all_errs] + } + + expose(:app_scope) { + apps = current_user.admin? ? App.all : current_user.apps + params[:app_id] ? apps.where(:_id => params[:app_id]) : apps } - expose(:selected_problems) { - Array(Problem.find(err_ids)) + expose(:params_environement) { + params[:environment] } - 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) - respond_to do |format| - format.html do - @problems = @problems.page(params[:page]).per(current_user.per_page) - end - format.atom + expose(:problems) { + pro = Problem.for_apps( + app_scope + ).in_env( + params_environement + ).all_else_unresolved(all_errs).ordered_by(params_sort, params_order) + + if request.format == :html + pro.page(params[:page]).per(current_user.per_page) + else + pro end - end + } + + def index; end def show @notices = problem.notices.reverse_ordered.page(params[:notice]).per(1) @@ -52,10 +62,11 @@ class ProblemsController < ApplicationController end def create_issue + IssueTracker.update_url_options(request) issue_creation = IssueCreation.new(problem, current_user, params[:tracker]) unless issue_creation.execute - flash[:error] = issue_creation.errors[:base].first + flash[:error] = issue_creation.errors.full_messages.join(', ') end redirect_to app_problem_path(app, problem) @@ -114,12 +125,7 @@ class ProblemsController < ApplicationController end def search - if params[:app_id] - app_scope = App.where(:_id => params[:app_id]) - else - app_scope = current_user.admin? ? App.all : current_user.apps - end - @problems = Problem.search(params[:search]).for_apps(app_scope).in_env(params[:environment]).all_else_unresolved(params[:all_errs]).ordered_by(@sort, @order) + @problems = Problem.search(params[:search]).for_apps(app_scope).in_env(params[:environment]).all_else_unresolved(params[:all_errs]).ordered_by(params_sort, params_order) @selected_problems = params[:problems] || [] @problems = @problems.page(params[:page]).per(current_user.per_page) render :content_type => 'text/javascript' @@ -127,18 +133,6 @@ class ProblemsController < ApplicationController protected - def set_tracker_params - IssueTracker.default_url_options[:host] = request.host - IssueTracker.default_url_options[:port] = request.port - IssueTracker.default_url_options[:protocol] = request.scheme - 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 # diff --git a/app/controllers/problems_searcher.rb b/app/controllers/problems_searcher.rb new file mode 100644 index 0000000..6f4ca0c --- /dev/null +++ b/app/controllers/problems_searcher.rb @@ -0,0 +1,33 @@ +# Include to do a Search +# TODO: Need to be in a Dedicated Object ProblemsSearch with params like input +# +module ProblemsSearcher + extend ActiveSupport::Concern + + included do + + expose(:params_sort) { + unless %w{app message last_notice_at last_deploy_at count}.member?(params[:sort]) + "last_notice_at" + else + params[:sort] + end + } + + expose(:params_order){ + unless %w{asc desc}.member?(params[:order]) + 'desc' + else + params[:order] + end + } + + expose(:selected_problems) { + Array(Problem.find(err_ids)) + } + + expose(:err_ids) { + (params[:problems] || []).compact + } + end +end diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb index 3c85b85..dec0a95 100644 --- a/app/helpers/sort_helper.rb +++ b/app/helpers/sort_helper.rb @@ -1,14 +1,14 @@ # encoding: utf-8 module SortHelper - + def link_for_sort(name, field=nil) field ||= name.underscore - current = (@sort == field) - order = (current && (@order == "asc")) ? "desc" : "asc" + current = (params_sort == field) + order = (current && (params_order == "asc")) ? "desc" : "asc" url = request.path + "?sort=#{field}&order=#{order}" options = {} options.merge!(:class => "current #{order}") if current link_to(name, url, options) end - + end diff --git a/app/interactors/issue_creation.rb b/app/interactors/issue_creation.rb index 8c074f8..ef0e70d 100644 --- a/app/interactors/issue_creation.rb +++ b/app/interactors/issue_creation.rb @@ -41,15 +41,11 @@ class IssueCreation end def execute - if tracker - begin - tracker.create_issue problem, user - rescue => ex - Rails.logger.error "Error during issue creation: " << ex.message - errors.add :base, "There was an error during issue creation: #{ex.message}" - end - end - + tracker.create_issue problem, user if tracker errors.empty? + rescue => ex + Rails.logger.error "Error during issue creation: " << ex.message + errors.add :base, "There was an error during issue creation: #{ex.message}" + false end end diff --git a/app/models/issue_tracker.rb b/app/models/issue_tracker.rb index ee25648..d1245b4 100644 --- a/app/models/issue_tracker.rb +++ b/app/models/issue_tracker.rb @@ -40,4 +40,15 @@ class IssueTracker def configured? project_id.present? end + + ## + # Update default_url_option with valid data from the request information + # + # @param [ Request ] a request with host, port and protocol + # + def self.update_url_options(request) + IssueTracker.default_url_options[:host] = request.host + IssueTracker.default_url_options[:port] = request.port + IssueTracker.default_url_options[:protocol] = request.scheme + end end diff --git a/app/views/problems/_table.html.haml b/app/views/problems/_table.html.haml index 3e0a0eb..58f7eee 100644 --- a/app/views/problems/_table.html.haml +++ b/app/views/problems/_table.html.haml @@ -16,7 +16,7 @@ - problems.each do |problem| %tr{:class => problem.resolved? ? 'resolved' : 'unresolved'} %td.select - = check_box_tag "problems[]", problem.id, @selected_problems.member?(problem.id.to_s) + = check_box_tag "problems[]", problem.id, selected_problems.member?(problem.id.to_s) %td.app = link_to problem.app.name, app_path(problem.app) - if current_page?(:controller => 'problems') diff --git a/app/views/problems/index.html.haml b/app/views/problems/index.html.haml index beed17b..d20402f 100644 --- a/app/views/problems/index.html.haml +++ b/app/views/problems/index.html.haml @@ -1,15 +1,17 @@ - content_for :title, @all_errs ? 'All Errors' : 'Unresolved Errors' - content_for :head do = auto_discovery_link_tag :atom, problems_path(User.token_authentication_key => current_user.authentication_token, :format => "atom"), :title => "Errbit notices at #{request.host}" + - content_for :action_bar do - if @all_errs = link_to 'hide resolved', problems_path, :class => 'button' - else = link_to 'show resolved', problems_path(:all_errs => true), :class => 'button' + %section = form_tag search_problems_path(:all_errs => @all_errs), :method => :get, :remote => true do = text_field_tag :search, params[:search], :placeholder => 'Search for issues' %br %section - .problem_table{:id => 'problem_table'} - = render 'problems/table', :problems => @problems + #problem_table.problem_table + = render 'problems/table' diff --git a/spec/controllers/problems_controller_spec.rb b/spec/controllers/problems_controller_spec.rb index b86a467..c142d0b 100644 --- a/spec/controllers/problems_controller_spec.rb +++ b/spec/controllers/problems_controller_spec.rb @@ -27,13 +27,13 @@ describe ProblemsController do it "should have default per_page value for user" do get :index - assigns(:problems).to_a.size.should == User::PER_PAGE + controller.problems.to_a.size.should == User::PER_PAGE end it "should be able to override default per_page value" do @user.update_attribute :per_page, 10 get :index - assigns(:problems).to_a.size.should == 10 + controller.problems.to_a.size.should == 10 end end @@ -48,35 +48,35 @@ describe ProblemsController do context 'no params' do it 'shows problems for all environments' do get :index - assigns(:problems).size.should == 21 + controller.problems.size.should == 21 end end context 'environment production' do it 'shows problems for just production' do get :index, :environment => 'production' - assigns(:problems).size.should == 6 + controller.problems.size.should == 6 end end context 'environment staging' do it 'shows problems for just staging' do get :index, :environment => 'staging' - assigns(:problems).size.should == 5 + controller.problems.size.should == 5 end end context 'environment development' do it 'shows problems for just development' do get :index, :environment => 'development' - assigns(:problems).size.should == 5 + controller.problems.size.should == 5 end end context 'environment test' do it 'shows problems for just test' do get :index, :environment => 'test' - assigns(:problems).size.should == 5 + controller.problems.size.should == 5 end end end @@ -89,8 +89,8 @@ describe ProblemsController do watched_unresolved_err = Fabricate(:err, :problem => Fabricate(:problem, :app => Fabricate(:user_watcher, :user => user).app, :resolved => false)) watched_resolved_err = Fabricate(:err, :problem => Fabricate(:problem, :app => Fabricate(:user_watcher, :user => user).app, :resolved => true)) get :index - assigns(:problems).should include(watched_unresolved_err.problem) - assigns(:problems).should_not include(unwatched_err.problem, watched_resolved_err.problem) + controller.problems.should include(watched_unresolved_err.problem) + controller.problems.should_not include(unwatched_err.problem, watched_resolved_err.problem) end end end @@ -106,7 +106,7 @@ describe ProblemsController do mock('proxy', :page => mock('other_proxy', :per => problems)) ) get :index, :all_errs => true - assigns(:problems).should == problems + controller.problems.should == problems end end @@ -117,8 +117,8 @@ describe ProblemsController do watched_unresolved_problem = Fabricate(:problem, :app => Fabricate(:user_watcher, :user => user).app, :resolved => false) watched_resolved_problem = Fabricate(:problem, :app => Fabricate(:user_watcher, :user => user).app, :resolved => true) get :index, :all_errs => true - assigns(:problems).should include(watched_resolved_problem, watched_unresolved_problem) - assigns(:problems).should_not include(unwatched_problem) + controller.problems.should include(watched_resolved_problem, watched_unresolved_problem) + controller.problems.should_not include(unwatched_problem) end end end diff --git a/spec/views/problems/index.html.haml_spec.rb b/spec/views/problems/index.html.haml_spec.rb new file mode 100644 index 0000000..cd1cf8a --- /dev/null +++ b/spec/views/problems/index.html.haml_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe "problems/index.html.haml" do + let(:problem_1) { Fabricate(:problem) } + let(:problem_2) { Fabricate(:problem, :app => problem_1.app) } + + before do + # view.stub(:app).and_return(problem.app) + view.stub(:selected_problems).and_return([]) + view.stub(:problems).and_return(Kaminari.paginate_array([problem_1, problem_2]).page(1).per(10)) + view.stub(:params_sort).and_return('asc') + controller.stub(:current_user) { Fabricate(:user) } + end + + describe "with problem" do + before { problem_1 && problem_2 } + + it 'should works' do + render + rendered.should have_selector('div#problem_table.problem_table') + end + end + +end + -- libgit2 0.21.2