From 1ee30c0bb7914a092125e66722a77f6cb0e23789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 23 Apr 2024 23:12:30 +0200 Subject: [PATCH 01/20] Remove the create action from CommentableActions We were only using it in one place: the debates controller. All the other controllers including CommentableActions were overwriting this action, except the ones in the admin area, where creating proposals, debates or investments isn't possible. Note this means that, most of the times, we weren't tracking events creating a resource. Also note that since, as mentioned in commit 3752fef6b, there are no geozones in the debates form, we don't have to load them when creating a debate fails due to validation rules. --- app/controllers/concerns/commentable_actions.rb | 15 --------------- app/controllers/debates_controller.rb | 12 ++++++++++++ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/app/controllers/concerns/commentable_actions.rb b/app/controllers/concerns/commentable_actions.rb index a4bc77a4d..8a47f509e 100644 --- a/app/controllers/concerns/commentable_actions.rb +++ b/app/controllers/concerns/commentable_actions.rb @@ -44,21 +44,6 @@ module CommentableActions @resources = @search_terms.present? ? resource_relation.search(@search_terms) : nil end - def create - @resource = resource_model.new(strong_params) - @resource.author = current_user - - if @resource.save - track_event - redirect_path = url_for(controller: controller_name, action: :show, id: @resource.id) - redirect_to redirect_path, notice: t("flash.actions.create.#{resource_name.underscore}") - else - load_geozones - set_resource_instance - render :new - end - end - def edit end diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index 33c7fd899..e2b1a1efb 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -19,6 +19,18 @@ class DebatesController < ApplicationController helper_method :resource_model, :resource_name respond_to :html, :js + def create + @debate = Debate.new(debate_params) + @debate.author = current_user + + if @debate.save + track_event + redirect_to debate_path(@debate), notice: t("flash.actions.create.debate") + else + render :new + end + end + def index_customization @featured_debates = @debates.featured end From 1d11fa93d16ce0bfac79117fbef87ed4d90f304b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 23 Apr 2024 23:46:40 +0200 Subject: [PATCH 02/20] Remove unnecessary paddings in admin stats links These elements were not aligned with the rest of the page due to their paddings, caused by the `column` class. --- app/views/admin/stats/show.html.erb | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index f5fa82d3c..286ded1f0 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -110,20 +110,16 @@ -
- <% @event_types.each do |event| %> -

- <%= link_to graph_link_text(event), - graph_admin_stats_path(event: event) %> -

- <% end %> -
+ <% @event_types.each do |event| %> +

+ <%= link_to graph_link_text(event), + graph_admin_stats_path(event: event) %> +

+ <% end %> <% if feature?(:budgets) %> -
-

<%= t "admin.stats.show.budgets_title" %>

- <%= budget_investments_chart_tag id: "budget_investments" %> -
+

<%= t "admin.stats.show.budgets_title" %>

+ <%= budget_investments_chart_tag id: "budget_investments" %> <% end %> From 772be11525cb58b6dad8a3c9c335fbf185b48fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 16:49:05 +0200 Subject: [PATCH 03/20] Fix budget investments chart in admin stats The JavaScript required to display the chart wasn't loaded on the admin stats page. We're not adding a test because we're going to move the budgets graph to a different page on the pull request containing this commit. Note we're changing the "Go back" link, since using a turbolinks refresh broke this link when using the Chromium browser. Besides, there was an inconsistency where some of the "Go back" links in admin stats pointed to the admin stats page but other links pointed to `:back`. --- app/components/admin/stats/sdg_component.html.erb | 2 +- app/views/admin/stats/direct_messages.html.erb | 2 +- app/views/admin/stats/polls.html.erb | 2 +- app/views/admin/stats/proposal_notifications.html.erb | 2 +- app/views/admin/stats/show.html.erb | 4 ++++ spec/components/admin/stats/sdg_component_spec.rb | 11 +++++++++++ spec/system/admin/stats_spec.rb | 6 ++++++ 7 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 spec/components/admin/stats/sdg_component_spec.rb diff --git a/app/components/admin/stats/sdg_component.html.erb b/app/components/admin/stats/sdg_component.html.erb index 754380b4b..c8900eb52 100644 --- a/app/components/admin/stats/sdg_component.html.erb +++ b/app/components/admin/stats/sdg_component.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_stats_path %> <%= header %> diff --git a/app/views/admin/stats/direct_messages.html.erb b/app/views/admin/stats/direct_messages.html.erb index df5621a13..ff9ccbc8c 100644 --- a/app/views/admin/stats/direct_messages.html.erb +++ b/app/views/admin/stats/direct_messages.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_stats_path %>

<%= t("admin.stats.direct_messages.title") %>

diff --git a/app/views/admin/stats/polls.html.erb b/app/views/admin/stats/polls.html.erb index 178d06bcb..7c30c0093 100644 --- a/app/views/admin/stats/polls.html.erb +++ b/app/views/admin/stats/polls.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_stats_path %>

<%= t("admin.stats.polls.title") %>

diff --git a/app/views/admin/stats/proposal_notifications.html.erb b/app/views/admin/stats/proposal_notifications.html.erb index 4a5501177..4c66ca46e 100644 --- a/app/views/admin/stats/proposal_notifications.html.erb +++ b/app/views/admin/stats/proposal_notifications.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_stats_path %>

<%= t("admin.stats.proposal_notifications.title") %>

diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index 286ded1f0..c0c6d0706 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -1,3 +1,7 @@ +<% content_for :head do %> + <%= javascript_include_tag "stat_graphs", "data-turbolinks-track" => "reload" %> +<% end %> +
diff --git a/spec/components/admin/stats/sdg_component_spec.rb b/spec/components/admin/stats/sdg_component_spec.rb new file mode 100644 index 000000000..2f222b44f --- /dev/null +++ b/spec/components/admin/stats/sdg_component_spec.rb @@ -0,0 +1,11 @@ +require "rails_helper" + +describe Admin::Stats::SDGComponent do + include Rails.application.routes.url_helpers + + it "renders a links to go back to the admin stats index" do + render_inline Admin::Stats::SDGComponent.new(SDG::Goal.all) + + expect(page).to have_link "Go back", href: admin_stats_path + end +end diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb index fe6103b75..9ac12611c 100644 --- a/spec/system/admin/stats_spec.rb +++ b/spec/system/admin/stats_spec.rb @@ -183,6 +183,8 @@ describe "Stats", :admin do visit admin_stats_path click_link "Proposal notifications" + expect(page).to have_link "Go back", href: admin_stats_path + within("#proposal_notifications_count") do expect(page).to have_content "3" end @@ -232,6 +234,8 @@ describe "Stats", :admin do visit admin_stats_path click_link "Direct messages" + expect(page).to have_link "Go back", href: admin_stats_path + within("#direct_messages_count") do expect(page).to have_content "3" end @@ -253,6 +257,8 @@ describe "Stats", :admin do click_link "Polls" end + expect(page).to have_link "Go back", href: admin_stats_path + within("#web_participants") do expect(page).to have_content "3" end From 646dccca0ab26abd7a9808cfd4e567ad8f68ae80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 19:58:33 +0200 Subject: [PATCH 04/20] Rename event_types variable to event_names This is consistent with the `name` column in `ahoy_events`. --- app/controllers/admin/stats_controller.rb | 2 +- app/views/admin/stats/show.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb index bd06d30fd..3c8e05579 100644 --- a/app/controllers/admin/stats_controller.rb +++ b/app/controllers/admin/stats_controller.rb @@ -1,6 +1,6 @@ class Admin::StatsController < Admin::BaseController def show - @event_types = Ahoy::Event.distinct.order(:name).pluck(:name) + @event_names = Ahoy::Event.distinct.order(:name).pluck(:name) @visits = Visit.count @debates = Debate.with_hidden.count diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index c0c6d0706..fadda0c9c 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -114,7 +114,7 @@
- <% @event_types.each do |event| %> + <% @event_names.each do |event| %>

<%= link_to graph_link_text(event), graph_admin_stats_path(event: event) %> From 4e9ed4dfa6ddb8b7b0affc00a45d926391d913cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 23 Apr 2024 23:51:05 +0200 Subject: [PATCH 05/20] Extract component to render links to event stats --- .../admin/stats/event_links_component.html.erb | 6 ++++++ .../admin/stats/event_links_component.rb | 17 +++++++++++++++++ app/helpers/stats_helper.rb | 8 -------- app/views/admin/stats/show.html.erb | 7 +------ 4 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 app/components/admin/stats/event_links_component.html.erb create mode 100644 app/components/admin/stats/event_links_component.rb diff --git a/app/components/admin/stats/event_links_component.html.erb b/app/components/admin/stats/event_links_component.html.erb new file mode 100644 index 000000000..e63a3624a --- /dev/null +++ b/app/components/admin/stats/event_links_component.html.erb @@ -0,0 +1,6 @@ +<% event_names.each do |event| %> +

+ <%= link_to link_text(event), + graph_admin_stats_path(event: event) %> +

+<% end %> diff --git a/app/components/admin/stats/event_links_component.rb b/app/components/admin/stats/event_links_component.rb new file mode 100644 index 000000000..71d3121b9 --- /dev/null +++ b/app/components/admin/stats/event_links_component.rb @@ -0,0 +1,17 @@ +class Admin::Stats::EventLinksComponent < ApplicationComponent + attr_reader :event_names + + def initialize(event_names) + @event_names = event_names + end + + private + + def link_text(event) + text = t("admin.stats.graph.#{event}") + if text.to_s.match(/translation missing/) + text = event + end + text + end +end diff --git a/app/helpers/stats_helper.rb b/app/helpers/stats_helper.rb index acc79319b..711804d5f 100644 --- a/app/helpers/stats_helper.rb +++ b/app/helpers/stats_helper.rb @@ -15,14 +15,6 @@ module StatsHelper data end - def graph_link_text(event) - text = t("admin.stats.graph.#{event}") - if text.to_s.match(/translation missing/) - text = event - end - text - end - def budget_investments_chart_tag(opt = {}) opt[:data] ||= {} opt[:data][:graph] = admin_api_stats_path(budget_investments: true) diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index fadda0c9c..0e8594da9 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -114,12 +114,7 @@

- <% @event_names.each do |event| %> -

- <%= link_to graph_link_text(event), - graph_admin_stats_path(event: event) %> -

- <% end %> + <%= render Admin::Stats::EventLinksComponent.new(@event_names) %> <% if feature?(:budgets) %>

<%= t "admin.stats.show.budgets_title" %>

From b04ac817f668bcecaf496dcfe1c0e3d4b9f872ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 23 Apr 2024 23:57:44 +0200 Subject: [PATCH 06/20] Use a list of links for admin stats events Using

headings for the links had two disadvantages. First, it was the wrong heading level to use, since there was no

