Merge pull request #2760 from projecthydra/guard_nil

Guard for nil GA profile in FileDownloadStat

.codeclimate.yml

@@ -5,6 +5,8 @@ engines:
     exclude_fingerprints:
       - 6ad4464dbb2a999591c7be8346dc137c3372b280f4a8b0c024fef91dfebeeb83
       - c05c1baf940f00a565ffe12f8b8a213fa06cebd734e0dae92057db28dc7c018b
+      - b388847a782f9d55d164f632b892125496082b9cfe5813720327ae6afbaf607b
+      - c7c241833ba150c2e4c72a26e1ac4d674230cb5e5c87322bedfe2b50dd09247d
   duplication:
     enabled: true
     exclude_fingerprints:

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

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

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

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

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

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

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

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))

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

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