From 8f6d57014a54cb1f92ac3badc2398dc755b07c38 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 21:08:48 -0500 Subject: [PATCH 01/13] Extract tag cloud class from stats controller --- app/controllers/stats_controller.rb | 56 ++++--------------------- app/models/stats/tag_cloud.rb | 65 +++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 48 deletions(-) create mode 100644 app/models/stats/tag_cloud.rb diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 4c05d78e..a0cc9b29 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -552,55 +552,15 @@ class StatsController < ApplicationController end def get_stats_tags - # tag cloud code inspired by this article - # http://www.juixe.com/techknow/index.php/2006/07/15/acts-as-taggable-tag-cloud/ + cloud = Stats::TagCloud.new(current_user, @cut_off_3months) + cloud.compute - levels=10 - # TODO: parameterize limit - - # Get the tag cloud for all tags for actions - query = "SELECT tags.id, name, count(*) AS count" - query << " FROM taggings, tags, todos" - query << " WHERE tags.id = tag_id" - query << " AND taggings.taggable_id = todos.id" - query << " AND todos.user_id="+current_user.id.to_s+" " - query << " AND taggings.taggable_type='Todo' " - query << " GROUP BY tags.id, tags.name" - query << " ORDER BY count DESC, name" - query << " LIMIT 100" - @tags_for_cloud = Tag.find_by_sql(query).sort_by { |tag| tag.name.downcase } - - max, @tags_min = 0, 0 - @tags_for_cloud.each { |t| - max = [t.count.to_i, max].max - @tags_min = [t.count.to_i, @tags_min].min - } - - @tags_divisor = ((max - @tags_min) / levels) + 1 - - # Get the tag cloud for all tags for actions - query = "SELECT tags.id, tags.name AS name, count(*) AS count" - query << " FROM taggings, tags, todos" - query << " WHERE tags.id = tag_id" - query << " AND todos.user_id=? " - query << " AND taggings.taggable_type='Todo' " - query << " AND taggings.taggable_id=todos.id " - query << " AND (todos.created_at > ? OR " - query << " todos.completed_at > ?) " - query << " GROUP BY tags.id, tags.name" - query << " ORDER BY count DESC, name" - query << " LIMIT 100" - @tags_for_cloud_90days = Tag.find_by_sql( - [query, current_user.id, @cut_off_3months, @cut_off_3months] - ).sort_by { |tag| tag.name.downcase } - - max_90days, @tags_min_90days = 0, 0 - @tags_for_cloud_90days.each { |t| - max_90days = [t.count.to_i, max_90days].max - @tags_min_90days = [t.count.to_i, @tags_min_90days].min - } - - @tags_divisor_90days = ((max_90days - @tags_min_90days) / levels) + 1 + @tags_for_cloud = cloud.tags_for_cloud + @tags_min = cloud.tags_min + @tags_divisor = cloud.tags_divisor + @tags_for_cloud_90days = cloud.tags_for_cloud_90days + @tags_min_90days = cloud.tags_min_90days + @tags_divisor_90days = cloud.tags_divisor_90days end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb new file mode 100644 index 00000000..78a49067 --- /dev/null +++ b/app/models/stats/tag_cloud.rb @@ -0,0 +1,65 @@ +# tag cloud code inspired by this article +# http://www.juixe.com/techknow/index.php/2006/07/15/acts-as-taggable-tag-cloud/ +module Stats + class TagCloud + + attr_reader :current_user, + :tags_for_cloud, :tags_min, :tags_divisor, + :tags_for_cloud_90days, :tags_min_90days, :tags_divisor_90days + def initialize(current_user, cut_off_3months) + @current_user = current_user + @cut_off_3months = cut_off_3months + end + + def compute + levels=10 + # TODO: parameterize limit + + # Get the tag cloud for all tags for actions + query = "SELECT tags.id, name, count(*) AS count" + query << " FROM taggings, tags, todos" + query << " WHERE tags.id = tag_id" + query << " AND taggings.taggable_id = todos.id" + query << " AND todos.user_id="+current_user.id.to_s+" " + query << " AND taggings.taggable_type='Todo' " + query << " GROUP BY tags.id, tags.name" + query << " ORDER BY count DESC, name" + query << " LIMIT 100" + @tags_for_cloud = Tag.find_by_sql(query).sort_by { |tag| tag.name.downcase } + + max, @tags_min = 0, 0 + @tags_for_cloud.each { |t| + max = [t.count.to_i, max].max + @tags_min = [t.count.to_i, @tags_min].min + } + + @tags_divisor = ((max - @tags_min) / levels) + 1 + + # Get the tag cloud for all tags for actions + query = "SELECT tags.id, tags.name AS name, count(*) AS count" + query << " FROM taggings, tags, todos" + query << " WHERE tags.id = tag_id" + query << " AND todos.user_id=? " + query << " AND taggings.taggable_type='Todo' " + query << " AND taggings.taggable_id=todos.id " + query << " AND (todos.created_at > ? OR " + query << " todos.completed_at > ?) " + query << " GROUP BY tags.id, tags.name" + query << " ORDER BY count DESC, name" + query << " LIMIT 100" + @tags_for_cloud_90days = Tag.find_by_sql( + [query, current_user.id, @cut_off_3months, @cut_off_3months] + ).sort_by { |tag| tag.name.downcase } + + max_90days, @tags_min_90days = 0, 0 + @tags_for_cloud_90days.each { |t| + max_90days = [t.count.to_i, max_90days].max + @tags_min_90days = [t.count.to_i, @tags_min_90days].min + } + + @tags_divisor_90days = ((max_90days - @tags_min_90days) / levels) + 1 + + end + + end +end From edc793e703b229cf237882336f8730bbbc5f2237 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 21:18:00 -0500 Subject: [PATCH 02/13] Rename variables The current user isn't necessarily current when it is in the model layer. The exposed attributes on the tag cloud no longer need to contain type information. --- app/controllers/stats_controller.rb | 12 +++++----- app/models/stats/tag_cloud.rb | 36 ++++++++++++++--------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index a0cc9b29..c1126105 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -555,12 +555,12 @@ class StatsController < ApplicationController cloud = Stats::TagCloud.new(current_user, @cut_off_3months) cloud.compute - @tags_for_cloud = cloud.tags_for_cloud - @tags_min = cloud.tags_min - @tags_divisor = cloud.tags_divisor - @tags_for_cloud_90days = cloud.tags_for_cloud_90days - @tags_min_90days = cloud.tags_min_90days - @tags_divisor_90days = cloud.tags_divisor_90days + @tags_for_cloud = cloud.tags + @tags_min = cloud.min + @tags_divisor = cloud.divisor + @tags_for_cloud_90days = cloud.tags_90days + @tags_min_90days = cloud.min_90days + @tags_divisor_90days = cloud.divisor_90days end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 78a49067..27e36f44 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -3,12 +3,12 @@ module Stats class TagCloud - attr_reader :current_user, - :tags_for_cloud, :tags_min, :tags_divisor, - :tags_for_cloud_90days, :tags_min_90days, :tags_divisor_90days - def initialize(current_user, cut_off_3months) - @current_user = current_user - @cut_off_3months = cut_off_3months + attr_reader :user, :cutoff, + :tags, :min, :divisor, + :tags_90days, :min_90days, :divisor_90days + def initialize(user, cutoff) + @user = user + @cutoff = cutoff end def compute @@ -20,20 +20,20 @@ module Stats query << " FROM taggings, tags, todos" query << " WHERE tags.id = tag_id" query << " AND taggings.taggable_id = todos.id" - query << " AND todos.user_id="+current_user.id.to_s+" " + query << " AND todos.user_id="+user.id.to_s+" " query << " AND taggings.taggable_type='Todo' " query << " GROUP BY tags.id, tags.name" query << " ORDER BY count DESC, name" query << " LIMIT 100" - @tags_for_cloud = Tag.find_by_sql(query).sort_by { |tag| tag.name.downcase } + @tags = Tag.find_by_sql(query).sort_by { |tag| tag.name.downcase } - max, @tags_min = 0, 0 - @tags_for_cloud.each { |t| + max, @min = 0, 0 + @tags.each { |t| max = [t.count.to_i, max].max - @tags_min = [t.count.to_i, @tags_min].min + @min = [t.count.to_i, @min].min } - @tags_divisor = ((max - @tags_min) / levels) + 1 + @divisor = ((max - @min) / levels) + 1 # Get the tag cloud for all tags for actions query = "SELECT tags.id, tags.name AS name, count(*) AS count" @@ -47,17 +47,17 @@ module Stats query << " GROUP BY tags.id, tags.name" query << " ORDER BY count DESC, name" query << " LIMIT 100" - @tags_for_cloud_90days = Tag.find_by_sql( - [query, current_user.id, @cut_off_3months, @cut_off_3months] + @tags_90days = Tag.find_by_sql( + [query, user.id, cutoff, cutoff] ).sort_by { |tag| tag.name.downcase } - max_90days, @tags_min_90days = 0, 0 - @tags_for_cloud_90days.each { |t| + max_90days, @min_90days = 0, 0 + @tags_90days.each { |t| max_90days = [t.count.to_i, max_90days].max - @tags_min_90days = [t.count.to_i, @tags_min_90days].min + @min_90days = [t.count.to_i, @min_90days].min } - @tags_divisor_90days = ((max_90days - @tags_min_90days) / levels) + 1 + @divisor_90days = ((max_90days - @min_90days) / levels) + 1 end From 447178bd7d8fdad5f5718a70665bf232dec66b31 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 21:30:36 -0500 Subject: [PATCH 03/13] Collapse duplication in tag cloud model --- app/controllers/stats_controller.rb | 11 +++--- app/models/stats/tag_cloud.rb | 55 +++++++++++------------------ 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index c1126105..b84c95e2 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -552,15 +552,18 @@ class StatsController < ApplicationController end def get_stats_tags - cloud = Stats::TagCloud.new(current_user, @cut_off_3months) + cloud = Stats::TagCloud.new(current_user) cloud.compute @tags_for_cloud = cloud.tags @tags_min = cloud.min @tags_divisor = cloud.divisor - @tags_for_cloud_90days = cloud.tags_90days - @tags_min_90days = cloud.min_90days - @tags_divisor_90days = cloud.divisor_90days + + cloud = Stats::TagCloud.new(current_user, @cut_off_3months) + cloud.compute + @tags_for_cloud_90days = cloud.tags + @tags_min_90days = cloud.min + @tags_divisor_90days = cloud.divisor end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 27e36f44..2b2e15bc 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -3,62 +3,47 @@ module Stats class TagCloud - attr_reader :user, :cutoff, - :tags, :min, :divisor, - :tags_90days, :min_90days, :divisor_90days - def initialize(user, cutoff) + attr_reader :user, :cutoff, :levels, + :tags, :min, :divisor + def initialize(user, cutoff = nil) @user = user @cutoff = cutoff + @levels = 10 end def compute - levels=10 - # TODO: parameterize limit - - # Get the tag cloud for all tags for actions - query = "SELECT tags.id, name, count(*) AS count" - query << " FROM taggings, tags, todos" - query << " WHERE tags.id = tag_id" - query << " AND taggings.taggable_id = todos.id" - query << " AND todos.user_id="+user.id.to_s+" " - query << " AND taggings.taggable_type='Todo' " - query << " GROUP BY tags.id, tags.name" - query << " ORDER BY count DESC, name" - query << " LIMIT 100" - @tags = Tag.find_by_sql(query).sort_by { |tag| tag.name.downcase } - + @tags = top_tags max, @min = 0, 0 @tags.each { |t| max = [t.count.to_i, max].max @min = [t.count.to_i, @min].min } - @divisor = ((max - @min) / levels) + 1 + end - # Get the tag cloud for all tags for actions + private + + def top_tags + params = [sql, user.id] + params += [cutoff, cutoff] if cutoff + Tag.find_by_sql(params).sort_by { |tag| tag.name.downcase } + end + + def sql + # TODO: parameterize limit query = "SELECT tags.id, tags.name AS name, count(*) AS count" query << " FROM taggings, tags, todos" query << " WHERE tags.id = tag_id" query << " AND todos.user_id=? " query << " AND taggings.taggable_type='Todo' " query << " AND taggings.taggable_id=todos.id " - query << " AND (todos.created_at > ? OR " - query << " todos.completed_at > ?) " + if cutoff + query << " AND (todos.created_at > ? OR " + query << " todos.completed_at > ?) " + end query << " GROUP BY tags.id, tags.name" query << " ORDER BY count DESC, name" query << " LIMIT 100" - @tags_90days = Tag.find_by_sql( - [query, user.id, cutoff, cutoff] - ).sort_by { |tag| tag.name.downcase } - - max_90days, @min_90days = 0, 0 - @tags_90days.each { |t| - max_90days = [t.count.to_i, max_90days].max - @min_90days = [t.count.to_i, @min_90days].min - } - - @divisor_90days = ((max_90days - @min_90days) / levels) + 1 - end end From 61e04a8258cba19a7cb9fde295f7f9c91a91168d Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 21:53:19 -0500 Subject: [PATCH 04/13] Extract methods in tag cloud This gets rid of the compute method, and makes each value that got set in it its own little method. --- app/controllers/stats_controller.rb | 2 -- app/models/stats/tag_cloud.rb | 43 ++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index b84c95e2..cdfdfb46 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -553,14 +553,12 @@ class StatsController < ApplicationController def get_stats_tags cloud = Stats::TagCloud.new(current_user) - cloud.compute @tags_for_cloud = cloud.tags @tags_min = cloud.min @tags_divisor = cloud.divisor cloud = Stats::TagCloud.new(current_user, @cut_off_3months) - cloud.compute @tags_for_cloud_90days = cloud.tags @tags_min_90days = cloud.min @tags_divisor_90days = cloud.divisor diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 2b2e15bc..1d9a1c42 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -3,30 +3,47 @@ module Stats class TagCloud - attr_reader :user, :cutoff, :levels, - :tags, :min, :divisor + attr_reader :user, :cutoff, :levels def initialize(user, cutoff = nil) @user = user @cutoff = cutoff @levels = 10 end - def compute - @tags = top_tags - max, @min = 0, 0 - @tags.each { |t| - max = [t.count.to_i, max].max - @min = [t.count.to_i, @min].min - } - @divisor = ((max - @min) / levels) + 1 + def tags + @tags ||= top_tags + end + + def max + @max ||= tag_counts.max + end + + # 2013-02-28: Possible bug. + # The original code always set the minimum to zero. + # This might need to use tag_counts.min + # https://github.com/TracksApp/tracks/commit/8c26ea7cb596c97e37213c0cc994e66ee5fd27b0#commitcomment-2719199 + def min + 0 + end + + def divisor + @divisor ||= ((max - min) / levels) + 1 end private + def tag_counts + @tag_counts ||= tags.map {|t| t.count.to_i} + end + def top_tags - params = [sql, user.id] - params += [cutoff, cutoff] if cutoff - Tag.find_by_sql(params).sort_by { |tag| tag.name.downcase } + Tag.find_by_sql(query_options).sort_by { |tag| tag.name.downcase } + end + + def query_options + options = [sql, user.id] + options += [cutoff, cutoff] if cutoff + options end def sql From b5868d5c704ff71c85d772ca9b61ff8ea2dc47f5 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 22:00:23 -0500 Subject: [PATCH 05/13] Use tag cloud objects in view This allows us to not set the individual instance variables for the tag cloud attributes. --- app/controllers/stats_controller.rb | 12 ++---------- app/views/stats/_tags.html.erb | 14 +++++++------- app/views/stats/index.html.erb | 4 ++-- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index cdfdfb46..f218c9b6 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -552,16 +552,8 @@ class StatsController < ApplicationController end def get_stats_tags - cloud = Stats::TagCloud.new(current_user) - - @tags_for_cloud = cloud.tags - @tags_min = cloud.min - @tags_divisor = cloud.divisor - - cloud = Stats::TagCloud.new(current_user, @cut_off_3months) - @tags_for_cloud_90days = cloud.tags - @tags_min_90days = cloud.min - @tags_divisor_90days = cloud.divisor + @tag_cloud = Stats::TagCloud.new(current_user) + @tag_cloud_90days = Stats::TagCloud.new(current_user, @cut_off_3months) end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/views/stats/_tags.html.erb b/app/views/stats/_tags.html.erb index e1f53f83..7025e118 100755 --- a/app/views/stats/_tags.html.erb +++ b/app/views/stats/_tags.html.erb @@ -3,12 +3,12 @@