tag before it. Second, headings are supposed to be followed by content associated to that heading; here, we had no content following the headings. So we're using a list of links and giving it a heading. We're adding styles so the page still looks like it used to, although these styles are certainly asking for improvements. --- .../stylesheets/admin/stats/event_links.scss | 14 ++++++++++++++ .../admin/stats/event_links_component.html.erb | 10 ++++------ .../admin/stats/event_links_component.rb | 11 +++++++++++ config/locales/en/admin.yml | 1 + config/locales/es/admin.yml | 1 + .../admin/stats/event_links_component_spec.rb | 17 +++++++++++++++++ 6 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 app/assets/stylesheets/admin/stats/event_links.scss create mode 100644 spec/components/admin/stats/event_links_component_spec.rb diff --git a/app/assets/stylesheets/admin/stats/event_links.scss b/app/assets/stylesheets/admin/stats/event_links.scss new file mode 100644 index 000000000..5375edc41 --- /dev/null +++ b/app/assets/stylesheets/admin/stats/event_links.scss @@ -0,0 +1,14 @@ +.stats-event-links { + margin-top: $line-height; + + ul { + @include header-font-size(h3); + font-weight: bold; + list-style-type: none; + margin-#{$global-left}: 0; + + * + * { + margin-top: $line-height; + } + } +} diff --git a/app/components/admin/stats/event_links_component.html.erb b/app/components/admin/stats/event_links_component.html.erb index e63a3624a..7a1181571 100644 --- a/app/components/admin/stats/event_links_component.html.erb +++ b/app/components/admin/stats/event_links_component.html.erb @@ -1,6 +1,4 @@ -<% event_names.each do |event| %> -

- <%= link_to link_text(event), - graph_admin_stats_path(event: event) %> -

-<% end %> +
+

<%= title %>

+ <%= link_list(*links) %> +
diff --git a/app/components/admin/stats/event_links_component.rb b/app/components/admin/stats/event_links_component.rb index 71d3121b9..9fcad74eb 100644 --- a/app/components/admin/stats/event_links_component.rb +++ b/app/components/admin/stats/event_links_component.rb @@ -1,5 +1,6 @@ class Admin::Stats::EventLinksComponent < ApplicationComponent attr_reader :event_names + use_helpers :link_list def initialize(event_names) @event_names = event_names @@ -14,4 +15,14 @@ class Admin::Stats::EventLinksComponent < ApplicationComponent end text end + + def title + t("admin.stats.graph.title") + end + + def links + event_names.map do |event| + [link_text(event), graph_admin_stats_path(event: event)] + end + end end diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index c1b8d651e..727be148e 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1492,6 +1492,7 @@ en: visit: Visits level_2_user: Level 2 users proposal_created: Citizen proposals + title: Graphs budgets: no_data_before_balloting_phase: "There isn't any data to show before the balloting phase." title: "Participatory Budgets - Participation stats" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index e0b9ce808..c562e63a3 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1492,6 +1492,7 @@ es: visit: Visitas level_2_user: Usuarios nivel 2 proposal_created: Propuestas Ciudadanas + title: Gráficos budgets: no_data_before_balloting_phase: "No hay datos que mostrar hasta que la fase de votación no esté abierta." title: "Presupuestos participativos - Estadisticas de participación" diff --git a/spec/components/admin/stats/event_links_component_spec.rb b/spec/components/admin/stats/event_links_component_spec.rb new file mode 100644 index 000000000..5c5728a74 --- /dev/null +++ b/spec/components/admin/stats/event_links_component_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +describe Admin::Stats::EventLinksComponent do + it "renders an

heading followed by a list of links" do + render_inline Admin::Stats::EventLinksComponent.new( + %w[legislation_annotation_created legislation_answer_created] + ) + + expect(page).to have_css "h2", exact_text: "Graphs" + expect(page).to have_link count: 2 + + page.find("ul") do |list| + expect(list).to have_link "legislation_annotation_created" + expect(list).to have_link "legislation_answer_created" + end + end +end From 1edf0d937d68c7295099663d73b25537964b919d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 01:45:48 +0200 Subject: [PATCH 07/20] Move stats graph partial to a component Note we're naming it "chart" in order to avoid possible conflicts with the "graph" view when we move it to a component. --- .../budget_supporting_component.html.erb | 2 +- .../admin/stats/chart_component.html.erb} | 0 app/components/admin/stats/chart_component.rb | 27 +++++++++++++++++++ app/helpers/stats_helper.rb | 16 ----------- app/views/admin/stats/graph.html.erb | 2 +- 5 files changed, 29 insertions(+), 18 deletions(-) rename app/{views/admin/stats/_graph.html.erb => components/admin/stats/chart_component.html.erb} (100%) create mode 100644 app/components/admin/stats/chart_component.rb diff --git a/app/components/admin/stats/budget_supporting_component.html.erb b/app/components/admin/stats/budget_supporting_component.html.erb index 2d74625d9..c946d73bf 100644 --- a/app/components/admin/stats/budget_supporting_component.html.erb +++ b/app/components/admin/stats/budget_supporting_component.html.erb @@ -30,7 +30,7 @@

-<%= render "admin/stats/graph", name: "user_supported_budgets", event: "", count: user_count %> +<%= render Admin::Stats::ChartComponent.new(name: "user_supported_budgets", event: "", count: user_count) %> diff --git a/app/views/admin/stats/_graph.html.erb b/app/components/admin/stats/chart_component.html.erb similarity index 100% rename from app/views/admin/stats/_graph.html.erb rename to app/components/admin/stats/chart_component.html.erb diff --git a/app/components/admin/stats/chart_component.rb b/app/components/admin/stats/chart_component.rb new file mode 100644 index 000000000..ac94fa4cf --- /dev/null +++ b/app/components/admin/stats/chart_component.rb @@ -0,0 +1,27 @@ +class Admin::Stats::ChartComponent < ApplicationComponent + attr_reader :name, :event, :count + + def initialize(name:, event:, count:) + @name = name + @event = event + @count = count + end + + private + + def chart_tag(opt = {}) + opt[:data] ||= {} + opt[:data][:graph] = admin_api_stats_path(chart_data(opt)) + tag.div(**opt) + end + + def chart_data(opt = {}) + data = nil + if opt[:id].present? + data = { opt[:id] => true } + elsif opt[:event].present? + data = { event: opt[:event] } + end + data + end +end diff --git a/app/helpers/stats_helper.rb b/app/helpers/stats_helper.rb index 711804d5f..30facd211 100644 --- a/app/helpers/stats_helper.rb +++ b/app/helpers/stats_helper.rb @@ -1,20 +1,4 @@ module StatsHelper - def chart_tag(opt = {}) - opt[:data] ||= {} - opt[:data][:graph] = admin_api_stats_path(chart_data(opt)) - tag.div(**opt) - end - - def chart_data(opt = {}) - data = nil - if opt[:id].present? - data = { opt[:id] => true } - elsif opt[:event].present? - data = { event: opt[:event] } - end - data - end - def budget_investments_chart_tag(opt = {}) opt[:data] ||= {} opt[:data][:graph] = admin_api_stats_path(budget_investments: true) diff --git a/app/views/admin/stats/graph.html.erb b/app/views/admin/stats/graph.html.erb index 04435660c..85c8395de 100644 --- a/app/views/admin/stats/graph.html.erb +++ b/app/views/admin/stats/graph.html.erb @@ -4,4 +4,4 @@ <%= back_link_to admin_stats_path %> -<%= render "graph", name: @name, event: @event, count: @count %> +<%= render Admin::Stats::ChartComponent.new(name: @name, event: @event, count: @count) %> From 4e72cbe42e7a66dec136b3a873520c0365c2fe82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 01:27:16 +0200 Subject: [PATCH 08/20] Simplify method to get chart data --- app/components/admin/stats/chart_component.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/components/admin/stats/chart_component.rb b/app/components/admin/stats/chart_component.rb index ac94fa4cf..2528e1e49 100644 --- a/app/components/admin/stats/chart_component.rb +++ b/app/components/admin/stats/chart_component.rb @@ -16,12 +16,10 @@ class Admin::Stats::ChartComponent < ApplicationComponent end def chart_data(opt = {}) - data = nil if opt[:id].present? - data = { opt[:id] => true } + { opt[:id] => true } elsif opt[:event].present? - data = { event: opt[:event] } + { event: opt[:event] } end - data end end From 0b2975194b80ea23dbfdc260d02d499c58c21d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 00:53:25 +0200 Subject: [PATCH 09/20] Extract model to handle chart data We're adding it inside the Ahoy module because the Ahoy::DataSource class is also in that module. We might move it to a different place in the future. --- app/controllers/admin/api/stats_controller.rb | 2 +- app/controllers/admin/stats_controller.rb | 4 ++-- app/models/ahoy/chart.rb | 20 +++++++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 app/models/ahoy/chart.rb diff --git a/app/controllers/admin/api/stats_controller.rb b/app/controllers/admin/api/stats_controller.rb index c86badceb..b025e3195 100644 --- a/app/controllers/admin/api/stats_controller.rb +++ b/app/controllers/admin/api/stats_controller.rb @@ -10,7 +10,7 @@ class Admin::Api::StatsController < Admin::Api::BaseController ds = Ahoy::DataSource.new if params[:event].present? - ds.add params[:event].titleize, Ahoy::Event.where(name: params[:event]).group_by_day(:time).count + ds.add params[:event].titleize, Ahoy::Chart.new(params[:event]).group_by_day(:time).count end if params[:visits].present? diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb index 3c8e05579..fbfc06618 100644 --- a/app/controllers/admin/stats_controller.rb +++ b/app/controllers/admin/stats_controller.rb @@ -1,6 +1,6 @@ class Admin::StatsController < Admin::BaseController def show - @event_names = Ahoy::Event.distinct.order(:name).pluck(:name) + @event_names = Ahoy::Chart.active_event_names @visits = Visit.count @debates = Debate.with_hidden.count @@ -34,7 +34,7 @@ class Admin::StatsController < Admin::BaseController @event = params[:event] if params[:event] - @count = Ahoy::Event.where(name: params[:event]).count + @count = Ahoy::Chart.new(params[:event]).count else @count = params[:count] end diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb new file mode 100644 index 000000000..fa71186e7 --- /dev/null +++ b/app/models/ahoy/chart.rb @@ -0,0 +1,20 @@ +module Ahoy + class Chart + attr_reader :event_name + delegate :count, :group_by_day, to: :events + + def initialize(event_name) + @event_name = event_name + end + + def self.active_event_names + Ahoy::Event.distinct.order(:name).pluck(:name) + end + + private + + def events + Ahoy::Event.where(name: event_name) + end + end +end From 448775a5e942f5d55b25493777a4b0174d7c6cc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2024 02:26:03 +0200 Subject: [PATCH 10/20] Remove unused Campaign model The only way to use campaigns is to manually insert them in the database, which IMHO isn't very practical. We're going to change every piece of code that generates an Ahoy event and, in this case, the easiest way to change the Campaing model so it doesn't use Ahoy events is to simply remove it. Note we're keeping the database tables until we release a new version, just in case some Consul Democracy installations are using them. We'll inform in the release notes that we'll remove the campaigns table after the release, so existing installations using the `campaigns` table can move the data somewhere else before we remove the table. --- app/controllers/application_controller.rb | 8 ----- app/models/campaign.rb | 2 -- spec/factories/analytics.rb | 5 --- spec/system/admin/stats_spec.rb | 14 +++++---- spec/system/campaigns_spec.rb | 37 ----------------------- 5 files changed, 8 insertions(+), 58 deletions(-) delete mode 100644 app/models/campaign.rb delete mode 100644 spec/system/campaigns_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 76092d526..082c74754 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,7 +11,6 @@ class ApplicationController < ActionController::Base before_action :ensure_signup_complete around_action :switch_locale - before_action :track_email_campaign before_action :set_return_url check_authorization unless: :devise_controller? @@ -91,13 +90,6 @@ class ApplicationController < ActionController::Base end end - def track_email_campaign - if params[:track_id] - campaign = Campaign.find_by(track_id: params[:track_id]) - ahoy.track campaign.name if campaign.present? - end - end - def set_return_url if request.get? && !devise_controller? && is_navigational_format? store_location_for(:user, request.fullpath) diff --git a/app/models/campaign.rb b/app/models/campaign.rb deleted file mode 100644 index 69ab7c811..000000000 --- a/app/models/campaign.rb +++ /dev/null @@ -1,2 +0,0 @@ -class Campaign < ApplicationRecord -end diff --git a/spec/factories/analytics.rb b/spec/factories/analytics.rb index bc7fa2654..0d564336e 100644 --- a/spec/factories/analytics.rb +++ b/spec/factories/analytics.rb @@ -9,9 +9,4 @@ FactoryBot.define do id { SecureRandom.uuid } started_at { DateTime.current } end - - factory :campaign do - sequence(:name) { |n| "Campaign #{n}" } - sequence(:track_id, &:to_s) - end end diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb index 9ac12611c..86a7a8afa 100644 --- a/spec/system/admin/stats_spec.rb +++ b/spec/system/admin/stats_spec.rb @@ -152,19 +152,21 @@ describe "Stats", :admin do context "graphs" do scenario "event graphs", :with_frozen_time do - campaign = create(:campaign) + visit new_debate_path + fill_in_new_debate_title with: "A title for a debate" + fill_in_ckeditor "Initial debate text", with: "This is very important because..." + check "debate_terms_of_service" + click_button "Start a debate" - visit root_path(track_id: campaign.track_id) - - expect(page).to have_content "Sign out" + expect(page).to have_content "Debate created successfully." visit admin_stats_path within("#stats") do - click_link campaign.name + click_link "Debates" end - expect(page).to have_content "#{campaign.name} (1)" + expect(page).to have_content "Debates (1)" within("#graph") do expect(page).to have_content Date.current.strftime("%Y-%m-%d") diff --git a/spec/system/campaigns_spec.rb b/spec/system/campaigns_spec.rb deleted file mode 100644 index 7fe4b6afe..000000000 --- a/spec/system/campaigns_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require "rails_helper" - -describe "Email campaigns", :admin do - let!(:campaign1) { create(:campaign) } - let!(:campaign2) { create(:campaign) } - - scenario "Track email templates" do - 3.times { visit root_path(track_id: campaign1.track_id) } - 5.times { visit root_path(track_id: campaign2.track_id) } - - visit admin_stats_path - click_link campaign1.name - - expect(page).to have_content "#{campaign1.name} (3)" - - click_link "Go back" - click_link campaign2.name - - expect(page).to have_content "#{campaign2.name} (5)" - end - - scenario "Do not track erroneous track_ids" do - invalid_id = Campaign.last.id + 1 - - visit root_path(track_id: campaign1.track_id) - visit root_path(track_id: invalid_id) - - visit admin_stats_path - - expect(page).to have_content campaign1.name - expect(page).not_to have_content campaign2.name - - click_link campaign1.name - - expect(page).to have_content "#{campaign1.name} (1)" - end -end From f7e2d724dd5bbf500e0d2ec5f8c51e394e14f118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 23 Apr 2024 23:58:02 +0200 Subject: [PATCH 11/20] Replace ahoy events with real data We were tracking some events with Ahoy, but in an inconsistent way. For example, we were tracking when a debate was created, but (probably accidentally) we were only tracking proposals when they were created from the management section. For budget investments and their supports, we weren't using Ahoy events but checking their database tables instead. And we were only using ahoy events for the charts; for the other stats, we were using the real data. While we could actually fix these issues and start tracking events correctly, existing production data would remain broken because we didn't track a certain event when it happened. And, besides, why should we bother, for instance, to track when a debate is created, when we can instead access that information in the debates table? There are probably some features related to tracking an event and their visits, but we weren't using them, and we were storing more user data than we needed to. So we're removing the track events, allowing us to simplify the code and make it more consistent. We aren't removing the `ahoy_events` table in case existing Consul Democracy installations use it, but we'll remove it after releasing version 2.2.0 and adding a warning in the release notes. This change fixes the proposal created chart, since we were only tracking proposals created in the management section, and opens the possibility to add more charts in the future using data we didn't track with Ahoy. Also note the "Level 2 user Graph" test wasn't testing the graph, so we're changing it in order to test it. We're also moving it next to the other graphs test and, since we were tracking the event when we were confirming the phone, we're renaming to "Level 3 users". Finally, note that, since we were tracking events when something was created, we're including the `with_hidden` scope. This is also consistent with the other stats shown in the admin section as well as the public stats. --- .../budget_supporting_component.html.erb | 2 +- .../stats/budget_supporting_component.rb | 4 + .../admin/stats/chart_component.html.erb | 4 +- app/components/admin/stats/chart_component.rb | 26 +++---- app/controllers/admin/api/stats_controller.rb | 27 +------ app/controllers/admin/stats_controller.rb | 9 +-- .../concerns/commentable_actions.rb | 4 - app/controllers/debates_controller.rb | 1 - .../legislation/annotations_controller.rb | 7 -- .../legislation/answers_controller.rb | 8 -- .../management/proposals_controller.rb | 1 - .../verification/sms_controller.rb | 1 - app/helpers/stats_helper.rb | 6 -- app/models/ahoy/chart.rb | 47 +++++++++++- app/models/visit.rb | 1 + app/views/admin/stats/graph.html.erb | 2 +- app/views/admin/stats/show.html.erb | 11 +-- config/locales/en/admin.yml | 4 +- config/locales/es/admin.yml | 4 +- .../admin/api/stats_controller_spec.rb | 48 ++---------- spec/controllers/debates_controller_spec.rb | 28 ------- .../annotations_controller_spec.rb | 21 ----- .../legislation/answers_controller_spec.rb | 14 ---- spec/factories/analytics.rb | 6 -- spec/models/ahoy/chart_spec.rb | 76 +++++++++++++++++++ spec/models/ahoy/data_source_spec.rb | 21 ++--- spec/system/admin/stats_spec.rb | 37 ++++----- 27 files changed, 178 insertions(+), 242 deletions(-) create mode 100644 spec/models/ahoy/chart_spec.rb diff --git a/app/components/admin/stats/budget_supporting_component.html.erb b/app/components/admin/stats/budget_supporting_component.html.erb index c946d73bf..957908e9c 100644 --- a/app/components/admin/stats/budget_supporting_component.html.erb +++ b/app/components/admin/stats/budget_supporting_component.html.erb @@ -30,7 +30,7 @@ -<%= render Admin::Stats::ChartComponent.new(name: "user_supported_budgets", event: "", count: user_count) %> +<%= render Admin::Stats::ChartComponent.new(chart) %>
diff --git a/app/components/admin/stats/budget_supporting_component.rb b/app/components/admin/stats/budget_supporting_component.rb index 249deed5c..9626b033f 100644 --- a/app/components/admin/stats/budget_supporting_component.rb +++ b/app/components/admin/stats/budget_supporting_component.rb @@ -28,4 +28,8 @@ class Admin::Stats::BudgetSupportingComponent < ApplicationComponent [heading, headings_stats[heading.id][:total_participants_support_phase]] end end + + def chart + @chart ||= Ahoy::Chart.new("user_supported_budgets") + end end diff --git a/app/components/admin/stats/chart_component.html.erb b/app/components/admin/stats/chart_component.html.erb index f31b0e21a..99f2ec4bf 100644 --- a/app/components/admin/stats/chart_component.html.erb +++ b/app/components/admin/stats/chart_component.html.erb @@ -1,4 +1,4 @@
-

<%= t "admin.stats.graph.#{name || event}" %> (<%= count %>)

- <%= chart_tag id: name, event: event %> +

<%= t "admin.stats.graph.#{event}" %> (<%= count %>)

+ <%= chart_tag %>
diff --git a/app/components/admin/stats/chart_component.rb b/app/components/admin/stats/chart_component.rb index 2528e1e49..94c729910 100644 --- a/app/components/admin/stats/chart_component.rb +++ b/app/components/admin/stats/chart_component.rb @@ -1,25 +1,21 @@ class Admin::Stats::ChartComponent < ApplicationComponent - attr_reader :name, :event, :count + attr_reader :chart - def initialize(name:, event:, count:) - @name = name - @event = event - @count = count + def initialize(chart) + @chart = chart end private - def chart_tag(opt = {}) - opt[:data] ||= {} - opt[:data][:graph] = admin_api_stats_path(chart_data(opt)) - tag.div(**opt) + def count + chart.count end - def chart_data(opt = {}) - if opt[:id].present? - { opt[:id] => true } - elsif opt[:event].present? - { event: opt[:event] } - end + def event + chart.event_name + end + + def chart_tag + tag.div("data-graph": admin_api_stats_path(event: event)) end end diff --git a/app/controllers/admin/api/stats_controller.rb b/app/controllers/admin/api/stats_controller.rb index b025e3195..577d11fff 100644 --- a/app/controllers/admin/api/stats_controller.rb +++ b/app/controllers/admin/api/stats_controller.rb @@ -1,30 +1,9 @@ class Admin::Api::StatsController < Admin::Api::BaseController def show - if params[:event].blank? && - params[:visits].blank? && - params[:budget_investments].blank? && - params[:user_supported_budgets].blank? - return render json: {}, status: :bad_request - end - - ds = Ahoy::DataSource.new - if params[:event].present? - ds.add params[:event].titleize, Ahoy::Chart.new(params[:event]).group_by_day(:time).count + render json: Ahoy::Chart.new(params[:event]).data_points + else + render json: {}, status: :bad_request end - - if params[:visits].present? - ds.add "Visits", Visit.group_by_day(:started_at).count - end - - if params[:budget_investments].present? - ds.add "Budget Investments", Budget::Investment.group_by_day(:created_at).count - end - - if params[:user_supported_budgets].present? - ds.add "User supported budgets", - Vote.where(votable_type: "Budget::Investment").group_by_day(:updated_at).count - end - render json: ds.build end end diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb index fbfc06618..4cd913f27 100644 --- a/app/controllers/admin/stats_controller.rb +++ b/app/controllers/admin/stats_controller.rb @@ -30,14 +30,7 @@ class Admin::StatsController < Admin::BaseController end def graph - @name = params[:id] - @event = params[:event] - - if params[:event] - @count = Ahoy::Chart.new(params[:event]).count - else - @count = params[:count] - end + @chart = Ahoy::Chart.new(params[:event]) end def proposal_notifications diff --git a/app/controllers/concerns/commentable_actions.rb b/app/controllers/concerns/commentable_actions.rb index 8a47f509e..f02a7bcf2 100644 --- a/app/controllers/concerns/commentable_actions.rb +++ b/app/controllers/concerns/commentable_actions.rb @@ -59,10 +59,6 @@ module CommentableActions private - def track_event - ahoy.track :"#{resource_name}_created", "#{resource_name}_id": resource.id - end - def tag_cloud TagCloud.new(resource_model, params[:search]) end diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index e2b1a1efb..0bf0f3e04 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -24,7 +24,6 @@ class DebatesController < ApplicationController @debate.author = current_user if @debate.save - track_event redirect_to debate_path(@debate), notice: t("flash.actions.create.debate") else render :new diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index ba1b55f89..ce83d684b 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -51,7 +51,6 @@ class Legislation::AnnotationsController < Legislation::BaseController @annotation = @draft_version.annotations.new(annotation_params) @annotation.author = current_user if @annotation.save - track_event render json: @annotation.to_json else render json: @annotation.errors.full_messages, status: :unprocessable_entity @@ -100,12 +99,6 @@ class Legislation::AnnotationsController < Legislation::BaseController [:quote, :text, ranges: [:start, :startOffset, :end, :endOffset]] end - def track_event - ahoy.track :legislation_annotation_created, - legislation_annotation_id: @annotation.id, - legislation_draft_version_id: @draft_version.id - end - def convert_ranges_parameters annotation = params[:legislation_annotation] if annotation && annotation[:ranges] && annotation[:ranges].is_a?(String) diff --git a/app/controllers/legislation/answers_controller.rb b/app/controllers/legislation/answers_controller.rb index 3c718617f..bbe55c3a8 100644 --- a/app/controllers/legislation/answers_controller.rb +++ b/app/controllers/legislation/answers_controller.rb @@ -12,7 +12,6 @@ class Legislation::AnswersController < Legislation::BaseController if @process.debate_phase.open? @answer.user = current_user @answer.save! - track_event respond_to do |format| format.js format.html { redirect_to legislation_process_question_path(@process, @question) } @@ -35,11 +34,4 @@ class Legislation::AnswersController < Legislation::BaseController def allowed_params [:legislation_question_option_id] end - - def track_event - ahoy.track :legislation_answer_created, - legislation_answer_id: @answer.id, - legislation_question_option_id: @answer.legislation_question_option_id, - legislation_question_id: @answer.legislation_question_id - end end diff --git a/app/controllers/management/proposals_controller.rb b/app/controllers/management/proposals_controller.rb index 83bfadb5c..d319d24a5 100644 --- a/app/controllers/management/proposals_controller.rb +++ b/app/controllers/management/proposals_controller.rb @@ -17,7 +17,6 @@ class Management::ProposalsController < Management::BaseController published_at: Time.current)) if @resource.save - track_event redirect_path = url_for(controller: controller_name, action: :show, id: @resource.id) redirect_to redirect_path, notice: t("flash.actions.create.#{resource_name.underscore}") else diff --git a/app/controllers/verification/sms_controller.rb b/app/controllers/verification/sms_controller.rb index 2dc628f6b..57d51ed09 100644 --- a/app/controllers/verification/sms_controller.rb +++ b/app/controllers/verification/sms_controller.rb @@ -28,7 +28,6 @@ class Verification::SmsController < ApplicationController @sms = Verification::Sms.new(sms_params.merge(user: current_user)) if @sms.verified? current_user.update!(confirmed_phone: current_user.unconfirmed_phone) - ahoy.track(:level_2_user, user_id: current_user.id) rescue nil if VerifiedUser.phone?(current_user) current_user.update(verified_at: Time.current) diff --git a/app/helpers/stats_helper.rb b/app/helpers/stats_helper.rb index 30facd211..89046e525 100644 --- a/app/helpers/stats_helper.rb +++ b/app/helpers/stats_helper.rb @@ -1,10 +1,4 @@ module StatsHelper - def budget_investments_chart_tag(opt = {}) - opt[:data] ||= {} - opt[:data][:graph] = admin_api_stats_path(budget_investments: true) - tag.div(**opt) - end - def number_to_stats_percentage(number, options = {}) number_to_percentage(number, { strip_insignificant_zeros: true, precision: 2 }.merge(options)) end diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb index fa71186e7..d4ff7c739 100644 --- a/app/models/ahoy/chart.rb +++ b/app/models/ahoy/chart.rb @@ -1,20 +1,59 @@ module Ahoy class Chart attr_reader :event_name - delegate :count, :group_by_day, to: :events + delegate :count, to: :records def initialize(event_name) @event_name = event_name end def self.active_event_names - Ahoy::Event.distinct.order(:name).pluck(:name) + event_names_with_collections.select { |name, collection| collection.any? }.keys + end + + def self.event_names_with_collections + { + budget_investment_created: Budget::Investment.with_hidden, + debate_created: Debate.with_hidden, + legislation_annotation_created: Legislation::Annotation.with_hidden, + legislation_answer_created: Legislation::Answer.with_hidden, + level_3_user: User.with_hidden.level_three_verified, + proposal_created: Proposal.with_hidden + } + end + + def data_points + ds = Ahoy::DataSource.new + ds.add event_name.to_s.titleize, records_by_day.count + + ds.build end private - def events - Ahoy::Event.where(name: event_name) + def records + case event_name.to_sym + when :user_supported_budgets + Vote.where(votable_type: "Budget::Investment") + when :visits + Visit.all + else + self.class.event_names_with_collections[event_name.to_sym] + end + end + + def records_by_day + raise "Unknown event #{event_name}" unless records.respond_to?(:group_by_day) + + records.group_by_day(date_field) + end + + def date_field + if event_name.to_sym == :level_3_user + :verified_at + else + :created_at + end end end end diff --git a/app/models/visit.rb b/app/models/visit.rb index 9bcf891d7..f34d979a1 100644 --- a/app/models/visit.rb +++ b/app/models/visit.rb @@ -1,4 +1,5 @@ class Visit < ApplicationRecord + alias_attribute :created_at, :started_at has_many :ahoy_events, class_name: "Ahoy::Event" belongs_to :user end diff --git a/app/views/admin/stats/graph.html.erb b/app/views/admin/stats/graph.html.erb index 85c8395de..de76aee54 100644 --- a/app/views/admin/stats/graph.html.erb +++ b/app/views/admin/stats/graph.html.erb @@ -4,4 +4,4 @@ <%= back_link_to admin_stats_path %> -<%= render Admin::Stats::ChartComponent.new(name: @name, event: @event, count: @count) %> +<%= render Admin::Stats::ChartComponent.new(@chart) %> diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index 0e8594da9..7c06341ef 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -1,7 +1,3 @@ -<% content_for :head do %> - <%= javascript_include_tag "stat_graphs", "data-turbolinks-track" => "reload" %> -<% end %> -
@@ -28,7 +24,7 @@

