Skip to content

Commit

Permalink
Guard for nil GA profile in FileDownloadStat
Browse files Browse the repository at this point in the history
Fixes #2758
Removed AnalyticsDate mixin and added a parent class:
Sufia::StatsUsagePresenter
  • Loading branch information
jcoyne committed Sep 30, 2016
1 parent f717172 commit 8d200d9
Show file tree
Hide file tree
Showing 17 changed files with 219 additions and 163 deletions.
2 changes: 2 additions & 0 deletions .codeclimate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ engines:
exclude_fingerprints:
- 6ad4464dbb2a999591c7be8346dc137c3372b280f4a8b0c024fef91dfebeeb83
- c05c1baf940f00a565ffe12f8b8a213fa06cebd734e0dae92057db28dc7c018b
- b388847a782f9d55d164f632b892125496082b9cfe5813720327ae6afbaf607b
- c7c241833ba150c2e4c72a26e1ac4d674230cb5e5c87322bedfe2b50dd09247d
duplication:
enabled: true
exclude_fingerprints:
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/stats_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ class StatsController < ApplicationController
before_action :build_breadcrumbs, only: [:work, :file]

def work
@stats = WorkUsage.new(params[:id])
@stats = Sufia::WorkUsage.new(params[:id])
end

def file
@stats = FileUsage.new(params[:id])
@stats = Sufia::FileUsage.new(params[:id])
end

protected
Expand Down
25 changes: 18 additions & 7 deletions app/models/file_download_stat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@ class FileDownloadStat < Sufia::Statistic
self.cache_column = :downloads
self.event_type = :totalEvents

# Sufia::Download is sent to Sufia::Analytics.profile as #sufia__download
# see Legato::ProfileMethods.method_name_from_klass
def self.ga_statistics(start_date, file)
Sufia::Analytics.profile.sufia__download(sort: 'date', start_date: start_date, end_date: Date.yesterday).for_file(file.id)
end
class << self
# Sufia::Download is sent to Sufia::Analytics.profile as #sufia__download
# see Legato::ProfileMethods.method_name_from_klass
def ga_statistics(start_date, file)
profile = Sufia::Analytics.profile
unless profile
Rails.logger.error("Google Analytics profile has not been established. Unable to fetch statistics.")
return []
end
profile.sufia__download(sort: 'date',
start_date: start_date,
end_date: Date.yesterday)
.for_file(file.id)
end

def self.filter(file)
{ file_id: file.id }
# this is called by the parent class
def filter(file)
{ file_id: file.id }
end
end
end
7 changes: 5 additions & 2 deletions app/models/file_view_stat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ class FileViewStat < Sufia::Statistic
self.cache_column = :views
self.event_type = :pageviews

def self.filter(file)
{ file_id: file.id }
class << self
# this is called by the parent class
def filter(file)
{ file_id: file.id }
end
end
end
4 changes: 0 additions & 4 deletions app/models/sufia/statistic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ def build_for(object, attrs)
new attrs.merge(filter(object))
end

def to_flots(stats)
stats.map(&:to_flot)
end

def convert_date(date_time)
date_time.to_datetime.to_i * 1000
end
Expand Down
21 changes: 0 additions & 21 deletions app/presenters/analytics_date.rb

This file was deleted.

33 changes: 0 additions & 33 deletions app/presenters/file_usage.rb

This file was deleted.

37 changes: 37 additions & 0 deletions app/presenters/sufia/file_usage.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Called by the stats controller, it finds cached file pageview data,
# and prepares it for visualization in /app/views/stats/file.html.erb
module Sufia
class FileUsage < StatsUsagePresenter
def initialize(id)
self.model = ::FileSet.find(id)
end

alias file model

def total_downloads
downloads.reduce(0) { |total, result| total + result[1].to_i }
end

def total_pageviews
pageviews.reduce(0) { |total, result| total + result[1].to_i }
end

# Package data for visualization using JQuery Flot
def to_flot
[
{ label: "Pageviews", data: pageviews },
{ label: "Downloads", data: downloads }
]
end

private

def downloads
to_flots(FileDownloadStat.statistics(model, created, user_id))
end

def pageviews
to_flots(FileViewStat.statistics(model, created, user_id))
end
end
end
43 changes: 43 additions & 0 deletions app/presenters/sufia/stats_usage_presenter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
module Sufia
# Methods used by both WorkUsage and FileUsage
class StatsUsagePresenter
attr_accessor :id, :model

def created
@created ||= date_for_analytics
end

private

def user_id
@user_id ||= begin
user = Hydra::Ability.user_class.find_by_user_key(model.depositor)
user ? user.id : nil
end
end

# TODO: make this a lazy enumerator
def to_flots(stats)
stats.map(&:to_flot)
end

# model.date_uploaded reflects the date the object was uploaded by the user
# and therefore (if available) the date that we want to use for the stats
# model.create_date reflects the date the file was added to Fedora. On data
# migrated from one repository to another the created_date can be later
# than the date the file was uploaded.
def date_for_analytics
earliest = Sufia.config.analytic_start_date
date_uploaded = string_to_date(model.date_uploaded)
date_analytics = date_uploaded ? date_uploaded : model.create_date
return date_analytics if earliest.blank?
earliest > date_analytics ? earliest : date_analytics
end