<%= t('stats.tag_cloud_description') %>

- <% if @tags_for_cloud.size < 1 + <% if tag_cloud.tags.size < 1 t('stats.no_tags_available') else - @tags_for_cloud.each do |t| %> + tag_cloud.tags.each do |t| %> <%= link_to t.name, tag_path(t.name), { - :style => "font-size: " + (9 + 2*(t.count.to_i-@tags_min)/@tags_divisor).to_s + "pt", + :style => "font-size: " + (9 + 2*(t.count.to_i-tag_cloud.min)/tag_cloud.divisor).to_s + "pt", :title => t.count.to_s+" #{t('common.actions_midsentence', :count => t.count)}"} -%> <% end @@ -20,15 +20,15 @@

<%= t('stats.tag_cloud_90days_title') %>

<%= t('stats.tag_cloud_90days_description') %>

- <% if @tags_for_cloud_90days.size < 1 + <% if tag_cloud_90days.tags.size < 1 t('stats.no_tags_available') else - @tags_for_cloud_90days.each do |t| %> + tag_cloud_90days.tags.each do |t| %> <%= link_to t.name, tag_path(t.name), { - :style => "font-size: " + (9 + 2*(t.count.to_i-@tags_min_90days)/@tags_divisor_90days).to_s + "pt", + :style => "font-size: " + (9 + 2*(t.count.to_i-tag_cloud_90days.min)/tag_cloud_90days.divisor).to_s + "pt", :title => t.count.to_s+" #{t('common.actions_midsentence', :count => t.count)}"} -%> <% end end-%>