<%= link_to t("admin.stats.show.summary.visits"), - graph_admin_stats_path(id: "visits", count: @visits) %>
+ graph_admin_stats_path(event: "visits") %>
<%= number_with_delimiter(@visits) %>

@@ -115,11 +111,6 @@

<%= render Admin::Stats::EventLinksComponent.new(@event_names) %> - - <% if feature?(:budgets) %> -

<%= t "admin.stats.show.budgets_title" %>

- <%= budget_investments_chart_tag id: "budget_investments" %> - <% end %>
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 727be148e..333bd6bcd 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1480,7 +1480,6 @@ en: verified_users_who_didnt_vote_proposals: Verified users who didn't votes proposals visits: Visits votes: Total votes - budgets_title: Participatory budgeting participatory_budgets: Participatory Budgets direct_messages: Direct messages proposal_notifications: Proposal notifications @@ -1488,9 +1487,10 @@ en: polls: Polls sdg: SDG graph: + budget_investment_created: Budget investments created debate_created: Debates visit: Visits - level_2_user: Level 2 users + level_3_user: Level 3 users proposal_created: Citizen proposals title: Graphs budgets: diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index c562e63a3..bac399ade 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1480,7 +1480,6 @@ es: verified_users_who_didnt_vote_proposals: Usuarios verificados que no han votado propuestas visits: Visitas votes: Votos - budgets_title: Presupuestos participativos participatory_budgets: Presupuestos Participativos direct_messages: Mensajes directos proposal_notifications: Notificaciones de propuestas @@ -1488,9 +1487,10 @@ es: polls: Votaciones sdg: ODS graph: + budget_investment_created: Proyectos de gasto creados debate_created: Debates visit: Visitas - level_2_user: Usuarios nivel 2 + level_3_user: Usuarios nivel 3 proposal_created: Propuestas Ciudadanas title: Gráficos budgets: diff --git a/spec/controllers/admin/api/stats_controller_spec.rb b/spec/controllers/admin/api/stats_controller_spec.rb index ee9fd7605..b30008bed 100644 --- a/spec/controllers/admin/api/stats_controller_spec.rb +++ b/spec/controllers/admin/api/stats_controller_spec.rb @@ -17,51 +17,19 @@ describe Admin::Api::StatsController, :admin do time_2 = Time.zone.local(2015, 01, 02) time_3 = Time.zone.local(2015, 01, 03) - create(:ahoy_event, name: "foo", time: time_1) - create(:ahoy_event, name: "foo", time: time_1) - create(:ahoy_event, name: "foo", time: time_2) - create(:ahoy_event, name: "bar", time: time_1) - create(:ahoy_event, name: "bar", time: time_3) - create(:ahoy_event, name: "bar", time: time_3) + create(:proposal, created_at: time_1) + create(:proposal, created_at: time_1) + create(:proposal, created_at: time_2) + create(:debate, created_at: time_1) + create(:debate, created_at: time_3) + create(:debate, created_at: time_3) end it "returns single events formated for working with c3.js" do - get :show, params: { event: "foo" } + get :show, params: { event: "proposal_created" } expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Foo" => [2, 1] - end - end - - context "visits present" do - it "returns visits formated for working with c3.js" do - time_1 = Time.zone.local(2015, 01, 01) - time_2 = Time.zone.local(2015, 01, 02) - - create(:visit, started_at: time_1) - create(:visit, started_at: time_1) - create(:visit, started_at: time_2) - - get :show, params: { visits: true } - - expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Visits" => [2, 1] - end - end - - context "budget investments present" do - it "returns budget investments formated for working with c3.js" do - time_1 = Time.zone.local(2017, 04, 01) - time_2 = Time.zone.local(2017, 04, 02) - - create(:budget_investment, created_at: time_1) - create(:budget_investment, created_at: time_2) - create(:budget_investment, created_at: time_2) - - get :show, params: { budget_investments: true } - - expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2017-04-01", "2017-04-02"], "Budget Investments" => [1, 2] + expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Proposal Created" => [2, 1] end end end diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb index c3d96dcea..a9ed78059 100644 --- a/spec/controllers/debates_controller_spec.rb +++ b/spec/controllers/debates_controller_spec.rb @@ -9,34 +9,6 @@ describe DebatesController do end end - describe "POST create" do - before do - InvisibleCaptcha.timestamp_enabled = false - end - - after do - InvisibleCaptcha.timestamp_enabled = true - end - - it "creates an ahoy event" do - debate_attributes = { - terms_of_service: "1", - translations_attributes: { - "0" => { - title: "A sample debate", - description: "this is a sample debate", - locale: "en" - } - } - } - sign_in create(:user) - - post :create, params: { debate: debate_attributes } - expect(Ahoy::Event.where(name: :debate_created).count).to eq 1 - expect(Ahoy::Event.last.properties["debate_id"]).to eq Debate.last.id - end - end - describe "PUT mark_featured" do it "ignores query parameters" do debate = create(:debate) diff --git a/spec/controllers/legislation/annotations_controller_spec.rb b/spec/controllers/legislation/annotations_controller_spec.rb index 575e9626d..cbefc4721 100644 --- a/spec/controllers/legislation/annotations_controller_spec.rb +++ b/spec/controllers/legislation/annotations_controller_spec.rb @@ -36,27 +36,6 @@ describe Legislation::AnnotationsController do end let(:user) { create(:user, :level_two) } - it "creates an ahoy event" do - sign_in user - - post :create, params: { - process_id: legal_process.id, - draft_version_id: draft_version.id, - legislation_annotation: { - "quote" => "ipsum", - "ranges" => [{ - "start" => "/p[1]", - "startOffset" => 6, - "end" => "/p[1]", - "endOffset" => 11 - }], - "text" => "una anotacion" - } - } - expect(Ahoy::Event.where(name: :legislation_annotation_created).count).to eq 1 - expect(Ahoy::Event.last.properties["legislation_annotation_id"]).to eq Legislation::Annotation.last.id - end - it "does not create an annotation if the draft version is a final version" do sign_in user diff --git a/spec/controllers/legislation/answers_controller_spec.rb b/spec/controllers/legislation/answers_controller_spec.rb index 04bc21dce..11dc1bc2a 100644 --- a/spec/controllers/legislation/answers_controller_spec.rb +++ b/spec/controllers/legislation/answers_controller_spec.rb @@ -10,20 +10,6 @@ describe Legislation::AnswersController do let(:question_option) { create(:legislation_question_option, question: question, value: "Yes") } let(:user) { create(:user, :level_two) } - it "creates an ahoy event" do - sign_in user - - post :create, params: { - process_id: legal_process.id, - question_id: question.id, - legislation_answer: { - legislation_question_option_id: question_option.id - } - } - expect(Ahoy::Event.where(name: :legislation_answer_created).count).to eq 1 - expect(Ahoy::Event.last.properties["legislation_answer_id"]).to eq Legislation::Answer.last.id - end - it "creates an answer if the process debate phase is open" do sign_in user diff --git a/spec/factories/analytics.rb b/spec/factories/analytics.rb index 0d564336e..8afd58ffa 100644 --- a/spec/factories/analytics.rb +++ b/spec/factories/analytics.rb @@ -1,10 +1,4 @@ FactoryBot.define do - factory :ahoy_event, class: "Ahoy::Event" do - id { SecureRandom.uuid } - time { DateTime.current } - sequence(:name) { |n| "Event #{n} type" } - end - factory :visit do id { SecureRandom.uuid } started_at { DateTime.current } diff --git a/spec/models/ahoy/chart_spec.rb b/spec/models/ahoy/chart_spec.rb new file mode 100644 index 000000000..086e340b1 --- /dev/null +++ b/spec/models/ahoy/chart_spec.rb @@ -0,0 +1,76 @@ +require "rails_helper" + +describe Ahoy::Chart do + describe "#data_points" do + it "raises an exception for unknown events" do + chart = Ahoy::Chart.new(:mystery) + + expect { chart.data_points }.to raise_exception "Unknown event mystery" + end + + it "returns data associated with the event" do + time_1 = Time.zone.local(2015, 01, 01) + time_2 = Time.zone.local(2015, 01, 02) + time_3 = Time.zone.local(2015, 01, 03) + + create(:proposal, created_at: time_1) + create(:proposal, created_at: time_1) + create(:proposal, created_at: time_2) + create(:debate, created_at: time_1) + create(:debate, created_at: time_3) + + chart = Ahoy::Chart.new(:proposal_created) + + expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Proposal Created" => [2, 1] + end + + it "accepts strings as the event name" do + create(:proposal, created_at: Time.zone.local(2015, 01, 01)) + create(:debate, created_at: Time.zone.local(2015, 01, 02)) + + chart = Ahoy::Chart.new("proposal_created") + + expect(chart.data_points).to eq x: ["2015-01-01"], "Proposal Created" => [1] + end + + it "returns visits data for the visits event" do + time_1 = Time.zone.local(2015, 01, 01) + time_2 = Time.zone.local(2015, 01, 02) + + create(:visit, started_at: time_1) + create(:visit, started_at: time_1) + create(:visit, started_at: time_2) + + chart = Ahoy::Chart.new(:visits) + + expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Visits" => [2, 1] + end + + it "returns user supports for the user_supported_budgets event" do + time_1 = Time.zone.local(2017, 04, 01) + time_2 = Time.zone.local(2017, 04, 02) + + create(:vote, votable: create(:budget_investment), created_at: time_1) + create(:vote, votable: create(:budget_investment), created_at: time_2) + create(:vote, votable: create(:budget_investment), created_at: time_2) + create(:vote, votable: create(:proposal), created_at: time_2) + + chart = Ahoy::Chart.new(:user_supported_budgets) + + expect(chart.data_points).to eq x: ["2017-04-01", "2017-04-02"], "User Supported Budgets" => [1, 2] + end + + it "returns level three verified dates for the level_3_user event" do + time_1 = Time.zone.local(2001, 01, 01) + time_2 = Time.zone.local(2001, 01, 02) + + create(:user, :level_two, level_two_verified_at: time_1) + create(:user, :level_three, verified_at: time_2) + create(:user, :level_three, verified_at: time_2, level_two_verified_at: time_1) + + chart = Ahoy::Chart.new(:level_3_user) + + expect(chart.data_points).to eq x: ["2001-01-02"], "Level 3 User" => [2] + end + end +end diff --git a/spec/models/ahoy/data_source_spec.rb b/spec/models/ahoy/data_source_spec.rb index df8d2756d..2766d1391 100644 --- a/spec/models/ahoy/data_source_spec.rb +++ b/spec/models/ahoy/data_source_spec.rb @@ -2,18 +2,9 @@ require "rails_helper" describe Ahoy::DataSource do describe "#build" do - before do - time_1 = Time.zone.local(2015, 01, 01) - time_2 = Time.zone.local(2015, 01, 02) - time_3 = Time.zone.local(2015, 01, 03) - - create(:ahoy_event, name: "foo", time: time_1) - create(:ahoy_event, name: "foo", time: time_1) - create(:ahoy_event, name: "foo", time: time_2) - create(:ahoy_event, name: "bar", time: time_1) - create(:ahoy_event, name: "bar", time: time_3) - create(:ahoy_event, name: "bar", time: time_3) - end + let(:january_first) { Time.zone.local(2015, 01, 01) } + let(:january_second) { Time.zone.local(2015, 01, 02) } + let(:january_third) { Time.zone.local(2015, 01, 03) } it "works without data sources" do ds = Ahoy::DataSource.new @@ -22,14 +13,14 @@ describe Ahoy::DataSource do it "works with single data sources" do ds = Ahoy::DataSource.new - ds.add "foo", Ahoy::Event.where(name: "foo").group_by_day(:time).count + ds.add "foo", { january_first => 2, january_second => 1 } expect(ds.build).to eq :x => ["2015-01-01", "2015-01-02"], "foo" => [2, 1] end it "combines data sources" do ds = Ahoy::DataSource.new - ds.add "foo", Ahoy::Event.where(name: "foo").group_by_day(:time).count - ds.add "bar", Ahoy::Event.where(name: "bar").group_by_day(:time).count + ds.add "foo", { january_first => 2, january_second => 1 } + ds.add "bar", { january_first => 1, january_third => 2 } expect(ds.build).to eq :x => ["2015-01-01", "2015-01-02", "2015-01-03"], "foo" => [2, 1, 0], "bar" => [1, 0, 2] diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb index 86a7a8afa..085f71c15 100644 --- a/spec/system/admin/stats_spec.rb +++ b/spec/system/admin/stats_spec.rb @@ -72,18 +72,6 @@ describe "Stats", :admin do expect(page).to have_content "UNVERIFIED USERS\n1" expect(page).to have_content "TOTAL USERS\n1" end - - scenario "Level 2 user Graph" do - create(:geozone) - visit account_path - click_link "Verify my account" - verify_residence - confirm_phone - - visit admin_stats_path - - expect(page).to have_content "LEVEL TWO USERS\n1" - end end describe "Budget investments" do @@ -150,15 +138,9 @@ describe "Stats", :admin do end end - context "graphs" do - scenario "event graphs", :with_frozen_time do - visit new_debate_path - fill_in_new_debate_title with: "A title for a debate" - fill_in_ckeditor "Initial debate text", with: "This is very important because..." - check "debate_terms_of_service" - click_button "Start a debate" - - expect(page).to have_content "Debate created successfully." + describe "graphs", :with_frozen_time do + scenario "event graphs" do + create(:debate) visit admin_stats_path @@ -172,6 +154,19 @@ describe "Stats", :admin do expect(page).to have_content Date.current.strftime("%Y-%m-%d") end end + + scenario "Level 3 user Graph" do + create(:user, :level_three) + + visit admin_stats_path + click_link "level_3_user" + + expect(page).to have_content "Level 3 User (1)" + + within("#graph") do + expect(page).to have_content Date.current.strftime("%Y-%m-%d") + end + end end context "Proposal notifications" do From 328413373ea04d988d49f891f8b771ee8ea0bac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 25 Apr 2024 03:18:12 +0200 Subject: [PATCH 12/20] Use the same texts on event links and graphs Note we're delegating the `t` method because i18n-tasks doesn't detect code like `ApplicationController.helpers.t` and so reports we aren't using the `admin.stats.graph` translations. --- app/components/admin/stats/chart_component.html.erb | 2 +- app/components/admin/stats/chart_component.rb | 4 ++++ app/components/admin/stats/event_links_component.rb | 6 +----- app/models/ahoy/chart.rb | 11 ++++++++++- .../admin/stats/event_links_component_spec.rb | 4 ++-- spec/controllers/admin/api/stats_controller_spec.rb | 2 +- spec/models/ahoy/chart_spec.rb | 6 +++--- spec/system/admin/stats_spec.rb | 4 ++-- 8 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app/components/admin/stats/chart_component.html.erb b/app/components/admin/stats/chart_component.html.erb index 99f2ec4bf..758bf90e8 100644 --- a/app/components/admin/stats/chart_component.html.erb +++ b/app/components/admin/stats/chart_component.html.erb @@ -1,4 +1,4 @@
-

