From 1493304fc14df03efe72cdc70d56960c627da0cb Mon Sep 17 00:00:00 2001 From: Don Cruse Date: Thu, 18 Jul 2013 16:13:01 -0500 Subject: [PATCH 1/2] Refactor last year chart Pulled some instance variable assignment into the controller. Also extracted a method regarding interpolated values for the present month. --- app/controllers/stats_controller.rb | 38 +++++++++++-------- .../actions_done_lastyears_data.html.erb | 13 ++----- test/controllers/stats_controller_test.rb | 3 -- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 7d1dcdcd..660852da 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -33,45 +33,51 @@ class StatsController < ApplicationController find_running_avg_array(@actions_done_last12monthsPlus3_array, @actions_created_last12monthsPlus3_array, 13) # interpolate avg for current month. - percent_of_month = Time.zone.now.day.to_f / Time.zone.now.end_of_month.day.to_f - @interpolated_actions_created_this_month = interpolate_avg(@actions_created_last12months_array, percent_of_month) - @interpolated_actions_done_this_month = interpolate_avg(@actions_done_last12months_array, percent_of_month) + interpolate_avg_for_current_month(@actions_created_last12months_array, @actions_done_last12months_array) render :layout => false end + def interpolate_avg_for_current_month(created_set, done_set) + percent_of_month = Time.zone.now.day.to_f / Time.zone.now.end_of_month.day.to_f + @interpolated_actions_created_this_month = interpolate_avg(created_set, percent_of_month) + @interpolated_actions_done_this_month = interpolate_avg(done_set, percent_of_month) + end + def actions_done_last_years @page_title = t('stats.index_title') @chart = Stats::Chart.new('actions_done_lastyears_data', :height => 400, :width => 900) end def actions_done_lastyears_data - @actions_done_last_months = current_user.todos.completed.select("completed_at").reorder("completed_at DESC") - @actions_created_last_months = current_user.todos.select("created_at").reorder("created_at DESC" ) + actions_done_last_months = current_user.todos.completed.select("completed_at").reorder("completed_at DESC") + actions_created_last_months = current_user.todos.select("created_at").reorder("created_at DESC" ) # query is sorted, so use last todo to calculate number of months - @month_count = [difference_in_months(@today, @actions_created_last_months.last.created_at), - difference_in_months(@today, @actions_done_last_months.last.completed_at)].max + month_count = [difference_in_months(@today, actions_created_last_months.last.created_at), + difference_in_months(@today, actions_done_last_months.last.completed_at)].max # convert to array and fill in non-existing months - @actions_done_last_months_array = convert_to_months_from_today_array(@actions_done_last_months, @month_count+1, :completed_at) - @actions_created_last_months_array = convert_to_months_from_today_array(@actions_created_last_months, @month_count+1, :created_at) + @actions_done_last_months_array = convert_to_months_from_today_array(actions_done_last_months, month_count+1, :completed_at) + @actions_created_last_months_array = convert_to_months_from_today_array(actions_created_last_months, month_count+1, :created_at) # find max for graph in both hashes @max = [@actions_done_last_months_array.max, @actions_created_last_months_array.max].max # find running avg @actions_done_avg_last_months_array, @actions_created_avg_last_months_array = - find_running_avg_array(@actions_done_last_months_array, @actions_created_last_months_array, @month_count+1) + find_running_avg_array(@actions_done_last_months_array, @actions_created_last_months_array, month_count+1) # correct last two months since the data of last+1 and last+2 are not available for avg - correct_last_two_months(@actions_done_avg_last_months_array, @month_count) - correct_last_two_months(@actions_created_avg_last_months_array, @month_count) + correct_last_two_months(@actions_done_avg_last_months_array, month_count) + correct_last_two_months(@actions_created_avg_last_months_array, month_count) # interpolate avg for this month. - percent_of_month = Time.zone.now.day.to_f / Time.zone.now.end_of_month.day.to_f - @interpolated_actions_created_this_month = interpolate_avg(@actions_created_last_months_array, percent_of_month) - @interpolated_actions_done_this_month = interpolate_avg(@actions_done_last_months_array, percent_of_month) + interpolate_avg_for_current_month(@actions_created_last_months_array, @actions_done_last_months_array) + + @created_count_array = Array.new(month_count+1, actions_created_last_months.size/month_count) + @done_count_array = Array.new(month_count+1, actions_done_last_months.size/month_count) + @month_names = Array.new(month_count+1){ |i| t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ]+ " " + (Time.now - i.months).year.to_s} render :layout => false end @@ -449,7 +455,7 @@ class StatsController < ApplicationController end def interpolate_avg(set, percent) - return (set[0]*(1/percent) + set[1] + set[2]) / 3.0 + (set[0]*(1/percent) + set[1] + set[2]) / 3.0 end def correct_last_two_months(month_data, count) diff --git a/app/views/stats/actions_done_lastyears_data.html.erb b/app/views/stats/actions_done_lastyears_data.html.erb index a155bf64..aa6bc284 100644 --- a/app/views/stats/actions_done_lastyears_data.html.erb +++ b/app/views/stats/actions_done_lastyears_data.html.erb @@ -1,8 +1,3 @@ -<%- -created_count_array = Array.new(@month_count+1){ |i| @actions_created_last_months.size/@month_count } -done_count_array = Array.new(@month_count+1){ |i| @actions_done_last_months.size/@month_count } -month_names = Array.new(@month_count+1){ |i| t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ]+ " " + (Time.now - i.months).year.to_s} --%> &title=<%= t('stats.actions_last_year') %>,{font-size:16},& &y_legend=<%= t('stats.actions_last_year_legend.number_of_actions') %>,12,0x736AFF& &x_legend=<%= t('stats.actions_last_year_legend.months_ago') %>,12,0x736AFF& @@ -17,15 +12,15 @@ month_names = Array.new(@month_count+1){ |i| t('date.month_names')[ (Tim &line_8=1,0x007700& &values=<%= @actions_created_last_months_array.join(",")%>& &values_2=<%= @actions_done_last_months_array.join(",")%>& -&values_3=<%= created_count_array.join(",")%>& -&values_4=<%= done_count_array.join(",")%>& +&values_3=<%= @created_count_array.join(",")%>& +&values_4=<%= @done_count_array.join(",")%>& &values_5=<%= @actions_created_avg_last_months_array.join(",")%>& &values_6=<%= @actions_done_avg_last_months_array.join(",")%>& &values_7=<%= @interpolated_actions_created_this_month%>,<%=@actions_done_avg_last_months_array[1]%>& &values_8=<%= @interpolated_actions_done_this_month%>,<%=@actions_created_avg_last_months_array[1]%>& -&x_labels=<%= month_names.join(",")%>& +&x_labels=<%= @month_names.join(",")%>& &y_min=0& <% # add one to @max for people who have no actions completed yet. # OpenFlashChart cannot handle y_max=0 -%> &y_max=<%=@max+@max/10+1-%>& -&x_label_style=9,,2,& \ No newline at end of file +&x_label_style=9,,2,& diff --git a/test/controllers/stats_controller_test.rb b/test/controllers/stats_controller_test.rb index 87077ded..5d768a3b 100644 --- a/test/controllers/stats_controller_test.rb +++ b/test/controllers/stats_controller_test.rb @@ -172,9 +172,6 @@ class StatsControllerTest < ActionController::TestCase # only tests difference with actions_done_last_12months_data - # Then the count of months should be calculated - assert_equal 27, assigns['month_count'], "two years and three months of last todo" - # And the last two months are corrected assert_equal 2/3.0, assigns['actions_done_avg_last_months_array'][23] assert_equal 2/3.0, assigns['actions_done_avg_last_months_array'][24] From 70f633c15069f895b324506a2102467841850a42 Mon Sep 17 00:00:00 2001 From: Don Cruse Date: Thu, 18 Jul 2013 21:01:39 -0500 Subject: [PATCH 2/2] Shifting more instance variables to the controller Also removing some instance variables that were never invoked in the views. This sets up a future refactoring of the method of comptuting rolling averages, which differs between the two types of "last year" views being refactored. --- app/controllers/stats_controller.rb | 22 +++++++++++-------- .../actions_done_last12months_data.html.erb | 19 +++++++--------- test/controllers/stats_controller_test.rb | 6 ++--- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 660852da..008bc3bc 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -14,27 +14,31 @@ class StatsController < ApplicationController def actions_done_last12months_data # get actions created and completed in the past 12+3 months. +3 for running # average - @actions_done_last12months = current_user.todos.completed_after(@cut_off_year).select("completed_at" ) - @actions_created_last12months = current_user.todos.created_after(@cut_off_year).select("created_at") - @actions_done_last12monthsPlus3 = current_user.todos.completed_after(@cut_off_year_plus3).select("completed_at" ) - @actions_created_last12monthsPlus3 = current_user.todos.created_after(@cut_off_year_plus3).select("created_at") + actions_done_last12months = current_user.todos.completed_after(@cut_off_year).select("completed_at" ) + actions_created_last12months = current_user.todos.created_after(@cut_off_year).select("created_at") # convert to array and fill in non-existing months - @actions_done_last12months_array = convert_to_months_from_today_array(@actions_done_last12months, 13, :completed_at) - @actions_created_last12months_array = convert_to_months_from_today_array(@actions_created_last12months, 13, :created_at) - @actions_done_last12monthsPlus3_array = convert_to_months_from_today_array(@actions_done_last12monthsPlus3, 16, :completed_at) - @actions_created_last12monthsPlus3_array = convert_to_months_from_today_array(@actions_created_last12monthsPlus3, 16, :created_at) + @actions_done_last12months_array = convert_to_months_from_today_array(actions_done_last12months, 13, :completed_at) + @actions_created_last12months_array = convert_to_months_from_today_array(actions_created_last12months, 13, :created_at) # find max for graph in both arrays @max = [@actions_done_last12months_array.max, @actions_created_last12months_array.max].max # find running avg + actions_done_last12monthsPlus3 = current_user.todos.completed_after(@cut_off_year_plus3).select("completed_at" ) + actions_created_last12monthsPlus3 = current_user.todos.created_after(@cut_off_year_plus3).select("created_at") + actions_done_last12monthsPlus3_array = convert_to_months_from_today_array(actions_done_last12monthsPlus3, 16, :completed_at) + actions_created_last12monthsPlus3_array = convert_to_months_from_today_array(actions_created_last12monthsPlus3, 16, :created_at) + @actions_done_avg_last12months_array, @actions_created_avg_last12months_array = - find_running_avg_array(@actions_done_last12monthsPlus3_array, @actions_created_last12monthsPlus3_array, 13) + find_running_avg_array(actions_done_last12monthsPlus3_array, actions_created_last12monthsPlus3_array, 13) # interpolate avg for current month. interpolate_avg_for_current_month(@actions_created_last12months_array, @actions_done_last12months_array) + @created_count_array = Array.new(13, actions_created_last12months.size/12.0) + @done_count_array = Array.new(13, actions_done_last12months.size/12.0) + @month_names = Array.new(13){ |i| t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ]} render :layout => false end diff --git a/app/views/stats/actions_done_last12months_data.html.erb b/app/views/stats/actions_done_last12months_data.html.erb index 2fc2469b..294e0c69 100755 --- a/app/views/stats/actions_done_last12months_data.html.erb +++ b/app/views/stats/actions_done_last12months_data.html.erb @@ -1,8 +1,5 @@ -<%- -url_array = Array.new(13){ |i| url_for :controller => 'stats', :action => 'actions_done_last_years'} -created_count_array = Array.new(13){ |i| @actions_created_last12months.size/12.0 } -done_count_array = Array.new(13){ |i| @actions_done_last12months.size/12.0 } -month_names = Array.new(13){ |i| t('date.month_names')[ (Time.now.mon - i -1 ) % 12 + 1 ]} +<%- + url = url_for :controller => 'stats', :action => 'actions_done_last_years' -%> &title=<%= t('stats.actions_lastyear_title') %>,{font-size:16},& &y_legend=<%= t('stats.legend.number_of_actions') %>,12,0x736AFF& @@ -17,18 +14,18 @@ month_names = Array.new(13){ |i| t('date.month_names')[ (Time.now.mon - &line_7=1,0xAA0000& &line_8=1,0x007700& &values=<%= @actions_created_last12months_array.join(",")%>& -&links=<%= url_array.join(",")%>& -&links_2=<%= url_array.join(",")%>& +&links=<%= Array.new(13,url).join(",") %>& +&links_2=<%= Array.new(13,url).join(",") %>& &values_2=<%= @actions_done_last12months_array.join(",")%>& -&values_3=<%= created_count_array.join(",")%>& -&values_4=<%= done_count_array.join(",")%>& +&values_3=<%= @created_count_array.join(",")%>& +&values_4=<%= @done_count_array.join(",")%>& &values_5=<%= @actions_created_avg_last12months_array.join(",")%>& &values_6=<%= @actions_done_avg_last12months_array.join(",")%>& &values_7=<%= @interpolated_actions_created_this_month%>,<%=@actions_done_avg_last12months_array[1]%>& &values_8=<%= @interpolated_actions_done_this_month%>,<%=@actions_created_avg_last12months_array[1]%>& -&x_labels=<%= month_names.join(",")%>& +&x_labels=<%= @month_names.join(",")%>& &y_min=0& <% # add one to @max for people who have no actions completed yet. # OpenFlashChart cannot handle y_max=0 -%> &y_max=<%=@max+@max/10+1-%>& -&x_label_style=9,,2,& \ No newline at end of file +&x_label_style=9,,2,& diff --git a/test/controllers/stats_controller_test.rb b/test/controllers/stats_controller_test.rb index 5d768a3b..aacef8be 100644 --- a/test/controllers/stats_controller_test.rb +++ b/test/controllers/stats_controller_test.rb @@ -109,9 +109,9 @@ class StatsControllerTest < ActionController::TestCase assert_response :success # Then the todos for the chart should be retrieved - assert_not_nil assigns['actions_done_last12months'] - assert_not_nil assigns['actions_created_last12months'] - assert_equal 7, assigns['actions_created_last12months'].count, "very old todo should not be retrieved" + #assert_not_nil assigns['actions_done_last12months'] + #assert_not_nil assigns['actions_created_last12months'] + #assert_equal 7, assigns['actions_created_last12months'].count, "very old todo should not be retrieved" # And they should be totalled in a hash assert_equal 2, assigns['actions_created_last12months_array'][0], "there should be two todos in current month"