- \ No newline at end of file + diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index 7470731e..d54b0219 100755 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -15,7 +15,7 @@ <%= render :partial => 'projects' -%>

<%= t('stats.tags') %>

- <%= render :partial => 'tags' -%> + <%= render :partial => 'tags', :locals => {:tag_cloud => @tag_cloud, :tag_cloud_90days => @tag_cloud_90days} -%> <% else -%> @@ -23,4 +23,4 @@ <% end -%> - \ No newline at end of file + From 2321d1f2a3eb60e42ce57e87215fd9995f0b2c3f Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 22:03:39 -0500 Subject: [PATCH 06/13] Call the cloud partial twice Delete half the partial, and use each of the cloud objects to call it. --- app/views/stats/_tags.html.erb | 21 ++------------------- app/views/stats/index.html.erb | 3 ++- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/app/views/stats/_tags.html.erb b/app/views/stats/_tags.html.erb index 7025e118..df4c763b 100755 --- a/app/views/stats/_tags.html.erb +++ b/app/views/stats/_tags.html.erb @@ -1,6 +1,6 @@
-

<%= t('stats.tag_cloud_title') %>

-

<%= t('stats.tag_cloud_description') %>

+

<%= t("stats.tag_cloud#{key}_title") %>

+

<%= t("stats.tag_cloud#{key}_description") %>