<%= t "admin.stats.graph.#{event}" %> (<%= count %>)

+

<%= title %>

<%= chart_tag %>
diff --git a/app/components/admin/stats/chart_component.rb b/app/components/admin/stats/chart_component.rb index 94c729910..2aeb1fd23 100644 --- a/app/components/admin/stats/chart_component.rb +++ b/app/components/admin/stats/chart_component.rb @@ -18,4 +18,8 @@ class Admin::Stats::ChartComponent < ApplicationComponent def chart_tag tag.div("data-graph": admin_api_stats_path(event: event)) end + + def title + "#{chart.title} (#{count})" + end end diff --git a/app/components/admin/stats/event_links_component.rb b/app/components/admin/stats/event_links_component.rb index 9fcad74eb..dbe79b26f 100644 --- a/app/components/admin/stats/event_links_component.rb +++ b/app/components/admin/stats/event_links_component.rb @@ -9,11 +9,7 @@ class Admin::Stats::EventLinksComponent < ApplicationComponent private def link_text(event) - text = t("admin.stats.graph.#{event}") - if text.to_s.match(/translation missing/) - text = event - end - text + Ahoy::Chart.new(event).title end def title diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb index d4ff7c739..7f445b469 100644 --- a/app/models/ahoy/chart.rb +++ b/app/models/ahoy/chart.rb @@ -2,6 +2,7 @@ module Ahoy class Chart attr_reader :event_name delegate :count, to: :records + delegate :t, to: "ApplicationController.helpers" def initialize(event_name) @event_name = event_name @@ -24,11 +25,19 @@ module Ahoy def data_points ds = Ahoy::DataSource.new - ds.add event_name.to_s.titleize, records_by_day.count + ds.add title, records_by_day.count ds.build end + def title + text = t("admin.stats.graph.#{event_name}") + if text.to_s.match(/translation missing/) + text = event_name.to_s.titleize + end + text + end + private def records diff --git a/spec/components/admin/stats/event_links_component_spec.rb b/spec/components/admin/stats/event_links_component_spec.rb index 5c5728a74..08eb07c01 100644 --- a/spec/components/admin/stats/event_links_component_spec.rb +++ b/spec/components/admin/stats/event_links_component_spec.rb @@ -10,8 +10,8 @@ describe Admin::Stats::EventLinksComponent do expect(page).to have_link count: 2 page.find("ul") do |list| - expect(list).to have_link "legislation_annotation_created" - expect(list).to have_link "legislation_answer_created" + expect(list).to have_link "Legislation Annotation Created" + expect(list).to have_link "Legislation Answer Created" end end end diff --git a/spec/controllers/admin/api/stats_controller_spec.rb b/spec/controllers/admin/api/stats_controller_spec.rb index b30008bed..ee6d0a014 100644 --- a/spec/controllers/admin/api/stats_controller_spec.rb +++ b/spec/controllers/admin/api/stats_controller_spec.rb @@ -29,7 +29,7 @@ describe Admin::Api::StatsController, :admin do get :show, params: { event: "proposal_created" } expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Proposal Created" => [2, 1] + expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Citizen proposals" => [2, 1] end end end diff --git a/spec/models/ahoy/chart_spec.rb b/spec/models/ahoy/chart_spec.rb index 086e340b1..fe42972a8 100644 --- a/spec/models/ahoy/chart_spec.rb +++ b/spec/models/ahoy/chart_spec.rb @@ -21,7 +21,7 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new(:proposal_created) - expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Proposal Created" => [2, 1] + expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Citizen proposals" => [2, 1] end it "accepts strings as the event name" do @@ -30,7 +30,7 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new("proposal_created") - expect(chart.data_points).to eq x: ["2015-01-01"], "Proposal Created" => [1] + expect(chart.data_points).to eq x: ["2015-01-01"], "Citizen proposals" => [1] end it "returns visits data for the visits event" do @@ -70,7 +70,7 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new(:level_3_user) - expect(chart.data_points).to eq x: ["2001-01-02"], "Level 3 User" => [2] + expect(chart.data_points).to eq x: ["2001-01-02"], "Level 3 users" => [2] end end end diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb index 085f71c15..f64a8eb23 100644 --- a/spec/system/admin/stats_spec.rb +++ b/spec/system/admin/stats_spec.rb @@ -159,9 +159,9 @@ describe "Stats", :admin do create(:user, :level_three) visit admin_stats_path - click_link "level_3_user" + click_link "Level 3 users" - expect(page).to have_content "Level 3 User (1)" + expect(page).to have_content "Level 3 users (1)" within("#graph") do expect(page).to have_content Date.current.strftime("%Y-%m-%d") From d25f2e7259c12589d4f97873f5cb86d0a1acfa8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 25 Apr 2024 03:44:09 +0200 Subject: [PATCH 13/20] Add missing event name translations We were always displaying the event names in English. Note we're changing the `user_supported_budgets` key because it didn't make much sense; the investments are supported, and not the budgets. We're also adding "created" to most of the event names in order to make the texts more explicit, since not all the events refer to created data. --- .../admin/stats/budget_supporting_component.rb | 2 +- app/models/ahoy/chart.rb | 8 ++------ config/locales/en/admin.yml | 11 +++++++---- config/locales/es/admin.yml | 11 +++++++---- .../admin/stats/event_links_component_spec.rb | 4 ++-- .../controllers/admin/api/stats_controller_spec.rb | 2 +- spec/models/ahoy/chart_spec.rb | 14 ++++++++------ spec/system/admin/stats_spec.rb | 8 ++++---- 8 files changed, 32 insertions(+), 28 deletions(-) diff --git a/app/components/admin/stats/budget_supporting_component.rb b/app/components/admin/stats/budget_supporting_component.rb index 9626b033f..5d7b9cea2 100644 --- a/app/components/admin/stats/budget_supporting_component.rb +++ b/app/components/admin/stats/budget_supporting_component.rb @@ -30,6 +30,6 @@ class Admin::Stats::BudgetSupportingComponent < ApplicationComponent end def chart - @chart ||= Ahoy::Chart.new("user_supported_budgets") + @chart ||= Ahoy::Chart.new("budget_investment_supported") end end diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb index 7f445b469..033a55c56 100644 --- a/app/models/ahoy/chart.rb +++ b/app/models/ahoy/chart.rb @@ -31,18 +31,14 @@ module Ahoy end def title - text = t("admin.stats.graph.#{event_name}") - if text.to_s.match(/translation missing/) - text = event_name.to_s.titleize - end - text + t("admin.stats.graph.#{event_name}") end private def records case event_name.to_sym - when :user_supported_budgets + when :budget_investment_supported Vote.where(votable_type: "Budget::Investment") when :visits Visit.all diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 333bd6bcd..66ff65ea1 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1488,11 +1488,14 @@ en: sdg: SDG graph: budget_investment_created: Budget investments created - debate_created: Debates - visit: Visits - level_3_user: Level 3 users - proposal_created: Citizen proposals + budget_investment_supported: Budget investments supported + debate_created: Debates created + legislation_annotation_created: Legislation annotations created + legislation_answer_created: Legislation answers created + level_3_user: Level 3 users verified + proposal_created: Citizen proposals created title: Graphs + visits: Visits budgets: no_data_before_balloting_phase: "There isn't any data to show before the balloting phase." title: "Participatory Budgets - Participation stats" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index bac399ade..81297a548 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1488,11 +1488,14 @@ es: sdg: ODS graph: budget_investment_created: Proyectos de gasto creados - debate_created: Debates - visit: Visitas - level_3_user: Usuarios nivel 3 - proposal_created: Propuestas Ciudadanas + budget_investment_supported: Proyectos de gasto apoyados + debate_created: Debates creados + legislation_annotation_created: Anotaciones creadas + legislation_answer_created: Repuestas de legislación colaborativa creadas + level_3_user: Usuarios de nivel 3 verificados + proposal_created: Propuestas Ciudadanas creadas title: Gráficos + visit: Visitas budgets: no_data_before_balloting_phase: "No hay datos que mostrar hasta que la fase de votación no esté abierta." title: "Presupuestos participativos - Estadisticas de participación" diff --git a/spec/components/admin/stats/event_links_component_spec.rb b/spec/components/admin/stats/event_links_component_spec.rb index 08eb07c01..847c2047a 100644 --- a/spec/components/admin/stats/event_links_component_spec.rb +++ b/spec/components/admin/stats/event_links_component_spec.rb @@ -10,8 +10,8 @@ describe Admin::Stats::EventLinksComponent do expect(page).to have_link count: 2 page.find("ul") do |list| - expect(list).to have_link "Legislation Annotation Created" - expect(list).to have_link "Legislation Answer Created" + expect(list).to have_link "Legislation annotations created" + expect(list).to have_link "Legislation answers created" end end end diff --git a/spec/controllers/admin/api/stats_controller_spec.rb b/spec/controllers/admin/api/stats_controller_spec.rb index ee6d0a014..472fbda86 100644 --- a/spec/controllers/admin/api/stats_controller_spec.rb +++ b/spec/controllers/admin/api/stats_controller_spec.rb @@ -29,7 +29,7 @@ describe Admin::Api::StatsController, :admin do get :show, params: { event: "proposal_created" } expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Citizen proposals" => [2, 1] + expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Citizen proposals created" => [2, 1] end end end diff --git a/spec/models/ahoy/chart_spec.rb b/spec/models/ahoy/chart_spec.rb index fe42972a8..3791aff51 100644 --- a/spec/models/ahoy/chart_spec.rb +++ b/spec/models/ahoy/chart_spec.rb @@ -21,7 +21,8 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new(:proposal_created) - expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Citizen proposals" => [2, 1] + expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], + "Citizen proposals created" => [2, 1] end it "accepts strings as the event name" do @@ -30,7 +31,7 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new("proposal_created") - expect(chart.data_points).to eq x: ["2015-01-01"], "Citizen proposals" => [1] + expect(chart.data_points).to eq x: ["2015-01-01"], "Citizen proposals created" => [1] end it "returns visits data for the visits event" do @@ -46,7 +47,7 @@ describe Ahoy::Chart do expect(chart.data_points).to eq x: ["2015-01-01", "2015-01-02"], "Visits" => [2, 1] end - it "returns user supports for the user_supported_budgets event" do + it "returns user supports for the budget_investment_supported event" do time_1 = Time.zone.local(2017, 04, 01) time_2 = Time.zone.local(2017, 04, 02) @@ -55,9 +56,10 @@ describe Ahoy::Chart do create(:vote, votable: create(:budget_investment), created_at: time_2) create(:vote, votable: create(:proposal), created_at: time_2) - chart = Ahoy::Chart.new(:user_supported_budgets) + chart = Ahoy::Chart.new(:budget_investment_supported) - expect(chart.data_points).to eq x: ["2017-04-01", "2017-04-02"], "User Supported Budgets" => [1, 2] + expect(chart.data_points).to eq x: ["2017-04-01", "2017-04-02"], + "Budget investments supported" => [1, 2] end it "returns level three verified dates for the level_3_user event" do @@ -70,7 +72,7 @@ describe Ahoy::Chart do chart = Ahoy::Chart.new(:level_3_user) - expect(chart.data_points).to eq x: ["2001-01-02"], "Level 3 users" => [2] + expect(chart.data_points).to eq x: ["2001-01-02"], "Level 3 users verified" => [2] end end end diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb index f64a8eb23..88c77cf2d 100644 --- a/spec/system/admin/stats_spec.rb +++ b/spec/system/admin/stats_spec.rb @@ -145,10 +145,10 @@ describe "Stats", :admin do visit admin_stats_path within("#stats") do - click_link "Debates" + click_link "Debates created" end - expect(page).to have_content "Debates (1)" + expect(page).to have_content "Debates created (1)" within("#graph") do expect(page).to have_content Date.current.strftime("%Y-%m-%d") @@ -159,9 +159,9 @@ describe "Stats", :admin do create(:user, :level_three) visit admin_stats_path - click_link "Level 3 users" + click_link "Level 3 users verified" - expect(page).to have_content "Level 3 users (1)" + expect(page).to have_content "Level 3 users verified (1)" within("#graph") do expect(page).to have_content Date.current.strftime("%Y-%m-%d") From 14454bdd45db4eb469cf7cf5fd6ab23dc0699fcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 25 Apr 2024 02:11:00 +0200 Subject: [PATCH 14/20] Include data in chart tags instead of using AJAX Now that we've moved the logic to generate the events data to a model, and we've got access to the model in the component rendering the chart, we can render the data inside the chart instead of doing an extra AJAX request to get the same data. Originally, this was problaby done this way so the page wouldn't take several seconds to load while preparing the data for the chart when there are thousands of dates being displayed. With an AJAX call, the page would load as fast as usual, and then the chart would render after a few seconds. However, we can have an even better performance improvement in this scenario if we use a Set instead of an Array. The method `Array#include?`, which we were calling for every date in the data, is much slower that `Set#merge`. So now both the page and the chart load as fast as expected. We could also use something like: ``` def add (...) shared_keys.push(*collection.keys) end def build (...) shared_keys.uniq.each do |k| (...) end def shared_keys @shared_keys ||= [] end ``` Or other approaches to avoid using `Array#include?`. The performance would be similar to the one we get when using `Set`. We're using a `Set` because it makes more obvious that `shared_keys` is supposed to contain unique elements. We've had some tests failing in the past due to these AJAX requests being triggered automatically during the tests and no expectations checking the requests have finished, so now we're reducing the amount of flaky tests. --- app/assets/javascripts/stats.js | 6 ++-- app/components/admin/stats/chart_component.rb | 2 +- app/controllers/admin/api/stats_controller.rb | 9 ----- app/models/ahoy/data_source.rb | 8 ++--- config/routes/admin.rb | 4 --- .../admin/stats/chart_component_spec.rb | 13 +++++++ .../admin/api/stats_controller_spec.rb | 36 ------------------- 7 files changed, 18 insertions(+), 60 deletions(-) delete mode 100644 app/controllers/admin/api/stats_controller.rb create mode 100644 spec/components/admin/stats/chart_component_spec.rb delete mode 100644 spec/controllers/admin/api/stats_controller_spec.rb diff --git a/app/assets/javascripts/stats.js b/app/assets/javascripts/stats.js index fe7e46d8b..924c4a2dc 100644 --- a/app/assets/javascripts/stats.js +++ b/app/assets/javascripts/stats.js @@ -5,14 +5,12 @@ var buildGraph; buildGraph = function(el) { - var conf, url; - url = $(el).data("graph"); + var conf; conf = { bindto: el, data: { x: "x", - url: url, - mimeType: "json" + json: $(el).data("graph") }, axis: { x: { diff --git a/app/components/admin/stats/chart_component.rb b/app/components/admin/stats/chart_component.rb index 2aeb1fd23..dbfde1678 100644 --- a/app/components/admin/stats/chart_component.rb +++ b/app/components/admin/stats/chart_component.rb @@ -16,7 +16,7 @@ class Admin::Stats::ChartComponent < ApplicationComponent end def chart_tag - tag.div("data-graph": admin_api_stats_path(event: event)) + tag.div("data-graph": chart.data_points.to_json) end def title diff --git a/app/controllers/admin/api/stats_controller.rb b/app/controllers/admin/api/stats_controller.rb deleted file mode 100644 index 577d11fff..000000000 --- a/app/controllers/admin/api/stats_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -class Admin::Api::StatsController < Admin::Api::BaseController - def show - if params[:event].present? - render json: Ahoy::Chart.new(params[:event]).data_points - else - render json: {}, status: :bad_request - end - end -end diff --git a/app/models/ahoy/data_source.rb b/app/models/ahoy/data_source.rb index e1430c5e8..a29902593 100644 --- a/app/models/ahoy/data_source.rb +++ b/app/models/ahoy/data_source.rb @@ -9,7 +9,7 @@ module Ahoy # chart def add(name, collection) collections.push data: collection, name: name - collection.each_key { |key| add_key key } + shared_keys.merge(collection.keys) end def build @@ -36,11 +36,7 @@ module Ahoy end def shared_keys - @shared_keys ||= [] - end - - def add_key(key) - shared_keys.push(key) unless shared_keys.include? key + @shared_keys ||= Set.new end end end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 5ffbf5eba..4e72dae01 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -241,10 +241,6 @@ namespace :admin do end end - namespace :api do - resource :stats, only: :show - end - resources :geozones, only: [:index, :new, :create, :edit, :update, :destroy] namespace :site_customization do diff --git a/spec/components/admin/stats/chart_component_spec.rb b/spec/components/admin/stats/chart_component_spec.rb new file mode 100644 index 000000000..c6d0bdafb --- /dev/null +++ b/spec/components/admin/stats/chart_component_spec.rb @@ -0,0 +1,13 @@ +require "rails_helper" + +describe Admin::Stats::ChartComponent do + it "renders a tag with JSON data" do + create(:user, :level_three, verified_at: "2009-09-09") + chart = Ahoy::Chart.new(:level_3_user) + + render_inline Admin::Stats::ChartComponent.new(chart) + + expect(page).to have_css "h2", exact_text: "Level 3 users verified (1)" + expect(page).to have_css "[data-graph='{\"x\":[\"2009-09-09\"],\"Level 3 users verified\":[1]}']" + end +end diff --git a/spec/controllers/admin/api/stats_controller_spec.rb b/spec/controllers/admin/api/stats_controller_spec.rb deleted file mode 100644 index 472fbda86..000000000 --- a/spec/controllers/admin/api/stats_controller_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -require "rails_helper" - -describe Admin::Api::StatsController, :admin do - describe "GET index" do - context "events or visits not present" do - it "responds with bad_request" do - get :show - - expect(response).not_to be_ok - expect(response).to have_http_status 400 - end - end - - context "events present" do - before do - time_1 = Time.zone.local(2015, 01, 01) - time_2 = Time.zone.local(2015, 01, 02) - time_3 = Time.zone.local(2015, 01, 03) - - create(:proposal, created_at: time_1) - create(:proposal, created_at: time_1) - create(:proposal, created_at: time_2) - create(:debate, created_at: time_1) - create(:debate, created_at: time_3) - create(:debate, created_at: time_3) - end - - it "returns single events formated for working with c3.js" do - get :show, params: { event: "proposal_created" } - - expect(response).to be_ok - expect(response.parsed_body).to eq "x" => ["2015-01-01", "2015-01-02"], "Citizen proposals created" => [2, 1] - end - end - end -end From b5d12959a01734567690e80212d3d4b13eb7fcf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 25 Apr 2024 02:11:16 +0200 Subject: [PATCH 15/20] Add a chart showing all events in admin stats Not sure it's useful, but it makes the main admin stats page a bit more interesting. --- .../stylesheets/admin/stats/event_links.scss | 15 ++++------ .../stats/event_links_component.html.erb | 5 +--- .../admin/stats/event_links_component.rb | 4 --- .../stats/global_chart_component.html.erb | 5 ++++ .../admin/stats/global_chart_component.rb | 19 +++++++++++++ app/controllers/admin/stats_controller.rb | 2 -- app/models/ahoy/chart.rb | 21 ++++++++++++-- app/views/admin/stats/show.html.erb | 6 +++- .../admin/stats/event_links_component_spec.rb | 3 +- .../stats/global_chart_component_spec.rb | 18 ++++++++++++ spec/models/ahoy/chart_spec.rb | 28 +++++++++++++++++++ 11 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 app/components/admin/stats/global_chart_component.html.erb create mode 100644 app/components/admin/stats/global_chart_component.rb create mode 100644 spec/components/admin/stats/global_chart_component_spec.rb diff --git a/app/assets/stylesheets/admin/stats/event_links.scss b/app/assets/stylesheets/admin/stats/event_links.scss index 5375edc41..e781d41a6 100644 --- a/app/assets/stylesheets/admin/stats/event_links.scss +++ b/app/assets/stylesheets/admin/stats/event_links.scss @@ -1,14 +1,11 @@ .stats-event-links { + @include header-font-size(h3); + font-weight: bold; + list-style-type: none; + margin-#{$global-left}: 0; margin-top: $line-height; - ul { - @include header-font-size(h3); - font-weight: bold; - list-style-type: none; - margin-#{$global-left}: 0; - - * + * { - margin-top: $line-height; - } + * + * { + margin-top: $line-height; } } diff --git a/app/components/admin/stats/event_links_component.html.erb b/app/components/admin/stats/event_links_component.html.erb index 7a1181571..b0c861d71 100644 --- a/app/components/admin/stats/event_links_component.html.erb +++ b/app/components/admin/stats/event_links_component.html.erb @@ -1,4 +1 @@ -
-

<%= title %>

- <%= link_list(*links) %> -
+<%= link_list(*links, class: "stats-event-links") %> diff --git a/app/components/admin/stats/event_links_component.rb b/app/components/admin/stats/event_links_component.rb index dbe79b26f..6f7480406 100644 --- a/app/components/admin/stats/event_links_component.rb +++ b/app/components/admin/stats/event_links_component.rb @@ -12,10 +12,6 @@ class Admin::Stats::EventLinksComponent < ApplicationComponent Ahoy::Chart.new(event).title end - def title - t("admin.stats.graph.title") - end - def links event_names.map do |event| [link_text(event), graph_admin_stats_path(event: event)] diff --git a/app/components/admin/stats/global_chart_component.html.erb b/app/components/admin/stats/global_chart_component.html.erb new file mode 100644 index 000000000..10a1368e5 --- /dev/null +++ b/app/components/admin/stats/global_chart_component.html.erb @@ -0,0 +1,5 @@ +
+

<%= title %>

+ <%= chart_tag %> + <%= render Admin::Stats::EventLinksComponent.new(event_names) %> +
diff --git a/app/components/admin/stats/global_chart_component.rb b/app/components/admin/stats/global_chart_component.rb new file mode 100644 index 000000000..ae4ecec1f --- /dev/null +++ b/app/components/admin/stats/global_chart_component.rb @@ -0,0 +1,19 @@ +class Admin::Stats::GlobalChartComponent < ApplicationComponent + private + + def event_names + Ahoy::Chart.active_event_names + end + + def chart_tag + tag.div("data-graph": data_points.to_json) + end + + def data_points + Ahoy::Chart.active_events_data_points + end + + def title + t("admin.stats.graph.title") + end +end diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb index 4cd913f27..3313cebd0 100644 --- a/app/controllers/admin/stats_controller.rb +++ b/app/controllers/admin/stats_controller.rb @@ -1,7 +1,5 @@ class Admin::StatsController < Admin::BaseController def show - @event_names = Ahoy::Chart.active_event_names - @visits = Visit.count @debates = Debate.with_hidden.count @proposals = Proposal.with_hidden.count diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb index 033a55c56..d2077390d 100644 --- a/app/models/ahoy/chart.rb +++ b/app/models/ahoy/chart.rb @@ -9,7 +9,9 @@ module Ahoy end def self.active_event_names - event_names_with_collections.select { |name, collection| collection.any? }.keys + event_names_with_collections.select { |name, collection| collection.any? }.keys.sort_by do |event_name| + new(event_name).title + end end def self.event_names_with_collections @@ -23,10 +25,19 @@ module Ahoy } end + def self.active_events_data_points + ds = Ahoy::DataSource.new + + active_event_names.map { |event_name| new(event_name) }.each do |chart| + ds.add chart.title, chart.data + end + + ds.build + end + def data_points ds = Ahoy::DataSource.new - ds.add title, records_by_day.count - + ds.add title, data ds.build end @@ -34,6 +45,10 @@ module Ahoy t("admin.stats.graph.#{event_name}") end + def data + records_by_day.count + end + private def records diff --git a/app/views/admin/stats/show.html.erb b/app/views/admin/stats/show.html.erb index 7c06341ef..b6eccdbc0 100644 --- a/app/views/admin/stats/show.html.erb +++ b/app/views/admin/stats/show.html.erb @@ -1,3 +1,7 @@ +<% content_for :head do %> + <%= javascript_include_tag "stat_graphs", "data-turbolinks-track" => "reload" %> +<% end %> +
@@ -110,7 +114,7 @@
- <%= render Admin::Stats::EventLinksComponent.new(@event_names) %> + <%= render Admin::Stats::GlobalChartComponent.new %>
diff --git a/spec/components/admin/stats/event_links_component_spec.rb b/spec/components/admin/stats/event_links_component_spec.rb index 847c2047a..489f22e1d 100644 --- a/spec/components/admin/stats/event_links_component_spec.rb +++ b/spec/components/admin/stats/event_links_component_spec.rb @@ -1,12 +1,11 @@ require "rails_helper" describe Admin::Stats::EventLinksComponent do - it "renders an

heading followed by a list of links" do + it "renders a list of links" do render_inline Admin::Stats::EventLinksComponent.new( %w[legislation_annotation_created legislation_answer_created] ) - expect(page).to have_css "h2", exact_text: "Graphs" expect(page).to have_link count: 2 page.find("ul") do |list| diff --git a/spec/components/admin/stats/global_chart_component_spec.rb b/spec/components/admin/stats/global_chart_component_spec.rb new file mode 100644 index 000000000..8d8eb63d6 --- /dev/null +++ b/spec/components/admin/stats/global_chart_component_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe Admin::Stats::GlobalChartComponent do + before do + allow(Ahoy::Chart).to receive(:active_event_names).and_return( + %i[budget_investment_created level_3_user] + ) + end + + it "renders an

tag with a graph and links" do + render_inline Admin::Stats::GlobalChartComponent.new + + expect(page).to have_css "h2", exact_text: "Graphs" + expect(page).to have_css "[data-graph]" + expect(page).to have_link "Budget investments created" + expect(page).to have_link "Level 3 users verified" + end +end diff --git a/spec/models/ahoy/chart_spec.rb b/spec/models/ahoy/chart_spec.rb index 3791aff51..e69d9d13e 100644 --- a/spec/models/ahoy/chart_spec.rb +++ b/spec/models/ahoy/chart_spec.rb @@ -1,6 +1,34 @@ require "rails_helper" describe Ahoy::Chart do + describe ".active_events_data_points" do + it "groups several events together" do + create(:budget_investment, created_at: "2010-01-01") + create(:budget_investment, created_at: "2010-01-02") + create(:legislation_annotation, created_at: "2010-01-01") + create(:legislation_annotation, created_at: "2010-01-03") + + expect(Ahoy::Chart.active_events_data_points).to eq({ + x: ["2010-01-01", "2010-01-02", "2010-01-03"], + "Budget investments created" => [1, 1, 0], + "Legislation annotations created" => [1, 0, 1] + }) + end + + it "sorts events alphabetically" do + create(:budget_investment, created_at: "2010-01-01") + create(:legislation_annotation, created_at: "2010-01-01") + + I18n.with_locale(:es) do + expect(Ahoy::Chart.active_events_data_points.keys).to eq([ + :x, + "Anotaciones creadas", + "Proyectos de gasto creados" + ]) + end + end + end + describe "#data_points" do it "raises an exception for unknown events" do chart = Ahoy::Chart.new(:mystery) From 5da3bc996959fea274b8a9b39e65dc94c4c4c584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 26 Apr 2024 00:57:14 +0200 Subject: [PATCH 16/20] Fix test checking notifications without a link The test was supposed to be testing notifications without a link, but it was adding a link. It was only passing because the expectations was checking whether there was a link whose text was a URL, but what we want to check here is whether the URL is present in the `href` attribute. We're about to delete the page showing public stats and, since this link was using the `/stats` path, we're fixing it. --- spec/system/notifications_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/system/notifications_spec.rb b/spec/system/notifications_spec.rb index 379aaed3a..e0139659e 100644 --- a/spec/system/notifications_spec.rb +++ b/spec/system/notifications_spec.rb @@ -175,12 +175,13 @@ describe "Notifications" do end scenario "Without a link" do - admin_notification.update!(link: "/stats") + admin_notification.update!(link: nil) visit notifications_path - expect(page).to have_content("Notification title") - expect(page).to have_content("Notification body") - expect(page).not_to have_link(notification_path(notification), visible: :all) + + expect(page).to have_content "Notification title" + expect(page).to have_content "Notification body" + expect(page).not_to have_link href: notification_path(notification), visible: :all end end From 631b48f5866e80a81f2ef271c6bd786c6ff0004e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 26 Apr 2024 01:03:07 +0200 Subject: [PATCH 17/20] Remove public stats This page isn't linked from anywhere and most Consul Democracy installations don't even know it exists, so it's useless for most people. If we ever bring it back, we should at least add a link pointing to this page. --- .../admin/settings/features_tab_component.rb | 1 - app/controllers/stats_controller.rb | 29 --------- app/models/setting.rb | 1 - app/views/stats/index.html.erb | 63 ------------------- app/views/stats/index.json.erb | 12 ---- config/locales/en/general.yml | 12 ---- config/locales/en/settings.yml | 2 - config/locales/es/general.yml | 12 ---- config/locales/es/settings.yml | 2 - config/routes.rb | 1 - spec/system/notifications_spec.rb | 5 +- spec/system/stats_spec.rb | 43 ------------- 12 files changed, 3 insertions(+), 180 deletions(-) delete mode 100644 app/controllers/stats_controller.rb delete mode 100644 app/views/stats/index.html.erb delete mode 100644 app/views/stats/index.json.erb delete mode 100644 spec/system/stats_spec.rb diff --git a/app/components/admin/settings/features_tab_component.rb b/app/components/admin/settings/features_tab_component.rb index 64e0ffce2..2feae25cd 100644 --- a/app/components/admin/settings/features_tab_component.rb +++ b/app/components/admin/settings/features_tab_component.rb @@ -6,7 +6,6 @@ class Admin::Settings::FeaturesTabComponent < ApplicationComponent feature.google_login feature.twitter_login feature.wordpress_login - feature.public_stats feature.signature_sheets feature.user.recommendations feature.user.recommendations_on_debates diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb deleted file mode 100644 index 15cfd7bb8..000000000 --- a/app/controllers/stats_controller.rb +++ /dev/null @@ -1,29 +0,0 @@ -class StatsController < ApplicationController - include FeatureFlags - - feature_flag :public_stats - - skip_authorization_check - - def index - @visits = daily_cache("visits") { Visit.count } - @debates = daily_cache("debates") { Debate.with_hidden.count } - @proposals = daily_cache("proposals") { Proposal.with_hidden.count } - @comments = daily_cache("comments") { Comment.not_valuations.with_hidden.count } - - @debate_votes = daily_cache("debate_votes") { Vote.count_for("Debate") } - @proposal_votes = daily_cache("proposal_votes") { Vote.count_for("Proposal") } - @comment_votes = daily_cache("comment_votes") { Vote.count_for("Comment") } - @investment_votes = daily_cache("budget_investment_votes") { Vote.count_for("Budget::Investment") } - @votes = daily_cache("votes") { Vote.count } - - @verified_users = daily_cache("verified_users") { User.with_hidden.level_two_or_three_verified.count } - @unverified_users = daily_cache("unverified_users") { User.with_hidden.unverified.count } - end - - private - - def daily_cache(key, &) - Rails.cache.fetch("public_stats/#{Time.current.strftime("%Y-%m-%d")}/#{key}", &) - end -end diff --git a/app/models/setting.rb b/app/models/setting.rb index 97fb97890..5427afccf 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -70,7 +70,6 @@ class Setting < ApplicationRecord "feature.google_login": true, "feature.twitter_login": true, "feature.wordpress_login": false, - "feature.public_stats": true, "feature.signature_sheets": true, "feature.user.recommendations": true, "feature.user.recommendations_on_debates": true, diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb deleted file mode 100644 index e34341ba8..000000000 --- a/app/views/stats/index.html.erb +++ /dev/null @@ -1,63 +0,0 @@ -
-
-
-

<%= t "admin.stats.show.stats_title" %>

- -
-
-

- <%= t "stats.index.visits" %>
- <%= number_with_delimiter(@visits) %> -

- -

- <%= t "stats.index.debates" %>
- <%= number_with_delimiter(@debates) %> -

-

- <%= t "stats.index.proposals" %>
- <%= number_with_delimiter(@proposals) %> -

-

- <%= t "stats.index.comments" %>
- <%= number_with_delimiter(@comments) %> -

-
- -
-

- <%= t "stats.index.proposal_votes" %>
- <%= number_with_delimiter(@proposal_votes) %>
 -

- -

- <%= t "stats.index.debate_votes" %>
- <%= number_with_delimiter(@debate_votes) %> -

- -

- <%= t "stats.index.comment_votes" %>
- <%= number_with_delimiter(@comment_votes) %> -

- -

- <%= t "stats.index.votes" %>
- <%= number_with_delimiter(@votes) %> -

-
- -
-

- <%= t "stats.index.verified_users" %>
- <%= number_with_delimiter(@verified_users) %> -

- -

- <%= t "stats.index.unverified_users" %>
- <%= number_with_delimiter(@unverified_users) %> -

-
- -
-
-
diff --git a/app/views/stats/index.json.erb b/app/views/stats/index.json.erb deleted file mode 100644 index c2c27702f..000000000 --- a/app/views/stats/index.json.erb +++ /dev/null @@ -1,12 +0,0 @@ -{ - "<%= t("stats.index.visits") %>" : "<%= number_with_delimiter(@visits) %>", - "<%= t("stats.index.debates") %>" : "<%= number_with_delimiter(@debates) %>", - "<%= t("stats.index.proposals") %>" : "<%= number_with_delimiter(@proposals) %>", - "<%= t("stats.index.comments") %>" : "<%= number_with_delimiter(@comments) %>", - "<%= t("stats.index.proposal_votes") %>" : "<%= number_with_delimiter(@proposal_votes) %>", - "<%= t("stats.index.debate_votes") %>" : "<%= number_with_delimiter(@debate_votes) %>", - "<%= t("stats.index.comment_votes") %>" : "<%= number_with_delimiter(@comment_votes) %>", - "<%= t("stats.index.votes") %>" : "<%= number_with_delimiter(@votes) %>", - "<%= t("stats.index.verified_users") %>" : "<%= number_with_delimiter(@verified_users) %>", - "<%= t("stats.index.unverified_users") %>" : "<%= number_with_delimiter(@unverified_users) %>" -} diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index f8c76bb55..755d67391 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -761,18 +761,6 @@ en: youtube: "%{org} YouTube" telegram: "%{org} Telegram" instagram: "%{org} Instagram" - stats: - index: - visits: Visits - debates: Debates - proposals: Proposals - comments: Comments - proposal_votes: Votes on proposals - debate_votes: Votes on debates - comment_votes: Votes on comments - votes: Total votes - verified_users: Verified users - unverified_users: Unverified users unauthorized: default: You do not have permission to access this page. manage: diff --git a/config/locales/en/settings.yml b/config/locales/en/settings.yml index de6059657..d1efc19a6 100644 --- a/config/locales/en/settings.yml +++ b/config/locales/en/settings.yml @@ -116,8 +116,6 @@ en: allow_attached_documents_description: "Allows users to upload documents when creating proposals and investment projects from Participatory Budgets" guides: "Guides to create proposals or investment projects" guides_description: "Displays a guide to differences between proposals and investment projects if there is an active participatory budget" - public_stats: "Public stats" - public_stats_description: "Display public stats in the Administration panel" help_page: "Help page" help_page_description: 'Displays a Help menu that contains a page with an info section about each enabled feature. Also custom pages and menus can be created in the "Custom pages" and "Custom content blocks" sections' remote_translations: "Remote translation" diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index 507ee7701..d8b03580f 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -761,18 +761,6 @@ es: youtube: "YouTube de %{org}" telegram: "Telegram de %{org}" instagram: "Instagram de %{org}" - stats: - index: - visits: Visitas - debates: Debates - proposals: Propuestas ciudadanas - comments: Comentarios - proposal_votes: Votos en propuestas - debate_votes: Votos en debates - comment_votes: Votos en comentarios - votes: Votos - verified_users: Usuarios verificados - unverified_users: Usuarios sin verificar unauthorized: default: No tienes permiso para acceder a esta página. manage: diff --git a/config/locales/es/settings.yml b/config/locales/es/settings.yml index 08d0b3ae8..13261a050 100644 --- a/config/locales/es/settings.yml +++ b/config/locales/es/settings.yml @@ -116,8 +116,6 @@ es: allow_attached_documents_description: "Permite que los usuarios suban documentos al crear propuestas y proyectos de gasto de los Presupuestos participativos" guides: "Guías para crear propuestas o proyectos de inversión" guides_description: "Muestra una guía de diferencias entre las propuestas y los proyectos de gasto si hay un presupuesto participativo activo" - public_stats: "Estadísticas públicas" - public_stats_description: "Muestra las estadísticas públicas en el panel de Administración" help_page: "Página de ayuda" help_page_description: 'Muestra un menú Ayuda que contiene una página con una sección de información sobre cada funcionalidad habilitada. También se pueden crear páginas y menús personalizados en las secciones "Personalizar páginas" y "Personalizar bloques"' remote_translations: "Traducciones remotas" diff --git a/config/routes.rb b/config/routes.rb index 29a4083c6..3d64fe247 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,7 +32,6 @@ Rails.application.routes.draw do get "/consul.json", to: "installation#details" get "robots.txt", to: "robots#index" - resources :stats, only: [:index] resources :images, only: [:destroy] resources :documents, only: [:destroy] resources :follows, only: [:create, :destroy] diff --git a/spec/system/notifications_spec.rb b/spec/system/notifications_spec.rb index e0139659e..c79cf239e 100644 --- a/spec/system/notifications_spec.rb +++ b/spec/system/notifications_spec.rb @@ -164,14 +164,15 @@ describe "Notifications" do end scenario "With internal link" do - admin_notification.update!(link: "/stats") + admin_notification.update!(link: "/debates") visit notifications_path expect(page).to have_content("Notification title") expect(page).to have_content("Notification body") first("#notification_#{notification.id} a").click - expect(page).to have_current_path("/stats") + + expect(page).to have_current_path "/debates" end scenario "Without a link" do diff --git a/spec/system/stats_spec.rb b/spec/system/stats_spec.rb deleted file mode 100644 index 595f737d2..000000000 --- a/spec/system/stats_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -require "rails_helper" - -describe "Stats" do - context "Summary" do - scenario "General" do - create(:debate) - 2.times { create(:proposal) } - 3.times { create(:comment, commentable: Debate.first) } - 4.times { create(:visit) } - - visit stats_path - - expect(page).to have_content "DEBATES\n1" - expect(page).to have_content "PROPOSALS\n2" - expect(page).to have_content "COMMENTS\n3" - expect(page).to have_content "VISITS\n4" - end - - scenario "Votes" do - create(:debate, voters: Array.new(1) { create(:user) }) - create(:proposal, voters: Array.new(2) { create(:user) }) - create(:comment, voters: Array.new(3) { create(:user) }) - - visit stats_path - - expect(page).to have_content "VOTES ON DEBATES\n1" - expect(page).to have_content "VOTES ON PROPOSALS\n2" - expect(page).to have_content "VOTES ON COMMENTS\n3" - expect(page).to have_content "TOTAL VOTES\n6" - end - - scenario "Users" do - 1.times { create(:user, :level_three) } - 2.times { create(:user, :level_two) } - 2.times { create(:user) } - - visit stats_path - - expect(page).to have_content "VERIFIED USERS\n3" - expect(page).to have_content "UNVERIFIED USERS\n2" - end - end -end From d44cff630d99dfdcc22480021fde18823a402eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 8 May 2024 16:08:28 +0200 Subject: [PATCH 18/20] Extract method to build a data source --- app/models/ahoy/chart.rb | 16 +++++++--------- app/models/ahoy/data_source.rb | 4 ++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/models/ahoy/chart.rb b/app/models/ahoy/chart.rb index d2077390d..51a894f65 100644 --- a/app/models/ahoy/chart.rb +++ b/app/models/ahoy/chart.rb @@ -26,19 +26,17 @@ module Ahoy end def self.active_events_data_points - ds = Ahoy::DataSource.new - - active_event_names.map { |event_name| new(event_name) }.each do |chart| - ds.add chart.title, chart.data + Ahoy::DataSource.build do |data_source| + active_event_names.map { |event_name| new(event_name) }.each do |chart| + data_source.add chart.title, chart.data + end end - - ds.build end def data_points - ds = Ahoy::DataSource.new - ds.add title, data - ds.build + Ahoy::DataSource.build do |data_source| + data_source.add title, data + end end def title diff --git a/app/models/ahoy/data_source.rb b/app/models/ahoy/data_source.rb index a29902593..f67bc2888 100644 --- a/app/models/ahoy/data_source.rb +++ b/app/models/ahoy/data_source.rb @@ -4,6 +4,10 @@ module Ahoy class DataSource + def self.build(&block) + new.tap { |data_source| block.call(data_source) }.build + end + # Adds a collection with the datasource # Name is the name of the collection and will be showed in the # chart From 86e131df26d0f43e6f77f9b322bfa3bd7a20205a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 8 May 2024 17:44:16 +0200 Subject: [PATCH 19/20] Make it more obvious that we're using dates in charts The `shared_keys` and `k` variable names could mean anything, so it wasn't clear what these variables contained. --- app/models/ahoy/data_source.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/ahoy/data_source.rb b/app/models/ahoy/data_source.rb index f67bc2888..1041c159b 100644 --- a/app/models/ahoy/data_source.rb +++ b/app/models/ahoy/data_source.rb @@ -13,19 +13,19 @@ module Ahoy # chart def add(name, collection) collections.push data: collection, name: name - shared_keys.merge(collection.keys) + dates.merge(collection.keys) end def build data = { x: [] } - shared_keys.each do |k| + dates.each do |date| # Add the key with a valid date format - data[:x].push k.strftime("%Y-%m-%d") + data[:x].push date.strftime("%Y-%m-%d") # Add the value for each column, or 0 if not present collections.each do |col| data[col[:name]] ||= [] - count = col[:data][k] || 0 + count = col[:data][date] || 0 data[col[:name]].push count end end @@ -39,8 +39,8 @@ module Ahoy @collections ||= [] end - def shared_keys - @shared_keys ||= Set.new + def dates + @dates ||= Set.new end end end From 3fcac3a9d8cf13ca070413f70b2891c4843db7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 8 May 2024 21:47:37 +0200 Subject: [PATCH 20/20] Sort data by date when building graphs This doesn't affect the end result because all collections used the same order, but it makes debugging easier. --- app/models/ahoy/data_source.rb | 2 +- spec/models/ahoy/data_source_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/ahoy/data_source.rb b/app/models/ahoy/data_source.rb index 1041c159b..6ca294365 100644 --- a/app/models/ahoy/data_source.rb +++ b/app/models/ahoy/data_source.rb @@ -18,7 +18,7 @@ module Ahoy def build data = { x: [] } - dates.each do |date| + dates.sort.each do |date| # Add the key with a valid date format data[:x].push date.strftime("%Y-%m-%d") diff --git a/spec/models/ahoy/data_source_spec.rb b/spec/models/ahoy/data_source_spec.rb index 2766d1391..185ba6a11 100644 --- a/spec/models/ahoy/data_source_spec.rb +++ b/spec/models/ahoy/data_source_spec.rb @@ -25,5 +25,15 @@ describe Ahoy::DataSource do "foo" => [2, 1, 0], "bar" => [1, 0, 2] end + + it "returns data ordered by dates" do + ds = Ahoy::DataSource.new + ds.add "foo", { january_third => 2, january_second => 1 } + ds.add "bar", { january_first => 2, january_second => 1 } + + expect(ds.build).to eq :x => ["2015-01-01", "2015-01-02", "2015-01-03"], + "foo" => [0, 1, 2], + "bar" => [2, 1, 0] + end end end