From 38907aea9b3e38718473e891ef3c3ca6c56dd592 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Thu, 16 Oct 2014 18:42:56 +0900 Subject: [PATCH 1/4] Make event-indicator a link to the events page, with new events highlighted. Currently it works as a reload button, but it gives you nothing but dismissing the indicator unless you are on the events page, and in any case you have no idea which events are newly created. With this change, clicking the indicator takes you to the events page on which newly created events are highlighted. --- .../components/worker-checker.js.coffee | 22 ++++++------ app/assets/stylesheets/tables.css.scss | 14 ++++++++ app/controllers/worker_status_controller.rb | 34 +++++++++++++++---- app/helpers/application_helper.rb | 27 +++++++++++++++ app/views/events/index.html.erb | 6 ++-- spec/helpers/application_helper_spec.rb | 32 +++++++++++++++++ 6 files changed, 115 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/components/worker-checker.js.coffee b/app/assets/javascripts/components/worker-checker.js.coffee index 88f31be0..c28fba40 100644 --- a/app/assets/javascripts/components/worker-checker.js.coffee +++ b/app/assets/javascripts/components/worker-checker.js.coffee @@ -1,10 +1,15 @@ $ -> - firstEventCount = null + sinceId = null previousJobs = null if $(".job-indicator").length check = -> - $.getJSON "/worker_status", (json) -> + query = + if sinceId? + '?since_id=' + sinceId + else + '' + $.getJSON "/worker_status" + query, (json) -> for method in ['pending', 'awaiting_retry', 'recent_failures'] count = json[method] elem = $(".job-indicator[role=#{method}]") @@ -23,16 +28,17 @@ $ -> if elem.is(":visible") elem.tooltip('destroy').fadeOut() - firstEventCount = json.event_count unless firstEventCount? - if firstEventCount? && json.event_count > firstEventCount + if sinceId? && json.event_count > 0 $("#event-indicator").tooltip('destroy'). - tooltip(title: "Click to reload", delay: 0, placement: "bottom", trigger: "hover"). + tooltip(title: "Click to see the events", delay: 0, placement: "bottom", trigger: "hover"). + find('a').attr(href: json.events_url).end(). fadeIn(). find(".number"). - text(json.event_count - firstEventCount) + text(json.event_count) else $("#event-indicator").tooltip('destroy').fadeOut() + sinceId ?= json.max_id currentJobs = [json.pending, json.awaiting_retry, json.recent_failures] if document.location.pathname == '/jobs' && $(".modal[aria-hidden=false]").length == 0 && previousJobs? && previousJobs.join(',') != currentJobs.join(',') $.get '/jobs', (data) => @@ -42,7 +48,3 @@ $ -> window.workerCheckTimeout = setTimeout check, 2000 check() - - $("#event-indicator a").on "click", (e) -> - e.preventDefault() - window.location.reload() \ No newline at end of file diff --git a/app/assets/stylesheets/tables.css.scss b/app/assets/stylesheets/tables.css.scss index 7a0c43f8..8b0371d6 100644 --- a/app/assets/stylesheets/tables.css.scss +++ b/app/assets/stylesheets/tables.css.scss @@ -20,6 +20,20 @@ } } +.table-striped > tbody > tr.hl { + &:nth-child(odd) { + > td, > th { + background-color: #ffeecc; + } + } + + &:nth-child(even) { + > td, > th { + background-color: #f9e8c6; + } + } +} + table.events { .payload { color: #999; diff --git a/app/controllers/worker_status_controller.rb b/app/controllers/worker_status_controller.rb index dda1251a..d84b1b26 100644 --- a/app/controllers/worker_status_controller.rb +++ b/app/controllers/worker_status_controller.rb @@ -1,12 +1,32 @@ class WorkerStatusController < ApplicationController def show - start = Time.now.to_f - render :json => { - :pending => Delayed::Job.where("run_at <= ? AND locked_at IS NULL AND attempts = 0", Time.now).count, - :awaiting_retry => Delayed::Job.where("failed_at IS NULL AND attempts > 0").count, - :recent_failures => Delayed::Job.where("failed_at IS NOT NULL AND failed_at > ?", 5.days.ago).count, - :event_count => current_user.events.count, - :compute_time => Time.now.to_f - start + start = Time.now + events = current_user.events + + if params[:since_id].present? + since_id = params[:since_id].to_i + events = events.where('id > ?', since_id) + end + + result = events.select('COUNT(id) AS count', 'MIN(id) AS min_id', 'MAX(id) AS max_id').first + count, min_id, max_id = result.count, result.min_id, result.max_id + + case max_id + when nil + when min_id + events_url = events_path(hl: max_id) + else + events_url = events_path(hl: "#{min_id}-#{max_id}") + end + + render json: { + pending: Delayed::Job.where("run_at <= ? AND locked_at IS NULL AND attempts = 0", start).count, + awaiting_retry: Delayed::Job.where("failed_at IS NULL AND attempts > 0").count, + recent_failures: Delayed::Job.where("failed_at IS NOT NULL AND failed_at > ?", 5.days.ago).count, + event_count: count, + max_id: max_id || 0, + events_url: events_url, + compute_time: Time.now - start } end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 7a78877b..33e46a5d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -71,4 +71,31 @@ module ApplicationHelper service_label_text(service) ].join.html_safe, class: "label label-default label-service service-#{service.provider}" end + + def highlighted?(id) + (@hl ||= + case hl = params[:hl].presence + when nil + ->x { false } + when String + values = hl.split(/,/).flat_map { |i| + case i + when /\A(\d+)\z/ + i.to_i + when /\A(\d+)?-(\d+)?\z/ + ($1 ? $1.to_i : 1)..($2 ? $2.to_i : (1/0.0)) + else + [] + end + } + ->x { + case x + when *values + true + else + false + end + } + end)[id] + end end diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index e6649f80..940d0eba 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -18,7 +18,7 @@ <% @events.each do |event| %> <% next unless event.agent %> - + <%= content_tag :tr, class: (highlighted?(event.id) ? 'hl' : nil) do %> <%= link_to event.agent.name, agent_path(event.agent) %> <%= time_ago_in_words event.created_at %> ago <%= truncate event.payload.to_json, :length => 90, :omission => "" %> @@ -29,12 +29,12 @@ <%= link_to 'Delete', event_path(event), method: :delete, data: { confirm: 'Are you sure?' }, class: "btn btn-default" %> - + <% end %> <% end %> - <%= paginate @events, :theme => 'twitter-bootstrap-3' %> + <%= paginate @events, params: params.slice(:hl), theme: 'twitter-bootstrap-3' %>
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index a06f9429..388d6da3 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -143,4 +143,36 @@ describe ApplicationHelper do expect(elem).to be_a Nokogiri::XML::Element end end + + describe '#highlighted?' do + it 'understands hl=6-8' do + stub(params).[](:hl) { '6-8' } + expect((1..10).select { |i| highlighted?(i) }).to eq [6, 7, 8] + end + + it 'understands hl=1,3-4,9' do + stub(params).[](:hl) { '1,3-4,9' } + expect((1..10).select { |i| highlighted?(i) }).to eq [1, 3, 4, 9] + end + + it 'understands hl=8-' do + stub(params).[](:hl) { '8-' } + expect((1..10).select { |i| highlighted?(i) }).to eq [8, 9, 10] + end + + it 'understands hl=-2' do + stub(params).[](:hl) { '-2' } + expect((1..10).select { |i| highlighted?(i) }).to eq [1, 2] + end + + it 'understands hl=-' do + stub(params).[](:hl) { '-' } + expect((1..10).select { |i| highlighted?(i) }).to eq [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + end + + it 'is OK with no hl' do + stub(params).[](:hl) { nil } + expect((1..10).select { |i| highlighted?(i) }).to be_empty + end + end end From 9294a2207ddea3bf91163f84bf5c49efde786536 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 21 Oct 2014 18:47:12 +0900 Subject: [PATCH 2/4] Use Float::INFINITY; We don't support Ruby 1.8. --- app/helpers/application_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 33e46a5d..8cdff609 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -83,7 +83,7 @@ module ApplicationHelper when /\A(\d+)\z/ i.to_i when /\A(\d+)?-(\d+)?\z/ - ($1 ? $1.to_i : 1)..($2 ? $2.to_i : (1/0.0)) + ($1 ? $1.to_i : 1)..($2 ? $2.to_i : Float::INFINITY) else [] end From 63f64f159727b50c999cf5d45f526740de47d47f Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 21 Oct 2014 18:51:16 +0900 Subject: [PATCH 3/4] Refactor `highlighted?` to avoid lambdas. --- app/helpers/application_helper.rb | 42 +++++++++++++------------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 8cdff609..93816c6c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -73,29 +73,23 @@ module ApplicationHelper end def highlighted?(id) - (@hl ||= - case hl = params[:hl].presence - when nil - ->x { false } - when String - values = hl.split(/,/).flat_map { |i| - case i - when /\A(\d+)\z/ - i.to_i - when /\A(\d+)?-(\d+)?\z/ - ($1 ? $1.to_i : 1)..($2 ? $2.to_i : Float::INFINITY) - else - [] - end - } - ->x { - case x - when *values - true - else - false - end - } - end)[id] + @highlighted_ranges ||= + case value = params[:hl].presence + when String + value.split(/,/).flat_map { |part| + case part + when /\A(\d+)\z/ + (part.to_i)..(part.to_i) + when /\A(\d+)?-(\d+)?\z/ + ($1 ? $1.to_i : 1)..($2 ? $2.to_i : Float::INFINITY) + else + [] + end + } + else + [] + end + + @highlighted_ranges.any? { |range| range.cover?(id) } end end From 89cc666212395bf277b4e160b32ad27a497d23e5 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Tue, 21 Oct 2014 19:15:54 +0900 Subject: [PATCH 4/4] Define some helper scopes in Delayed::Job for worker status API. --- app/controllers/worker_status_controller.rb | 6 +++--- config/initializers/delayed_job.rb | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/controllers/worker_status_controller.rb b/app/controllers/worker_status_controller.rb index d84b1b26..084f8366 100644 --- a/app/controllers/worker_status_controller.rb +++ b/app/controllers/worker_status_controller.rb @@ -20,9 +20,9 @@ class WorkerStatusController < ApplicationController end render json: { - pending: Delayed::Job.where("run_at <= ? AND locked_at IS NULL AND attempts = 0", start).count, - awaiting_retry: Delayed::Job.where("failed_at IS NULL AND attempts > 0").count, - recent_failures: Delayed::Job.where("failed_at IS NOT NULL AND failed_at > ?", 5.days.ago).count, + pending: Delayed::Job.pending.where("run_at <= ?", start).count, + awaiting_retry: Delayed::Job.awaiting_retry.count, + recent_failures: Delayed::Job.failed.where('failed_at > ?', 5.days.ago).count, event_count: count, max_id: max_id || 0, events_url: events_url, diff --git a/config/initializers/delayed_job.rb b/config/initializers/delayed_job.rb index c3c7e78d..cf2d29c1 100644 --- a/config/initializers/delayed_job.rb +++ b/config/initializers/delayed_job.rb @@ -7,3 +7,9 @@ Delayed::Worker.delay_jobs = !Rails.env.test? # Delayed::Worker.logger = Logger.new(Rails.root.join('log', 'delayed_job.log')) # Delayed::Worker.logger.level = Logger::DEBUG + +class Delayed::Job + scope :pending, ->{ where("locked_at IS NULL AND attempts = 0") } + scope :awaiting_retry, ->{ where("failed_at IS NULL AND attempts > 0") } + scope :failed, -> { where("failed_at IS NOT NULL") } +end