<% if tag_cloud.tags.size < 1 @@ -15,20 +15,3 @@ end-%>

- -
-

<%= t('stats.tag_cloud_90days_title') %>

-

<%= t('stats.tag_cloud_90days_description') %>

-

- <% if tag_cloud_90days.tags.size < 1 - t('stats.no_tags_available') - else - tag_cloud_90days.tags.each do |t| %> - <%= link_to t.name, tag_path(t.name), { - :style => "font-size: " + (9 + 2*(t.count.to_i-tag_cloud_90days.min)/tag_cloud_90days.divisor).to_s + "pt", - :title => t.count.to_s+" #{t('common.actions_midsentence', :count => t.count)}"} - -%> <% - end - end-%> -

-
diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index d54b0219..0f44f2c3 100755 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -15,7 +15,8 @@ <%= render :partial => 'projects' -%>

<%= t('stats.tags') %>

- <%= render :partial => 'tags', :locals => {:tag_cloud => @tag_cloud, :tag_cloud_90days => @tag_cloud_90days} -%> + <%= render :partial => 'tags', :locals => {:tag_cloud => @tag_cloud, :key => ''} -%> + <%= render :partial => 'tags', :locals => {:tag_cloud => @tag_cloud_90days, :key => '_90days'} -%> <% else -%> From d478efdd011f395d89cc6ab8903fdbe853bc90ab Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 22:26:57 -0500 Subject: [PATCH 07/13] Move font calculation into tag cloud --- app/models/stats/tag_cloud.rb | 12 ++++++++++-- app/views/stats/_tags.html.erb | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 1d9a1c42..400400e4 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -10,10 +10,20 @@ module Stats @levels = 10 end + def empty? + tags.empty? + end + + def font_size(tag) + (9 + 2*(tag.count.to_i-min)/divisor) + end + def tags @tags ||= top_tags end + private + def max @max ||= tag_counts.max end @@ -30,8 +40,6 @@ module Stats @divisor ||= ((max - min) / levels) + 1 end - private - def tag_counts @tag_counts ||= tags.map {|t| t.count.to_i} end diff --git a/app/views/stats/_tags.html.erb b/app/views/stats/_tags.html.erb index df4c763b..e512c4e2 100755 --- a/app/views/stats/_tags.html.erb +++ b/app/views/stats/_tags.html.erb @@ -8,7 +8,7 @@ else tag_cloud.tags.each do |t| %> <%= link_to t.name, tag_path(t.name), { - :style => "font-size: " + (9 + 2*(t.count.to_i-tag_cloud.min)/tag_cloud.divisor).to_s + "pt", + :style => "font-size: " + "#{tag_cloud.font_size(t)}pt", :title => t.count.to_s+" #{t('common.actions_midsentence', :count => t.count)}"} -%> <% end From d577dd92017e3dce612a0c30a156bae5389062e6 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 22:32:24 -0500 Subject: [PATCH 08/13] Completely whimsical whitespace adjustment. --- app/views/stats/_tags.html.erb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/views/stats/_tags.html.erb b/app/views/stats/_tags.html.erb index e512c4e2..98140297 100755 --- a/app/views/stats/_tags.html.erb +++ b/app/views/stats/_tags.html.erb @@ -1,17 +1,19 @@

