Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sparklines with ajax and rubocop clean #1315

Merged
merged 7 commits into from Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions app/assets/stylesheets/errbit.css.erb
Expand Up @@ -746,6 +746,31 @@ table.tally th.value {
text-transform: none;
}

/* Sparklines */

#sparkline-placeholder {
display: none;
}

.spark {
position: relative;
border: 1px solid #ddd;
float: left;
padding: 1px;
margin: 0;
height: 30px;
}

.spark > i {
display: inline-block;
position: relative;
width: 10px;
background: black;
margin: 0;
padding: 0;
vertical-align: bottom;
}

/* Resolve Errs */
#action-bar .resolve:before {
font-family: FontAwesome;
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/problems_controller.rb
@@ -1,3 +1,5 @@
require 'sparklines'

class ProblemsController < ApplicationController
include ProblemsSearcher

Expand Down Expand Up @@ -46,6 +48,10 @@ def show
@comment = Comment.new
end

def xhr_sparkline
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question. Should this action go in the "API"? Any opinions on that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like keeping this out of the API so folks don't confuse it for something that they might call from a script or something. it returns html, and it's built just for this rails ajaxy integration. i'm not recommending this necessarily, but one day, if the app were rewritten in reactjs, many of the existing controller actions would return json instead of html, and we could make this one fit that model as well.

render partial: 'problems/sparkline', layout: false
end

def close_issue
issue = Issue.new(problem: problem, user: current_user)
flash[:error] = issue.errors.full_messages.join(', ') unless issue.close
Expand Down
46 changes: 45 additions & 1 deletion app/models/problem.rb
Expand Up @@ -207,10 +207,54 @@ def unmerge!

# recache each new problem
new_problems.each(&:recache)

new_problems
end

def grouped_notice_counts(since, group_by = 'day')
key_op = [['year', '$year'], ['day', '$dayOfYear'], ['hour', '$hour']]
key_op = key_op.take(1 + key_op.find_index { |key, _op| group_by == key })
project_date_fields = Hash[*key_op.collect { |key, op| [key, { op => "$created_at" }] }.flatten]
group_id_fields = Hash[*key_op.collect { |key, _op| [key, "$#{key}"] }.flatten]
pipeline = [
{
"$match" => {
"err_id" => { '$in' => errs.map(&:id) },
"created_at" => { "$gt" => since }
}
},
{ "$project" => project_date_fields },
{ "$group" => { "_id" => group_id_fields, "count" => { "$sum" => 1 } } },
{ "$sort" => { "_id" => 1 } }
]
Notice.collection.aggregate(pipeline).find.to_a
end

def zero_filled_grouped_noticed_counts(since, group_by = 'day')
non_zero_filled = grouped_notice_counts(since, group_by)
buckets = group_by == 'day' ? 14 : 24

ruby_time_method = group_by == 'day' ? :yday : :hour
# rubocop:disable Performance/TimesMap
bucket_times = buckets.times.map { |ii| (since + ii.send(group_by)).send(ruby_time_method) }
# rubocop:enable Performance/TimesMap
bucket_times.to_a.map do |bucket_time|
count = if (data_for_day = non_zero_filled.detect { |item| item.dig('_id', group_by) == bucket_time })
data_for_day['count']
else
0
end
{ bucket_time => count }
end
end

def grouped_notice_count_relative_percentages(since, group_by = 'day')
zero_filled = zero_filled_grouped_noticed_counts(since, group_by).map { |h| h.values.first }
max = zero_filled.max
zero_filled.map do |number|
max.zero? ? 0 : number.to_f / max.to_f * 100.0
end
end

def self.ordered_by(sort, order)
case sort
when "app" then order_by(["app_name", order])
Expand Down
5 changes: 5 additions & 0 deletions app/views/notices/_summary.html.haml
Expand Up @@ -19,6 +19,7 @@
%tr
%th= t('.similar')
%td= problem.notices_count - 1
%tr#sparkline-placeholder
%tr
%th= t('.browser')
%td= user_agent_graph(problem)
Expand All @@ -41,3 +42,7 @@
%th= t('.relative_directory')
%td= notice.server_environment && notice.server_environment["project-root"]

:javascript
$.ajax({url: '/apps/#{notice.app.id}/problems/#{problem.id}/xhr_sparkline'}).then(function(resp) {
$("#sparkline-placeholder").replaceWith(resp);
});
9 changes: 9 additions & 0 deletions app/views/problems/_sparkline.html.haml
@@ -0,0 +1,9 @@
= now = Time.current
%tr
- two_week_percentages = problem.grouped_notice_count_relative_percentages((now - 13.days).at_beginning_of_day)
- twenty_four_hour_percentages = problem.grouped_notice_count_relative_percentages((now - 23.hours).at_beginning_of_hour)
%th= t('.graph-2-weeks')
%td= Sparklines.for_relative_percentages(two_week_percentages)
%tr
%th= t('.graph (24 hours)')
%td= Sparklines.for_relative_percentages(twenty_four_hour_percentages)
2 changes: 2 additions & 0 deletions config/locales/en.yml
Expand Up @@ -254,6 +254,8 @@ en:
similar: Similar
browser: Browser
tenant: Origin
graph-2-weeks: Graph (2 weeks)
graph-24-hours: Graph (24 hours)
app_server: App Server
app_version: App Version
framework: Framework
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Expand Up @@ -40,6 +40,7 @@
end