def string_to_date(date_str)
Time.zone.parse(date_str)
rescue ArgumentError, TypeError
nil
end
end
end
30 changes: 30 additions & 0 deletions app/presenters/sufia/work_usage.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# class WorkUsage follows the model established by FileUsage
# Called by the stats controller, it finds cached work pageview data,
# and prepares it for visualization in /app/views/stats/work.html.erb
module Sufia
class WorkUsage < StatsUsagePresenter
def initialize(id)
self.model = CurationConcerns::WorkRelation.new.find(id)
end

alias work model
delegate :to_s, to: :model

def total_pageviews
pageviews.reduce(0) { |total, result| total + result[1].to_i }
end

# Package data for visualization using JQuery Flot
def to_flot
[
{ label: "Pageviews", data: pageviews }
]
end

private

def pageviews
to_flots WorkViewStat.statistics(model, created, user_id)
end
end
end
30 changes: 0 additions & 30 deletions app/presenters/work_usage.rb

This file was deleted.

4 changes: 2 additions & 2 deletions spec/controllers/stats_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
end

it 'renders the stats view' do
expect(FileUsage).to receive(:new).with(file_set.id).and_return(usage)
expect(Sufia::FileUsage).to receive(:new).with(file_set.id).and_return(usage)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.dashboard.title'), Sufia::Engine.routes.url_helpers.dashboard_index_path)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.dashboard.my.works'), Sufia::Engine.routes.url_helpers.dashboard_works_path)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.file_set.browse_view'), Rails.application.routes.url_helpers.curation_concerns_file_set_path(file_set))
Expand Down Expand Up @@ -56,7 +56,7 @@
end

it 'renders the stats view' do
expect(WorkUsage).to receive(:new).with(work.id).and_return(usage)
expect(Sufia::WorkUsage).to receive(:new).with(work.id).and_return(usage)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.dashboard.my.works'), Sufia::Engine.routes.url_helpers.dashboard_works_path)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.dashboard.title'), Sufia::Engine.routes.url_helpers.dashboard_index_path)
expect(controller).to receive(:add_breadcrumb).with(I18n.t('sufia.work.browse_view'), main_app.curation_concerns_generic_work_path(work))
Expand Down
31 changes: 27 additions & 4 deletions spec/models/file_download_stat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,29 @@
expect(file_stat.downloads).to eq(2)
end

describe ".ga_statistic" do
let(:start_date) { 2.days.ago }
let(:expected_path) { Rails.application.routes.url_helpers.curation_concerns_file_set_path(file) }
before do
allow(Sufia::Analytics).to receive(:profile).and_return(profile)
end
context "when a profile is available" do
let(:views) { double }
let(:profile) { double(sufia__download: views) }
it "calls the Legato method with the correct path" do
expect(views).to receive(:for_file).with(99)
described_class.ga_statistics(start_date, file)
end
end

context "when a profile not available" do
let(:profile) { nil }
it "calls the Legato method with the correct path" do
expect(described_class.ga_statistics(start_date, file)).to be_empty
end
end
end

describe "#statistics" do
let(:dates) do
ldates = []
Expand Down Expand Up @@ -48,17 +71,17 @@
end

it "includes cached ga data" do
expect(described_class.to_flots(stats)).to include(*download_output)
expect(stats.map(&:to_flot)).to include(*download_output)
end

it "caches data" do
expect(described_class.to_flots(stats)).to include(*download_output)
expect(stats.map(&:to_flot)).to include(*download_output)

# at this point all data should be cached
allow(described_class).to receive(:ga_statistics).with(Time.zone.today, file).and_raise("We should not call Google Analytics All data should be cached!")

stats2 = described_class.statistics(file, Time.zone.today - 4.days)
expect(described_class.to_flots(stats2)).to include(*download_output)
expect(stats2.map(&:to_flot)).to include(*download_output)
end
end

Expand All @@ -71,7 +94,7 @@
end

it "includes cached data" do
expect(described_class.to_flots(stats)).to include([file_download_stat.date.to_i * 1000, file_download_stat.downloads], *download_output)
expect(stats.map(&:to_flot)).to include([file_download_stat.date.to_i * 1000, file_download_stat.downloads], *download_output)
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/models/file_view_stat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,18 @@
end

it "includes cached ga data" do
expect(described_class.to_flots(stats)).to include(*view_output)
expect(stats.map(&:to_flot)).to include(*view_output)
end

it "caches data" do
expect(described_class.to_flots(stats)).to include(*view_output)
expect(stats.map(&:to_flot)).to include(*view_output)
expect(stats.first.user_id).to eq user_id

# at this point all data should be cached
allow(described_class).to receive(:ga_statistics).with(Time.zone.today, file).and_raise("We should not call Google Analytics All data should be cached!")

stats2 = described_class.statistics(file, Time.zone.today - 5.days)
expect(described_class.to_flots(stats2)).to include(*view_output)
expect(stats2.map(&:to_flot)).to include(*view_output)
end
end

Expand All @@ -73,7 +73,7 @@
end

it "includes cached data" do
expect(described_class.to_flots(stats)).to include([file_view_stat.date.to_i * 1000, file_view_stat.views], *view_output)
expect(stats.map(&:to_flot)).to include([file_view_stat.date.to_i * 1000, file_view_stat.views], *view_output)
end
end
end
Expand Down
Loading

0 comments on commit 8d200d9

Please sign in to comment.