<%= t("stats.tag_cloud#{key}_title") %>

<%= t("stats.tag_cloud#{key}_description") %>

- -

- <% if tag_cloud.tags.size < 1 +

+<% + if tag_cloud.empty? t('stats.no_tags_available') - else - tag_cloud.tags.each do |t| %> - <%= link_to t.name, tag_path(t.name), { - :style => "font-size: " + "#{tag_cloud.font_size(t)}pt", - :title => t.count.to_s+" #{t('common.actions_midsentence', :count => t.count)}"} - -%> <% + else + tag_cloud.tags.each do |t| +%><%= + link_to t.name, tag_path(t.name), { + :style => "font-size: " + "#{tag_cloud.font_size(t)}pt", + :title => "#{t.count} #{t('common.actions_midsentence', :count => t.count)}"} +-%><% + end end - end-%> +-%>

From 8f42b25e688c0de2af57d1b4323909c37f1581aa Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Thu, 28 Feb 2013 23:36:08 -0500 Subject: [PATCH 09/13] Use lowest tag count as minimum --- app/models/stats/tag_cloud.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 400400e4..0fb68215 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -25,23 +25,19 @@ module Stats private def max - @max ||= tag_counts.max + @max ||= counts.max end - # 2013-02-28: Possible bug. - # The original code always set the minimum to zero. - # This might need to use tag_counts.min - # https://github.com/TracksApp/tracks/commit/8c26ea7cb596c97e37213c0cc994e66ee5fd27b0#commitcomment-2719199 def min - 0 + @min ||= counts.min end def divisor @divisor ||= ((max - min) / levels) + 1 end - def tag_counts - @tag_counts ||= tags.map {|t| t.count.to_i} + def counts + @counts ||= tags.map {|t| t.count.to_i} end def top_tags From a81f5b76f378830537d555b98cc7e855e061fe3a Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Fri, 1 Mar 2013 13:14:27 -0500 Subject: [PATCH 10/13] Separate query from cloud This will make testing easier --- app/controllers/stats_controller.rb | 7 ++++-- app/models/stats/tag_cloud.rb | 39 +++-------------------------- app/models/stats/tag_cloud_query.rb | 38 ++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 38 deletions(-) create mode 100644 app/models/stats/tag_cloud_query.rb diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index f218c9b6..b1b7372a 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -552,8 +552,11 @@ class StatsController < ApplicationController end def get_stats_tags - @tag_cloud = Stats::TagCloud.new(current_user) - @tag_cloud_90days = Stats::TagCloud.new(current_user, @cut_off_3months) + tags = Stats::TagCloudQuery.new(current_user).result + @tag_cloud = Stats::TagCloud.new(tags) + + tags_90days = Stats::TagCloudQuery.new(current_user, @cut_off_3months).result + @tag_cloud_90days = Stats::TagCloud.new(tags_90days) end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 0fb68215..6ea4d843 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -3,11 +3,10 @@ module Stats class TagCloud - attr_reader :user, :cutoff, :levels - def initialize(user, cutoff = nil) - @user = user - @cutoff = cutoff + attr_reader :levels, :tags + def initialize(tags) @levels = 10 + @tags = tags.sort_by { |tag| tag.name.downcase } end def empty? @@ -18,10 +17,6 @@ module Stats (9 + 2*(tag.count.to_i-min)/divisor) end - def tags - @tags ||= top_tags - end - private def max @@ -39,33 +34,5 @@ module Stats def counts @counts ||= tags.map {|t| t.count.to_i} end - - def top_tags - Tag.find_by_sql(query_options).sort_by { |tag| tag.name.downcase } - end - - def query_options - options = [sql, user.id] - options += [cutoff, cutoff] if cutoff - options - end - - def sql - # TODO: parameterize limit - query = "SELECT tags.id, tags.name AS name, count(*) AS count" - query << " FROM taggings, tags, todos" - query << " WHERE tags.id = tag_id" - query << " AND todos.user_id=? " - query << " AND taggings.taggable_type='Todo' " - query << " AND taggings.taggable_id=todos.id " - if cutoff - query << " AND (todos.created_at > ? OR " - query << " todos.completed_at > ?) " - end - query << " GROUP BY tags.id, tags.name" - query << " ORDER BY count DESC, name" - query << " LIMIT 100" - end - end end diff --git a/app/models/stats/tag_cloud_query.rb b/app/models/stats/tag_cloud_query.rb new file mode 100644 index 00000000..e754cc6c --- /dev/null +++ b/app/models/stats/tag_cloud_query.rb @@ -0,0 +1,38 @@ +module Stats + class TagCloudQuery + + attr_reader :user, :cutoff + def initialize(user, cutoff = nil) + @user = user + @cutoff = cutoff + end + + def result + Tag.find_by_sql(query_options) + end + + def query_options + options = [sql, user.id] + options += [cutoff, cutoff] if cutoff + options + end + + def sql + # TODO: parameterize limit + query = "SELECT tags.id, tags.name AS name, count(*) AS count" + query << " FROM taggings, tags, todos" + query << " WHERE tags.id = tag_id" + query << " AND todos.user_id=? " + query << " AND taggings.taggable_type='Todo' " + query << " AND taggings.taggable_id=todos.id " + if cutoff + query << " AND (todos.created_at > ? OR " + query << " todos.completed_at > ?) " + end + query << " GROUP BY tags.id, tags.name" + query << " ORDER BY count DESC, name" + query << " LIMIT 100" + end + + end +end From 63fb7a589c40a2588830ee32b5fafd866b073886 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Fri, 1 Mar 2013 13:26:59 -0500 Subject: [PATCH 11/13] Add test for tag cloud query --- test/unit/tag_cloud_query_test.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 test/unit/tag_cloud_query_test.rb diff --git a/test/unit/tag_cloud_query_test.rb b/test/unit/tag_cloud_query_test.rb new file mode 100644 index 00000000..e0dc252e --- /dev/null +++ b/test/unit/tag_cloud_query_test.rb @@ -0,0 +1,31 @@ +require File.expand_path(File.dirname(__FILE__) + '/../test_helper') + +class TagCloudQueryTest < ActiveSupport::TestCase + + fixtures :tags, :taggings, :users + + def user + @user ||= User.find 1 + end + + def test_get_all_tags + tags = Stats::TagCloudQuery.new(user).result + assert_equal 2, tags.size + tags.sort_by! {|t| t.id} + tag = tags.first + assert_equal 3, tag.count + assert_equal "foo", tag.name + + tag = tags.last + assert_equal 1, tag.count + assert_equal "bar", tag.name + end + + def test_get_subset_of_tags + tags = Stats::TagCloudQuery.new(user, 1.week.ago).result + + assert_equal 1, tags.size + assert_equal 2, tags.first.count + assert_equal "foo", tags.first.name + end +end From a13199cdda095b0a203ccfcaeac2537c20b18c26 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Fri, 1 Mar 2013 16:03:35 -0500 Subject: [PATCH 12/13] Add unit test for tag cloud --- app/controllers/stats_controller.rb | 4 ++-- app/models/stats/tag_cloud.rb | 2 +- test/minimal_test_helper.rb | 7 +++++++ test/test_helper.rb | 5 +---- test/unit/tag_cloud_test.rb | 23 +++++++++++++++++++++++ 5 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 test/minimal_test_helper.rb create mode 100644 test/unit/tag_cloud_test.rb diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index b1b7372a..8ea53558 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -555,8 +555,8 @@ class StatsController < ApplicationController tags = Stats::TagCloudQuery.new(current_user).result @tag_cloud = Stats::TagCloud.new(tags) - tags_90days = Stats::TagCloudQuery.new(current_user, @cut_off_3months).result - @tag_cloud_90days = Stats::TagCloud.new(tags_90days) + tags = Stats::TagCloudQuery.new(current_user, @cut_off_3months).result + @tag_cloud_90days = Stats::TagCloud.new(tags) end def get_ids_from (actions, week_from, week_to, at_end) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index 6ea4d843..c49048c4 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -14,7 +14,7 @@ module Stats end def font_size(tag) - (9 + 2*(tag.count.to_i-min)/divisor) + (9 + 2*(tag.count-min)/divisor) end private diff --git a/test/minimal_test_helper.rb b/test/minimal_test_helper.rb new file mode 100644 index 00000000..0c8ffc32 --- /dev/null +++ b/test/minimal_test_helper.rb @@ -0,0 +1,7 @@ +require 'simplecov' +SimpleCov.start 'rails' + +ENV["RAILS_ENV"] = "test" +require 'test/unit' + +$:.unshift File.dirname(File.dirname(__FILE__)) diff --git a/test/test_helper.rb b/test/test_helper.rb index 8779c868..ec92d0a4 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,7 +1,4 @@ -require 'simplecov' -SimpleCov.start 'rails' - -ENV["RAILS_ENV"] = "test" +require File.expand_path('../minimal_test_helper', __FILE__) require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' diff --git a/test/unit/tag_cloud_test.rb b/test/unit/tag_cloud_test.rb new file mode 100644 index 00000000..d40bcc51 --- /dev/null +++ b/test/unit/tag_cloud_test.rb @@ -0,0 +1,23 @@ +require File.expand_path(File.dirname(__FILE__) + '/../minimal_test_helper') +require 'app/models/stats/tag_cloud' + +class TagCloudTest < Test::Unit::TestCase + + FakeTag = Struct.new(:name, :count) + + def test_tags_get_sorted_alphabetically + tags = [FakeTag.new("bee", 1), FakeTag.new("See", 10), FakeTag.new("aye", 100)] + + assert_equal %w(aye bee See), Stats::TagCloud.new(tags).tags.map(&:name) + end + + def test_tag_font_size + tags = [FakeTag.new("bee", 1), FakeTag.new("See", 10), FakeTag.new("aye", 100)] + cloud = Stats::TagCloud.new(tags) + + assert_equal 9, cloud.font_size(FakeTag.new("whatever", 1)) + assert_equal 18, cloud.font_size(FakeTag.new("whatever", 50)) + assert_equal 28, cloud.font_size(FakeTag.new("whatever", 100)) + end + +end From cd7a5e08660281722c4d0d118e1250b03548934a Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Fri, 1 Mar 2013 16:12:00 -0500 Subject: [PATCH 13/13] Delete redundant casting in tag cloud --- app/models/stats/tag_cloud.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/stats/tag_cloud.rb b/app/models/stats/tag_cloud.rb index c49048c4..ef03c8fc 100644 --- a/app/models/stats/tag_cloud.rb +++ b/app/models/stats/tag_cloud.rb @@ -32,7 +32,7 @@ module Stats end def counts - @counts ||= tags.map {|t| t.count.to_i} + @counts ||= tags.map {|t| t.count} end end end