member do
get :xhr_sparkline
put :resolve
put :unresolve
post :create_issue
Expand Down
10 changes: 10 additions & 0 deletions lib/sparklines.rb
@@ -0,0 +1,10 @@
module Sparklines
class << self
def for_relative_percentages(array_of_relative_percentages)
bars = array_of_relative_percentages.map do |percent|
"<i style='height:#{percent}%'></i>"
end.join
"<div class='spark'>#{bars}</div>".html_safe
end
end
end
11 changes: 11 additions & 0 deletions spec/controllers/problems_controller_spec.rb
Expand Up @@ -173,6 +173,17 @@
end
end

describe "GET /apps/:app_id/problems/:id/xhr_sparkline" do
before do
sign_in user
end

it "renders without error" do
get :xhr_sparkline, app_id: app.id, id: err.problem.id
expect(response).to be_success
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're going to need something a little more robust in terms of testing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a bunch of tests for the underlying data in problems_spec.

end
end

describe "PUT /apps/:app_id/problems/:id/resolve" do
before do
sign_in user
Expand Down
16 changes: 16 additions & 0 deletions spec/lib/sparklines_spec.rb
@@ -0,0 +1,16 @@
describe Sparklines do
it 'includes each percentage and adds a percent sign' do
percentages = [33, 75, 100]
sparklines_html = Sparklines.for_relative_percentages(percentages)
percentages.each do |percentage|
expect(sparklines_html).to include("#{percentage}%")
end
end

it 'has the right number of i tags' do
percentages = [75, 100]
sparklines_html = Sparklines.for_relative_percentages(percentages)
number_of_i_tags = sparklines_html.scan(/<i/).size
expect(number_of_i_tags).to eq(2)
end
end
58 changes: 58 additions & 0 deletions spec/models/problem_spec.rb
@@ -1,3 +1,4 @@
# rubocop:disable Metrics/BlockLength
describe Problem, type: 'model' do
context 'validations' do
it 'requires an environment' do
Expand Down Expand Up @@ -215,6 +216,62 @@
end
end

context "sparklines-related methods" do
before do
@app = Fabricate(:app)
@problem = Fabricate(:problem, app: @app)
@err = Fabricate(:err, problem: @problem)
end

it "gets correct notice counts when grouping by day" do
now = Time.current
two_weeks_ago = 13.days.ago
Fabricate(:notice, err: @err, message: 'ERR 1')
Fabricate(:notice, err: @err, message: 'ERR 2', created_at: 3.days.ago)
Fabricate(:notice, err: @err, message: 'ERR 3', created_at: 3.days.ago)
three_days_ago_yday = (now - 3.days).yday
three_days_ago = @problem.grouped_notice_counts(two_weeks_ago, 'day').detect { |grouping| grouping['_id']['day'] == three_days_ago_yday }
expect(three_days_ago['count']).to eq(2)
count_by_day_for_last_two_weeks = @problem.zero_filled_grouped_noticed_counts(two_weeks_ago, 'day').map { |h| h.values.first }
expect(count_by_day_for_last_two_weeks.size).to eq(14)
expect(count_by_day_for_last_two_weeks).to eq([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 1])
end

it "gets correct notice counts when grouping by hour" do
twenty_four_hours_ago = 23.hours.ago
Fabricate(:notice, err: @err, message: 'ERR 1')
Fabricate(:notice, err: @err, message: 'ERR 2', created_at: 3.hours.ago)
Fabricate(:notice, err: @err, message: 'ERR 3', created_at: 3.hours.ago)
count_by_hour_for_last_24_hours = @problem.zero_filled_grouped_noticed_counts(twenty_four_hours_ago, 'hour').map { |h| h.values.first }
expect(count_by_hour_for_last_24_hours.size).to eq(24)
expect(count_by_hour_for_last_24_hours).to eq(([0] * 20) + [2, 0, 0, 1])
end

it "gets correct relative percentages when grouping by hour" do
two_weeks_ago = 13.days.ago
Fabricate(:notice, err: @err, message: 'ERR 1')
Fabricate(:notice, err: @err, message: 'ERR 2', created_at: 3.days.ago)
Fabricate(:notice, err: @err, message: 'ERR 3', created_at: 3.days.ago)
relative_percentages = @problem.grouped_notice_count_relative_percentages(two_weeks_ago, 'day')
expect(relative_percentages).to eq(([0] * 10) + [100, 0, 0, 50])
end

it "gets correct relative percentages when grouping by hour" do
twenty_four_hours_ago = 23.hours.ago
Fabricate(:notice, err: @err, message: 'ERR 1')
Fabricate(:notice, err: @err, message: 'ERR 2', created_at: 3.hours.ago)
Fabricate(:notice, err: @err, message: 'ERR 3', created_at: 3.hours.ago)
relative_percentages = @problem.grouped_notice_count_relative_percentages(twenty_four_hours_ago, 'hour')
expect(relative_percentages).to eq(([0] * 20) + [100, 0, 0, 50])
end

it "gets correct relative percentages when all zeros for data" do
two_weeks_ago = 13.days.ago
relative_percentages = @problem.grouped_notice_count_relative_percentages(two_weeks_ago, 'day')
expect(relative_percentages).to eq(([0] * 14))
end
end

context "#app_name" do
let!(:app) { Fabricate(:app) }
let!(:problem) { Fabricate(:problem, app: app) }
Expand Down Expand Up @@ -498,3 +555,4 @@
end
end
end
# rubocop:enable Metrics/BlockLength