diff --git a/.codeclimate.yml b/.codeclimate.yml index 520b445b81..79be5c2d40 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -5,6 +5,8 @@ engines: exclude_fingerprints: - 6ad4464dbb2a999591c7be8346dc137c3372b280f4a8b0c024fef91dfebeeb83 - c05c1baf940f00a565ffe12f8b8a213fa06cebd734e0dae92057db28dc7c018b + - b388847a782f9d55d164f632b892125496082b9cfe5813720327ae6afbaf607b + - c7c241833ba150c2e4c72a26e1ac4d674230cb5e5c87322bedfe2b50dd09247d duplication: enabled: true exclude_fingerprints: diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 5352ccaed5..b05185f23c 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -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 diff --git a/app/models/file_download_stat.rb b/app/models/file_download_stat.rb index 13447f12da..f92e92d0e3 100644 --- a/app/models/file_download_stat.rb +++ b/app/models/file_download_stat.rb @@ -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 diff --git a/app/models/file_view_stat.rb b/app/models/file_view_stat.rb index 6ad7cd576b..a595a21be8 100644 --- a/app/models/file_view_stat.rb +++ b/app/models/file_view_stat.rb @@ -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 diff --git a/app/models/sufia/statistic.rb b/app/models/sufia/statistic.rb index f04b5230bf..4479c433cb 100644 --- a/app/models/sufia/statistic.rb +++ b/app/models/sufia/statistic.rb @@ -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 diff --git a/app/presenters/analytics_date.rb b/app/presenters/analytics_date.rb deleted file mode 100644 index 016785a6ad..0000000000 --- a/app/presenters/analytics_date.rb +++ /dev/null @@ -1,21 +0,0 @@ -# This module wraps up two methods used by both WorkUsage and FileUsage -module AnalyticsDate - # object.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 - # object.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(object) - earliest = Sufia.config.analytic_start_date - date_uploaded = string_to_date(object.date_uploaded) - date_analytics = date_uploaded ? date_uploaded : object.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 diff --git a/app/presenters/file_usage.rb b/app/presenters/file_usage.rb deleted file mode 100644 index 3739e58615..0000000000 --- a/app/presenters/file_usage.rb +++ /dev/null @@ -1,33 +0,0 @@ -# Called by the stats controller, it finds cached file pageview data, -# and prepares it for visualization in /app/views/stats/file.html.erb -class FileUsage - include AnalyticsDate - attr_accessor :id, :created, :downloads, :pageviews - - def initialize(id) - file = ::FileSet.find(id) - user = User.find_by(email: file.depositor) - user_id = user ? user.id : nil - - self.id = id - self.created = date_for_analytics(file) - self.downloads = FileDownloadStat.to_flots FileDownloadStat.statistics(file, created, user_id) - self.pageviews = FileViewStat.to_flots FileViewStat.statistics(file, created, user_id) - end - - 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 -end diff --git a/app/presenters/sufia/file_usage.rb b/app/presenters/sufia/file_usage.rb new file mode 100644 index 0000000000..ddce636c98 --- /dev/null +++ b/app/presenters/sufia/file_usage.rb @@ -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 diff --git a/app/presenters/sufia/stats_usage_presenter.rb b/app/presenters/sufia/stats_usage_presenter.rb new file mode 100644 index 0000000000..2dacd84a50 --- /dev/null +++ b/app/presenters/sufia/stats_usage_presenter.rb @@ -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 diff --git a/app/presenters/sufia/work_usage.rb b/app/presenters/sufia/work_usage.rb new file mode 100644 index 0000000000..23b9e23dba --- /dev/null +++ b/app/presenters/sufia/work_usage.rb @@ -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 diff --git a/app/presenters/work_usage.rb b/app/presenters/work_usage.rb deleted file mode 100644 index d167e2a136..0000000000 --- a/app/presenters/work_usage.rb +++ /dev/null @@ -1,30 +0,0 @@ -# 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 -class WorkUsage - include AnalyticsDate - attr_accessor :id, :created, :pageviews, :work - - def initialize(id) - @work = CurationConcerns::WorkRelation.new.find(id) - user = User.find_by(email: work.depositor) - user_id = user ? user.id : nil - - self.id = id - self.created = date_for_analytics(work) - self.pageviews = WorkViewStat.to_flots WorkViewStat.statistics(work, created, user_id) - end - - delegate :to_s, to: :work - - 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 -end diff --git a/spec/controllers/stats_controller_spec.rb b/spec/controllers/stats_controller_spec.rb index 18a386428e..e53dadbb95 100644 --- a/spec/controllers/stats_controller_spec.rb +++ b/spec/controllers/stats_controller_spec.rb @@ -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)) @@ -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)) diff --git a/spec/models/file_download_stat_spec.rb b/spec/models/file_download_stat_spec.rb index 6b2bc889f2..8139285bcc 100644 --- a/spec/models/file_download_stat_spec.rb +++ b/spec/models/file_download_stat_spec.rb @@ -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 = [] @@ -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 @@ -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 diff --git a/spec/models/file_view_stat_spec.rb b/spec/models/file_view_stat_spec.rb index f61c8cb97a..a8e74dd96f 100644 --- a/spec/models/file_view_stat_spec.rb +++ b/spec/models/file_view_stat_spec.rb @@ -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 @@ -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 diff --git a/spec/models/work_view_stat_spec.rb b/spec/models/work_view_stat_spec.rb index 529ae6f81c..b4d642a847 100644 --- a/spec/models/work_view_stat_spec.rb +++ b/spec/models/work_view_stat_spec.rb @@ -72,18 +72,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, work).and_raise("We should not call Google Analytics All data should be cached!") stats2 = described_class.statistics(work, 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 @@ -96,7 +96,7 @@ end it "includes cached data" do - expect(described_class.to_flots(stats)).to include([work_view_stat.date.to_i * 1000, work_view_stat.work_views], *view_output) + expect(stats.map(&:to_flot)).to include([work_view_stat.date.to_i * 1000, work_view_stat.work_views], *view_output) end end end diff --git a/spec/presenters/sufia/file_usage_spec.rb b/spec/presenters/sufia/file_usage_spec.rb index beff741249..e99e272674 100644 --- a/spec/presenters/sufia/file_usage_spec.rb +++ b/spec/presenters/sufia/file_usage_spec.rb @@ -1,8 +1,7 @@ -describe FileUsage, type: :model do +RSpec.describe Sufia::FileUsage, type: :model do let(:file) do - FileSet.new.tap do |file| + FileSet.create do |file| file.apply_depositor_metadata("awead") - file.save end end @@ -53,22 +52,28 @@ let(:usage) do allow_any_instance_of(FileSet).to receive(:create_date).and_return((Time.zone.today - 4.days).to_s) - expect(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) - expect(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) + allow(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) + allow(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(file.id) end describe "#initialize" do - it "sets the id" do - expect(usage.id).to eq(file.id) + it "sets the model" do + expect(usage.model).to eq file end + end - it "sets the created date" do - expect(usage.created).to eq(file.create_date) + describe "#to_flot" do + let(:flots) { usage.to_flot } + it "returns an array of hashes for use with JQuery Flot" do + expect(flots[0][:label]).to eq("Pageviews") + expect(flots[1][:label]).to eq("Downloads") + expect(flots[0][:data]).to include(*view_output) + expect(flots[1][:data]).to include(*download_output) end end - describe "statistics" do + describe "#created" do let!(:system_timezone) { ENV['TZ'] } before do ENV['TZ'] = 'UTC' @@ -78,6 +83,10 @@ ENV['TZ'] = system_timezone end + it "sets the created date" do + expect(usage.created).to eq(file.create_date) + end + it "counts the total numver of downloads" do expect(usage.total_downloads).to eq(12) end @@ -86,17 +95,9 @@ expect(usage.total_pageviews).to eq(30) end - it "returns an array of hashes for use with JQuery Flot" do - expect(usage.to_flot[0][:label]).to eq("Pageviews") - expect(usage.to_flot[1][:label]).to eq("Downloads") - expect(usage.to_flot[0][:data]).to include(*view_output) - expect(usage.to_flot[1][:data]).to include(*download_output) - end - - let(:create_date) { DateTime.new(2014, 1, 1).iso8601 } - - describe "analytics start date set" do + context "when the analytics start date is set" do let(:earliest) { DateTime.new(2014, 1, 2).iso8601 } + let(:create_date) { DateTime.new(2014, 1, 1).iso8601 } before do Sufia.config.analytic_start_date = earliest @@ -105,8 +106,6 @@ describe "create date before earliest date set" do let(:usage) do allow_any_instance_of(FileSet).to receive(:create_date).and_return(create_date.to_s) - expect(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) - expect(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(file.id) end it "sets the created date to the earliest date not the created date" do @@ -117,8 +116,6 @@ describe "create date after earliest" do let(:usage) do allow_any_instance_of(FileSet).to receive(:create_date).and_return((Time.zone.today - 4.days).to_s) - expect(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) - expect(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) Sufia.config.analytic_start_date = earliest described_class.new(file.id) end @@ -127,17 +124,18 @@ end end end - describe "start date not set" do + + context "when the start date is not set" do before do Sufia.config.analytic_start_date = nil end + let(:create_date) { DateTime.new(2014, 1, 1).iso8601 } let(:usage) do allow_any_instance_of(FileSet).to receive(:create_date).and_return(create_date.to_s) - expect(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) - expect(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(file.id) end + it "sets the created date to the earliest date not the created date" do expect(usage.created).to eq(create_date) end @@ -148,16 +146,12 @@ let(:date_uploaded) { "2014-12-31" } let(:file_migrated) do - FileSet.new.tap do |file| + FileSet.create(date_uploaded: date_uploaded) do |file| file.apply_depositor_metadata("awead") - file.date_uploaded = date_uploaded - file.save end end let(:usage) do - expect(FileDownloadStat).to receive(:ga_statistics).and_return(sample_download_statistics) - expect(FileViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(file_migrated.id) end diff --git a/spec/presenters/sufia/work_usage_spec.rb b/spec/presenters/sufia/work_usage_spec.rb index 0b83b30d5a..d596a0da35 100644 --- a/spec/presenters/sufia/work_usage_spec.rb +++ b/spec/presenters/sufia/work_usage_spec.rb @@ -1,4 +1,4 @@ -describe WorkUsage, type: :model do +RSpec.describe Sufia::WorkUsage, type: :model do let!(:work) { create(:work, id: 'abc12345xy') } let(:dates) do @@ -34,13 +34,13 @@ let(:usage) do allow_any_instance_of(GenericWork).to receive(:create_date).and_return((Time.zone.today - 4.days).to_s) - expect(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) + allow(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(work.id) end describe "#initialize" do - it "sets the id" do - expect(usage.id).to eq(work.id) + it "sets the model" do + expect(usage.model).to eq work end it "sets the created date" do @@ -54,7 +54,21 @@ it { is_expected.to eq 'Butter sculpture' } end - describe "statistics" do + describe "#to_flot" do + let(:flots) { usage.to_flot } + it "returns an array of hashes for use with JQuery Flot" do + expect(flots[0][:label]).to eq("Pageviews") + expect(flots[0][:data]).to include(*view_output) + end + end + + describe "#total_pageviews" do + it "counts the total number of pageviews" do + expect(usage.total_pageviews).to eq(30) + end + end + + describe "#created" do let!(:system_timezone) { ENV['TZ'] } before do ENV['TZ'] = 'UTC' @@ -64,15 +78,6 @@ ENV['TZ'] = system_timezone end - it "counts the total number of pageviews" do - expect(usage.total_pageviews).to eq(30) - end - - it "returns an array of hashes for use with JQuery Flot" do - expect(usage.to_flot[0][:label]).to eq("Pageviews") - expect(usage.to_flot[0][:data]).to include(*view_output) - end - let(:create_date) { Time.zone.parse('2014-01-02 12:00:00').iso8601 } describe "analytics start date set" do @@ -85,7 +90,6 @@ describe "create date before earliest date set" do let(:usage) do allow_any_instance_of(GenericWork).to receive(:create_date).and_return(create_date.to_s) - expect(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(work.id) end it "sets the created date to the earliest date not the created date" do @@ -96,7 +100,6 @@ describe "create date after earliest" do let(:usage) do allow_any_instance_of(GenericWork).to receive(:create_date).and_return((Time.zone.today - 4.days).to_s) - expect(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) Sufia.config.analytic_start_date = earliest described_class.new(work.id) end @@ -112,7 +115,6 @@ let(:usage) do allow_any_instance_of(GenericWork).to receive(:create_date).and_return(create_date.to_s) - expect(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(work.id) end it "sets the created date to the earliest date not the created date" do @@ -126,7 +128,6 @@ let(:work_migrated) { create(:work, id: '678901234', date_uploaded: date_uploaded) } let(:usage) do - expect(WorkViewStat).to receive(:ga_statistics).and_return(sample_pageview_statistics) described_class.new(work_migrated.id) end