From e549876493e453971c28cd048c333f0e56f59cb9 Mon Sep 17 00:00:00 2001 From: Bob Lail Date: Sun, 11 Sep 2011 10:40:43 -0500 Subject: [PATCH] add the ability to sort the errs table by its headers; --- app/controllers/apps_controller.rb | 6 +++++- app/controllers/errs_controller.rb | 8 +++++++- app/helpers/sort_helper.rb | 14 ++++++++++++++ app/models/app.rb | 7 ++++++- app/models/deploy.rb | 6 +++++- app/models/problem.rb | 33 ++++++++++++++++++++++++++++++++- app/views/errs/_table.html.haml | 12 ++++++------ spec/models/problem_spec.rb | 44 +++++++++++++++++++++++++++++++++++++++++++- 8 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 app/helpers/sort_helper.rb diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index 2ead924..e027f84 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -8,10 +8,14 @@ class AppsController < InheritedResources::Base respond_to do |format| format.html do @all_errs = !!params[:all_errs] + @sort = params[:sort] + @order = params[:order] + @sort = "app" unless %w{message last_notice_at last_deploy_at count}.member?(@sort) + @order = "asc" unless (@order == "desc") @problems = resource.problems @problems = @problems.unresolved unless @all_errs - @problems = @problems.in_env(params[:environment]).ordered.paginate(:page => params[:page], :per_page => current_user.per_page) + @problems = @problems.in_env(params[:environment]).ordered_by(@sort, @order).paginate(:page => params[:page], :per_page => current_user.per_page) @selected_problems = params[:problems] || [] @deploys = @app.deploys.order_by(:created_at.desc).limit(5) diff --git a/app/controllers/errs_controller.rb b/app/controllers/errs_controller.rb index bbaafe1..e081356 100644 --- a/app/controllers/errs_controller.rb +++ b/app/controllers/errs_controller.rb @@ -9,7 +9,13 @@ class ErrsController < ApplicationController def index app_scope = current_user.admin? ? App.all : current_user.apps - @problems = Problem.for_apps(app_scope).in_env(params[:environment]).unresolved.ordered + + @sort = params[:sort] + @order = params[:order] + @sort = "app" unless %w{message last_notice_at last_deploy_at count}.member?(@sort) + @order = "asc" unless (@order == "desc") + + @problems = Problem.for_apps(app_scope).in_env(params[:environment]).unresolved.ordered_by(@sort, @order) @selected_problems = params[:problems] || [] respond_to do |format| format.html do diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb new file mode 100644 index 0000000..3ebf0ea --- /dev/null +++ b/app/helpers/sort_helper.rb @@ -0,0 +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" + url = request.path + "?sort=#{field}&order=#{order}" + options = {} + options.merge(:class => "current") if current + link_to(name, url, options) + end + +end diff --git a/app/models/app.rb b/app/models/app.rb index 2f3964b..949f5d1 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -28,6 +28,7 @@ class App before_validation :generate_api_key, :on => :create before_save :normalize_github_url + after_update :store_cached_attributes_on_problems validates_presence_of :name, :api_key validates_uniqueness_of :name, :allow_blank => true @@ -148,7 +149,11 @@ class App end protected - + + def store_cached_attributes_on_problems + problems.each(&:cache_app_attributes) + end + def generate_api_key self.api_key ||= ActiveSupport::SecureRandom.hex end diff --git a/app/models/deploy.rb b/app/models/deploy.rb index 0d88d01..8814229 100644 --- a/app/models/deploy.rb +++ b/app/models/deploy.rb @@ -14,6 +14,7 @@ class Deploy after_create :deliver_notification, :if => :should_notify? after_create :resolve_app_errs, :if => :should_resolve_app_errs? + after_create :store_cached_attributes_on_problems validates_presence_of :username, :environment @@ -39,5 +40,8 @@ class Deploy app.resolve_errs_on_deploy? end + def store_cached_attributes_on_problems + Problem.where(:app_id => app.id).each(&:cache_app_attributes) + end + end - diff --git a/app/models/problem.rb b/app/models/problem.rb index ecd9282..17226d6 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -7,18 +7,24 @@ class Problem include Mongoid::Timestamps field :last_notice_at, :type => DateTime + field :last_deploy_at, :type => Time field :resolved, :type => Boolean, :default => false field :issue_link, :type => String # Cached fields + field :app_name, :type => String field :notices_count, :type => Integer, :default => 0 field :message field :environment field :klass field :where - index :last_notice_at index :app_id + index :app_name + index :message + index :last_notice_at + index :last_deploy_at + index :notices_count belongs_to :app has_many :errs, :inverse_of => :problem, :dependent => :destroy @@ -29,6 +35,10 @@ class Problem scope :ordered, order_by(:last_notice_at.desc) scope :for_apps, lambda {|apps| where(:app_id.in => apps.all.map(&:id))} + before_create :cache_app_attributes + + + def self.in_env(env) env.present? ? where(:environment => env) : scoped end @@ -67,6 +77,18 @@ class Problem merged_problem end + def self.ordered_by(sort, order) + case sort + when "app"; order_by(["app_name", order]) + when "message"; order_by(["message", order]) + when "last_notice_at"; order_by(["last_notice_at", order]) + when "last_deploy_at"; order_by(["last_deploy_at", order]) + when "count"; order_by(["notices_count", order]) + else raise("\"#{sort}\" is not a recognized sort") + end + end + + def merged? errs.length > 1 end @@ -85,9 +107,18 @@ class Problem def reset_cached_attributes update_attribute(:notices_count, notices.count) + cache_app_attributes cache_notice_attributes end + def cache_app_attributes + if app + self.app_name = app.name + self.last_deploy_at = app.last_deploy_at + self.save if persisted? + end + end + def cache_notice_attributes(notice=nil) notice ||= notices.first attrs = {:last_notice_at => notices.max(:created_at)} diff --git a/app/views/errs/_table.html.haml b/app/views/errs/_table.html.haml index 876fdb0..44fc89e 100644 --- a/app/views/errs/_table.html.haml +++ b/app/views/errs/_table.html.haml @@ -3,11 +3,11 @@ %thead %tr %th - %th App - %th What & Where - %th Latest - %th Deploy - %th Count + %th= link_for_sort "App" + %th= link_for_sort "What & Where".html_safe, "message" + %th= link_for_sort "Latest", "last_notice_at" + %th= link_for_sort "Deploy", "last_deploy_at" + %th= link_for_sort "Count" %th Resolve %tbody - errs.each do |problem| @@ -24,7 +24,7 @@ = link_to problem.message, app_err_path(problem.app, problem) %em= problem.where %td.latest #{time_ago_in_words(last_notice_at problem)} ago - %td.deploy= problem.app.last_deploy_at ? problem.app.last_deploy_at.to_s(:micro) : 'n/a' + %td.deploy= problem.last_deploy_at ? problem.last_deploy_at.to_s(:micro) : 'n/a' %td.count= link_to problem.notices.count, app_err_path(problem.app, problem) %td.resolve= link_to image_tag("thumbs-up.png"), resolve_app_err_path(problem.app, problem), :title => "Resolve", :method => :put, :confirm => err_confirm, :class => 'resolve' if problem.unresolved? - if errs.none? diff --git a/spec/models/problem_spec.rb b/spec/models/problem_spec.rb index afea25d..2600d0a 100644 --- a/spec/models/problem_spec.rb +++ b/spec/models/problem_spec.rb @@ -131,7 +131,6 @@ describe Problem do context "notice counter cache" do - before do @app = Factory(:app) @problem = Factory(:problem, :app => @app) @@ -158,4 +157,47 @@ describe Problem do end + context "#app_name" do + before do + @app = Factory(:app) + end + + it "is set when a problem is created" do + problem = Factory(:problem, :app => @app) + assert_equal @app.name, problem.app_name + end + + it "is updated when an app is updated" do + problem = Factory(:problem, :app => @app) + lambda { + @app.update_attributes!(:name => "Bar App") + problem.reload + }.should change(problem, :app_name).to("Bar App") + end + end + + + context "#last_deploy_at" do + before do + @app = Factory(:app) + @last_deploy = 10.days.ago.localtime.round(0) + deploy = Factory(:deploy, :app => @app, :created_at => @last_deploy) + end + + it "is set when a problem is created" do + problem = Factory(:problem, :app => @app) + assert_equal @last_deploy, problem.last_deploy_at + end + + it "is updated when a deploy is created" do + problem = Factory(:problem, :app => @app) + next_deploy = 5.minutes.ago.localtime.round(0) + lambda { + @deploy = Factory(:deploy, :app => @app, :created_at => next_deploy) + problem.reload + }.should change(problem, :last_deploy_at).from(@last_deploy).to(next_deploy) + end + end + + end -- libgit2 0.21.2