From c5807205b5151c8f64f0ffab67521343e1d93049 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 13 Nov 2018 20:37:29 -0500 Subject: [PATCH] Adjust dashboard analytics to be more reliable (#1115) * Update analytics * Remove analytics file * Remove unused files * Add delete button back in on dashboard --- app/assets/javascripts/initializePage.js.erb | 1 - .../initializers/initializeAnalytics.js | 32 ------- app/assets/stylesheets/dashboard.scss | 69 +++++++------- app/black_box/google_analytics.rb | 4 +- app/controllers/analytics_controller.rb | 58 ------------ app/controllers/async_info_controller.rb | 4 +- app/controllers/dashboards_controller.rb | 7 +- app/labor/article_analytics_fetcher.rb | 34 +++++++ app/views/dashboards/_analytics.html.erb | 18 ++++ .../dashboards/_dashboard_article.html.erb | 22 +++++ app/views/dashboards/show.html.erb | 91 +++++-------------- config/routes.rb | 5 +- spec/requests/analytics_spec.rb | 53 ----------- 13 files changed, 149 insertions(+), 249 deletions(-) delete mode 100644 app/assets/javascripts/initializers/initializeAnalytics.js delete mode 100644 app/controllers/analytics_controller.rb create mode 100644 app/labor/article_analytics_fetcher.rb create mode 100644 app/views/dashboards/_analytics.html.erb create mode 100644 app/views/dashboards/_dashboard_article.html.erb delete mode 100644 spec/requests/analytics_spec.rb diff --git a/app/assets/javascripts/initializePage.js.erb b/app/assets/javascripts/initializePage.js.erb index 575dc4b3785d..ff7d34f945ea 100644 --- a/app/assets/javascripts/initializePage.js.erb +++ b/app/assets/javascripts/initializePage.js.erb @@ -32,7 +32,6 @@ function callInitalizers(){ initNotifications(); initializeSplitTestTracking(); initializeStylesheetAppend(); - initializeAnalytics(); initializeCommentDropdown(); initializeSettings(); initializeFooterMod(); diff --git a/app/assets/javascripts/initializers/initializeAnalytics.js b/app/assets/javascripts/initializers/initializeAnalytics.js deleted file mode 100644 index aaff7ec04dab..000000000000 --- a/app/assets/javascripts/initializers/initializeAnalytics.js +++ /dev/null @@ -1,32 +0,0 @@ -function initializeAnalytics() { - if (getCurrentPage("dashboards-show")) { - var els = document.querySelectorAll('[data-analytics-pageviews]'); - if (els.length === 0) { return; } - var ids = []; - for(var i = 0; i < els.length; i++) { - ids.push(els[i].dataset.articleId) - } - // .map(function() {return this.dataset.reactableId}); - window.fetch('/analytics?article_ids='+ids.join(","), {headers: {Accept: 'application/json'}, credentials: 'same-origin'}) - .then(function(response) { - response.json().then(function(json) { - var total = 0; - for (var k in json){ - total = total + Number(json[k]); - var numString = parseInt(json[k]) < 100 ? "< 100 " : numberWithCommas(json[k]) ; - var totalString = parseInt(total) < 100 ? "< 100 " : numberWithCommas(total) ; - document.getElementById("pageviews-"+k).innerHTML = numString + " Views"; - document.getElementById("pageviews-"+k).classList.add("loaded"); - document.getElementById("dashboard-analytics").innerHTML = totalString+ " Total Views"; - } - document.getElementById("dashboard-analytics-header").classList.add("loaded"); - }); - }).catch(function(err) { - console.log(err); - }); - } -} - -function numberWithCommas(x) { - return x.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ","); -} diff --git a/app/assets/stylesheets/dashboard.scss b/app/assets/stylesheets/dashboard.scss index 0a30ef3a5e04..2c49b176ff32 100644 --- a/app/assets/stylesheets/dashboard.scss +++ b/app/assets/stylesheets/dashboard.scss @@ -6,18 +6,21 @@ max-width:96%; margin:auto; h1{ - font-size:30px; + font-size:22px; font-family: $helvetica-condensed; font-stretch:condensed; padding-bottom: 20px; a{ - padding: 0px 20px; - border-radius: 3px; + padding: 6px 25px; + border-radius: 100px; margin-right: 18px; display: inline-block; color: $black; + &:hover{ + background: lighten($light-medium-gray, 3%); + } &.active{ - background: $black; + background: $dark-gray; color:white; } } @@ -70,39 +73,49 @@ .dashboard-analytics-header-wrapper{ text-align: center; margin-bottom: 28px; + background: $dark-gray; + box-shadow: $bold-shadow; + padding: 15px 0px; } .dashboard-analytics-header{ - color:$black; + color: white; margin:5px auto; - font-size:calc(0.77em + 0.22vw); - padding: 20px 0px; + font-size:calc(1.1em + 0.1vw); + padding: 18px 0px 7px; padding-left: 10px; text-align: left; font-family: $monospace; - border-radius: 200px; - border: 1px solid $light-medium-gray; - box-shadow: $bold-shadow; + // border-radius: 200px; + border: 1px solid $light-gray; + // box-shadow: $bold-shadow; display: inline-block; width: 90%; + text-align: center; margin: 1.1%; @media screen and ( min-width: 800px ) { width: 43%; margin: 1.2%; } span{ - opacity:0.3; + font-size: 1.8em; + font-weight: bold; + margin-left: 8px; + display: inline-block; } img{ - width: calc(0.9em + 1.2vw); - height: calc(0.9em + 1.2vw); - margin-right: 0.4em; - margin-left: 1.1em; - vertical-align: calc(-0.6vw - 2px); - } - &.loaded{ - span{ - opacity:1; - } + width: calc(1.2em + 0.2vw); + height: calc(1.2em + 0.2vw); + margin-left: 0.4em; + vertical-align: calc(-0.2vw - 0.3em); + background: $sky-blue; + border-radius: 100px; + border: 2px solid darken($light-gray, 7%); + } + .dashboard-analytics-sub-indicator{ + color: darken($light-gray, 7%); + padding: 10px 0px; + font-weight: bold; + font-size:0.66em; } } @@ -152,7 +165,7 @@ color:rgb(67, 78, 87); } .dashboard-actions{ - padding: 8px 0px; + padding: 5px 0px; } .pill{ background:$bold-blue; @@ -173,22 +186,16 @@ background:$red; } &.black{ - background: $black; + background: $dark-gray; } } .dashboard-pageviews-indicator{ display:inline-block; font-family: $monospace; - color: $black; + color: $dark-gray; background: white; text-align:center; - opacity:0.4; - height: 23px; - line-height: 22px; padding: 2px 8px; - &.loaded{ - opacity:1; - } } form{ padding: 10px 0px; @@ -241,4 +248,4 @@ min-height: 350px; } } -} +} \ No newline at end of file diff --git a/app/black_box/google_analytics.rb b/app/black_box/google_analytics.rb index 2c88af89b36f..6a32c6bfbec7 100644 --- a/app/black_box/google_analytics.rb +++ b/app/black_box/google_analytics.rb @@ -72,8 +72,6 @@ def fetch_analytics_for(*report_requests) grr = GetReportsRequest.new(report_requests: report_requests, quota_user: @user_id.to_s) response = @client.batch_get_reports(grr) response.reports.map do |report| - p report.data - p report.data (report.data.maximums || report.data.totals)[0].values[0] end end @@ -84,4 +82,4 @@ def create_service_account_credential scope: [AUTH_ANALYTICS_READONLY], ) end -end +end \ No newline at end of file diff --git a/app/controllers/analytics_controller.rb b/app/controllers/analytics_controller.rb deleted file mode 100644 index 0300813acf01..000000000000 --- a/app/controllers/analytics_controller.rb +++ /dev/null @@ -1,58 +0,0 @@ -class AnalyticsController < ApplicationController - caches_action :index, - cache_path: Proc.new { "#{request.params}___#{current_user.id}" }, - expires_in: 12.minutes - after_action :verify_authorized - - def index - @article_ids = analytics_params.split(",") - articles_to_check = Article.where(id: @article_ids) - authorize articles_to_check, :analytics_index? - - qualified_articles = get_articles_that_qualify(articles_to_check) - fetch_and_update_page_views_and_reaction_counts(qualified_articles) - - render_finalized_json - end - - def get_articles_that_qualify(articles_to_check) - qualified_articles = [] - articles_to_check.each do |article| - if should_fetch(article) - qualified_articles << article - end - end - qualified_articles - end - - def fetch_and_update_page_views_and_reaction_counts(qualified_articles) - qualified_articles.each_slice(25).to_a.each do |chunk| - pageviews = GoogleAnalytics.new(chunk.pluck(:id), current_user.id).get_pageviews - page_views_obj = pageviews.to_h - chunk.each do |article| - next if article.page_views_count > page_views_obj[article.id].to_i - article.update_columns(page_views_count: page_views_obj[article.id].to_i, - previous_positive_reactions_count: article.positive_reactions_count) - end - end - end - - def render_finalized_json - finalized_object = {} - Article.where(id: @article_ids). - pluck(:id, :page_views_count).map { |a| finalized_object[a[0]] = a[1].to_s } - render json: finalized_object.to_json - end - - def should_fetch(article) - new_reactions = (article.positive_reactions_count > article.previous_positive_reactions_count) - random = (rand(80) == 1) - new_reactions || random - end - - private - - def analytics_params - params.require(:article_ids) - end -end diff --git a/app/controllers/async_info_controller.rb b/app/controllers/async_info_controller.rb index 1227264afbe1..8a14d3cafef9 100644 --- a/app/controllers/async_info_controller.rb +++ b/app/controllers/async_info_controller.rb @@ -17,6 +17,8 @@ def base_data remember_me(current_user) end @user = current_user.decorate + # Updates article analytics periodically: + ArticleAnalyticsFetcher.new.delay.update_analytics(@user.id) if rand(20) == 1 respond_to do |format| format.json do render json: { @@ -60,4 +62,4 @@ def user_cache_key #{current_user&.articles_count}__ #{cookies[:remember_user_token]}" end -end +end \ No newline at end of file diff --git a/app/controllers/dashboards_controller.rb b/app/controllers/dashboards_controller.rb index 915082532b08..b5cf9cda6127 100644 --- a/app/controllers/dashboards_controller.rb +++ b/app/controllers/dashboards_controller.rb @@ -16,10 +16,15 @@ def show elsif params[:which] == "user_followers" @follows = Follow.where(followable_id: @user.id, followable_type: "User"). includes(:follower).order("created_at DESC").limit(80) + elsif params[:which] == "organization_user_followers" + @follows = Follow.where(followable_id: @user.organization_id, followable_type: "Organization"). + includes(:follower).order("created_at DESC").limit(80) elsif @user&.organization && @user&.org_admin && params[:which] == "organization" @articles = @user.organization.articles.order("created_at DESC").decorate elsif @user @articles = @user.articles.order("created_at DESC").decorate end + # Updates analytics in background if appropriate: + ArticleAnalyticsFetcher.new.delay.update_analytics(current_user.id) if @articles end -end +end \ No newline at end of file diff --git a/app/labor/article_analytics_fetcher.rb b/app/labor/article_analytics_fetcher.rb new file mode 100644 index 000000000000..8982dd8d7710 --- /dev/null +++ b/app/labor/article_analytics_fetcher.rb @@ -0,0 +1,34 @@ +class ArticleAnalyticsFetcher + def update_analytics(user_id) + articles_to_check = Article.where(user_id: user_id, published: true) + qualified_articles = get_articles_that_qualify(articles_to_check) + return if qualified_articles.none? + fetch_and_update_page_views_and_reaction_counts(qualified_articles, user_id) + end + + def fetch_and_update_page_views_and_reaction_counts(qualified_articles, user_id) + qualified_articles.each_slice(15).to_a.each do |chunk| + pageviews = GoogleAnalytics.new(chunk.pluck(:id), user_id).get_pageviews + page_views_obj = pageviews.to_h + chunk.each do |article| + article.update_columns(previous_positive_reactions_count: article.positive_reactions_count) + next if article.page_views_count > page_views_obj[article.id].to_i + article.update_columns(page_views_count: page_views_obj[article.id].to_i) + end + end + end + + def get_articles_that_qualify(articles_to_check) + qualified_articles = [] + articles_to_check.each do |article| + if should_fetch(article) + qualified_articles << article + end + end + qualified_articles + end + + def should_fetch(article) + article.positive_reactions_count > article.previous_positive_reactions_count + end +end \ No newline at end of file diff --git a/app/views/dashboards/_analytics.html.erb b/app/views/dashboards/_analytics.html.erb new file mode 100644 index 000000000000..a088c234babc --- /dev/null +++ b/app/views/dashboards/_analytics.html.erb @@ -0,0 +1,18 @@ +<% num_views = @articles.sum(&:page_views_count) %> + +
+
+ <%= number_with_delimiter(@articles.sum(&:positive_reactions_count), delimeter: ",") %> +
+ " />" />" /> + Total Post Reactions +
+
+
+ <%= num_views > 500 ? number_with_delimiter(num_views, delimeter: ",") : "under 500" %> +
+ " /> + Total Post Views +
+
+
\ No newline at end of file diff --git a/app/views/dashboards/_dashboard_article.html.erb b/app/views/dashboards/_dashboard_article.html.erb new file mode 100644 index 000000000000..9a223b424737 --- /dev/null +++ b/app/views/dashboards/_dashboard_article.html.erb @@ -0,0 +1,22 @@ +
+

<%= article.title %>

 +
+ <% if !article.published %> + DRAFT + <% end %> + EDIT + DELETE + <% if article.published && current_user.can_view_analytics? %> + + <%= article.page_views_count > 100 ? article.page_views_count : "under 100" %> views // <%= article.positive_reactions_count %> reactions + + <% end %> +
+ <% if org_admin %> + <%= form_for(article) do |f| %> + + AUTHOR: <%= f.select(:user_id, options_for_select(@user.organization.users.map {|x| [x.name, x.id] }, article.user_id )) %> + <%= f.submit "SUBMIT AUTHOR CHANGE", class:"cta pill black" %> + <% end %> + <% end %> +
\ No newline at end of file diff --git a/app/views/dashboards/show.html.erb b/app/views/dashboards/show.html.erb index 47ca3c1dac4e..ffc6acab97ae 100644 --- a/app/views/dashboards/show.html.erb +++ b/app/views/dashboards/show.html.erb @@ -6,7 +6,7 @@ POSTS (<%= @user.articles_count %>) - " href="/dashboard/user_followers"> + " href="/dashboard/user_followers"> FOLLOWERS (<%= @user.followers_count %>) @@ -15,66 +15,43 @@ (<%= @user.following_users_count %>) -<% if @user.org_admin && @user.organization %> +<% if @user.org_admin && @user.organization && (params[:which].blank? || params[:which] == "organization") %>

>MY POSTS - ><%= @user.organization.name.upcase %> POSTS + ><%= @user.organization.name.upcase %> POSTS (<%= @user.organization.articles.size %>) +

+<% elsif @user.org_admin && @user.organization && (params[:which] == "organization_user_followers" || params[:which] == "user_followers")%> +

+ >MY FOLLOWERS + ><%= @user.organization.name.upcase %> FOLLOWERS (<%= @user.organization.followers_count %>)

<% end %> <% if @user.org_admin && @user.organization && params[:which] == "organization" %> <% if current_user.can_view_analytics? %> -
-
- " /><%= number_with_delimiter(@articles.sum(&:positive_reactions_count), delimeter: ",") %> Total Post Reactions -
-
- " />Loading Views Data... -
-
+ <%= render "analytics" %> <% end %> <% @articles.each do |article| %> -
-

<%= article.title %>

 -
- <% if !article.published %> - DRAFT - <% end %> - EDIT - <% if article.published && current_user.can_view_analytics? %> - Loading View Data... - <% end %> -
- <%= form_for(article) do |f| %> - - AUTHOR: <%= f.select(:user_id, options_for_select(@user.organization.users.map {|x| [x.name, x.id] }, article.user_id )) %> - <%= f.submit "SUBMIT AUTHOR CHANGE", class:"cta pill black" %> - <% end %> -
+ <%= render "dashboard_article", article: article, org_admin: true %> <% end %> - <% elsif params[:which] == "user_followers" || params[:which] == "following_users" %> + <% elsif params[:which] == "user_followers" || params[:which] == "following_users" || params[:which] == "organization_user_followers" %> <% @follows.each do |follow| %> - <% user = params[:which] == "user_followers" ? follow.follower : follow.followable %> -
- -

- <%= user.username %> profile image - <%= user.name %> - @<%= user.username %> -

- -
+ <% user = params[:which].include?("followers") ? follow.follower : follow.followable %> + <% if user %> +
+ +

+ <%= user.username %> profile image + <%= user.name %> + @<%= user.username %> +

+ +
+ <% end %> <% end %> <% elsif @articles.any? %> <% if current_user.can_view_analytics? %> -
-
- " /><%= number_with_delimiter(@articles.sum(&:positive_reactions_count), delimeter: ",") %> Total Post Reactions -
-
- " />Loading Views Data... -
-
+ <%= render "analytics" %> <% end %> <% if current_user.has_role?(:video_permission) %> @@ -82,25 +59,7 @@ <% end %> <% @articles.each do |article| %> -
- -

- <%= article.title %> -

- -
- <% if article.published %> - PUBLISHED - <% else %> - DRAFT - <% end %> - EDIT - DELETE - <% if article.published && current_user.can_view_analytics? %> - Loading View Data... - <% end %> -
-
+ <%= render "dashboard_article", article: article, org_admin: false %> <% end %> <% else %>
diff --git a/config/routes.rb b/config/routes.rb index f02c16b34e8d..10b716c376b2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,7 +107,6 @@ resources :blocks resources :notifications, only: [:index] resources :tags, only: [:index] - resources :analytics, only: [:index] resources :stripe_subscriptions, only: %i[create update destroy] resources :stripe_active_cards, only: %i[create update destroy] resources :live_articles, only: [:index] @@ -237,7 +236,7 @@ get "/dashboard" => "dashboards#show" get "/dashboard/:which" => "dashboards#show", constraints: { - which: /organization|user_followers|following_users|reading/ + which: /organization|organization_user_followers|user_followers|following_users|reading/ } get "/dashboard/:username" => "dashboards#show" @@ -299,4 +298,4 @@ root "stories#index" end -# rubocop:enable Metrics/BlockLength +# rubocop:enable Metrics/BlockLength \ No newline at end of file diff --git a/spec/requests/analytics_spec.rb b/spec/requests/analytics_spec.rb deleted file mode 100644 index 5dd5f9be625b..000000000000 --- a/spec/requests/analytics_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require "rails_helper" - -vcr_option = { - cassette_name: "google_api_request_spec" -} - -RSpec.describe "Analytics", type: :request, vcr: vcr_option do - describe "GET /analytics" do - context "when signed in as an authorized user" do - let(:user) { create(:user, :analytics) } - let(:article1) { create(:article, user_id: user.id) } - let(:article2) { create(:article, user_id: user.id) } - let(:ga_double) { instance_double(GoogleAnalytics) } - - before do - allow(GoogleAnalytics).to receive(:new).and_return(ga_double) - allow(ga_double).to receive(:create_service_account_credential).and_return({}) - allow(ga_double).to receive(:get_pageviews) do - { article1.id.to_s => "0", article2.id.to_s => "0" } - end - login_as user - end - - it "raise ParameterMissing if no proper params is given" do - expect { get "/analytics" }.to raise_error ActionController::ParameterMissing - end - - it "returns pageviews" do - get "/analytics?article_ids=#{article1.id},#{article2.id}" - expect(JSON.parse(response.body)).to eq(article1.id.to_s => "0", article2.id.to_s => "0") - end - - it "returns pageviews for super_admins" do - user.remove_role :analytics_beta_tester - user.add_role :super_admin - get "/analytics?article_ids=#{article1.id},#{article2.id}" - expect(JSON.parse(response.body)).to eq(article1.id.to_s => "0", article2.id.to_s => "0") - end - - it "updates article view counts" do - Reaction.create!( - user_id: user.id, - reactable_id: article1.id, - reactable_type: "Article", - category: "readinglist", - ) - expect(article1.reload.previous_positive_reactions_count).not_to eq(article1.positive_reactions_count) - get "/analytics?article_ids=#{article1.id},#{article2.id}" - expect(article1.reload.previous_positive_reactions_count).to eq(article1.positive_reactions_count) - end - end - end -end