From 5da0c01de38e7fc4e0e9bf22c1d2937d496f41c1 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Mon, 23 Sep 2024 13:01:49 -0400 Subject: [PATCH 1/4] Fork administrate navigation --- .../admin/application/_navigation.html.erb | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 app/views/admin/application/_navigation.html.erb diff --git a/app/views/admin/application/_navigation.html.erb b/app/views/admin/application/_navigation.html.erb new file mode 100644 index 0000000..23ef8d3 --- /dev/null +++ b/app/views/admin/application/_navigation.html.erb @@ -0,0 +1,20 @@ +<%# +# Navigation + +This partial is used to display the navigation in Administrate. +By default, the navigation contains navigation links +for all resources in the admin dashboard, +as defined by the routes in the `admin/` namespace +%> + + From 1e66de52ad06023a3a88237ccc6658b2e44d86e8 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Mon, 23 Sep 2024 13:02:08 -0400 Subject: [PATCH 2/4] Update forked administrate navigation --- app/views/admin/application/_navigation.html.erb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/views/admin/application/_navigation.html.erb b/app/views/admin/application/_navigation.html.erb index 23ef8d3..abfe4cc 100644 --- a/app/views/admin/application/_navigation.html.erb +++ b/app/views/admin/application/_navigation.html.erb @@ -5,16 +5,23 @@ This partial is used to display the navigation in Administrate. By default, the navigation contains navigation links for all resources in the admin dashboard, as defined by the routes in the `admin/` namespace + +This local template was forked from the administrate version to allow direct usage of +the `can?` check rather than administrates `accessible_action?` which didn't work in +our environment. Our option was to fork this template or monkey patch that method. This +felt least likely to have undesired side effects as `accessible_action?` is used in more +places than just this and we haven't noted it not working as expected there. If we realize +other templates are also having it not work as needed, we should monkey patch that method +and delete this template. %> From 9581062ae1d36e25ccc8e3f141f182d95a31e068 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Mon, 23 Sep 2024 13:14:42 -0400 Subject: [PATCH 3/4] Adds initial algorithm metric reports Why are these changes being introduced: * Allowing a way to access our historical matches over time will be useful Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TCO-63 * https://mitlibraries.atlassian.net/browse/TCO-19 How does this address that need: * Adds Report index * Adds Metrics Algorithm report with monthly and aggregate support Document any side effects to this change: * Updates ability.rb for SuggestedResources to handle the difference in syntax when working in Administrate context versus the rest of our app --- app/controllers/report_controller.rb | 13 +++++++ app/helpers/metrics_helper.rb | 18 ++++++++++ app/models/ability.rb | 12 +++++-- app/models/metrics/algorithms.rb | 3 ++ app/views/layouts/_site_nav.html.erb | 12 +++++-- app/views/report/algorithm_metrics.html.erb | 40 +++++++++++++++++++++ app/views/report/index.html.erb | 6 ++++ config/routes.rb | 2 ++ test/helpers/metrics_helper_test.rb | 29 +++++++++++++++ 9 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 app/controllers/report_controller.rb create mode 100644 app/helpers/metrics_helper.rb create mode 100644 app/views/report/algorithm_metrics.html.erb create mode 100644 app/views/report/index.html.erb create mode 100644 test/helpers/metrics_helper_test.rb diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb new file mode 100644 index 0000000..9cf7e4e --- /dev/null +++ b/app/controllers/report_controller.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ReportController < ApplicationController + def index; end + + def algorithm_metrics + @metrics = if params[:type] == 'aggregate' + Metrics::Algorithms.aggregates + else + Metrics::Algorithms.monthlies + end + end +end diff --git a/app/helpers/metrics_helper.rb b/app/helpers/metrics_helper.rb new file mode 100644 index 0000000..adbe747 --- /dev/null +++ b/app/helpers/metrics_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module MetricsHelper + # Calculate percentage of search events in which we had any Detection match + def percent_match(metric) + (sum_matched(metric) / sum_total(metric) * 100).round(2) + end + + # Sums all detection matches for a given Metrics::Algorithms record + def sum_matched(metric) + metric.doi.to_f + metric.issn.to_f + metric.isbn.to_f + metric.pmid.to_f + metric.journal_exact.to_f + metric.suggested_resource_exact.to_f + end + + # Calculates total events for a given Metrics::Algorithms record + def sum_total(metric) + sum_matched(metric) + metric.unmatched.to_f + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index 4c4f547..6687700 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,12 +7,18 @@ class Ability # See the wiki for details: # https://github.com/CanCanCommunity/cancancan/blob/develop/docs/define_check_abilities.md def initialize(user) + # Actions allowed for non-authenticated Users should add `skip_before_action :require_user` + + # Start of Rules for all authenticated user with no additional roles required return if user.blank? - # Rules will go here. - # all authenticated - # can :view, :playground + can :manage, :detector__suggested_resource + can :manage, Detector::SuggestedResource + + can :view, :report + # End of Rules for all authenticated user with no additional roles required + # Rules for admins return unless user.admin? can :manage, :all diff --git a/app/models/metrics/algorithms.rb b/app/models/metrics/algorithms.rb index 82b7fd7..3368641 100644 --- a/app/models/metrics/algorithms.rb +++ b/app/models/metrics/algorithms.rb @@ -21,6 +21,9 @@ module Metrics class Algorithms < ApplicationRecord self.table_name = 'metrics_algorithms' + scope :monthlies, -> { where.not(month: nil).order(month: :asc) } + scope :aggregates, -> { where(month: nil).order(month: :asc) } + # generate metrics data about SearchEvents matches # # @note This is expected to only be run once per month per type of aggregation (once with no month supplied, once diff --git a/app/views/layouts/_site_nav.html.erb b/app/views/layouts/_site_nav.html.erb index 7f1be8a..f5f798f 100644 --- a/app/views/layouts/_site_nav.html.erb +++ b/app/views/layouts/_site_nav.html.erb @@ -16,6 +16,12 @@ <% if can? :view, :playground %> <%= link_to('Playground', '/playground', class: 'nav-item') %> <% end %> + <% if can? :manage, :detector__suggested_resource %> + <%= link_to('Suggested Resources', admin_detector_suggested_resources_path, class: 'nav-item') %> + <% end %> + <% if can? :view, :report %> + <%= link_to('Reports', report_path, class: 'nav-item') %> + <% end %> <% end %> diff --git a/app/views/report/algorithm_metrics.html.erb b/app/views/report/algorithm_metrics.html.erb new file mode 100644 index 0000000..5aac198 --- /dev/null +++ b/app/views/report/algorithm_metrics.html.erb @@ -0,0 +1,40 @@ +
+

Percentage match is not accurate for Terms that match multiple algorithms. The actual percentage match will be lower than the reported value. At this time, the error is not believed to be significant based on the currently low volume of Terms that match multiple algorithms. This may change as we develop new algorithms to a point where we need to address this discrepancy.

+
+ + + + <% if params[:type] == 'aggregate' %> + + <% else %> + + <% end %> + + + + + + + + + +<% @metrics.each do |metric| %> + + <% if params[:type] == 'aggregate' %> + + <% else %> + + <% end %> + + + + + + + + + + <% end %> +
Aggregation dateMonthDOIISSNISBNPMIDJournalSuggestedResourceUnmatched% matched
<%= metric.updated_at.strftime("%d %B %Y") %><%= metric.month.strftime("%B %Y") %><%= metric.doi %><%= metric.issn %><%= metric.isbn %><%= metric.pmid %><%= metric.journal_exact %><%= metric.suggested_resource_exact %><%= metric.unmatched %> + <%= percent_match(metric) %> +
diff --git a/app/views/report/index.html.erb b/app/views/report/index.html.erb new file mode 100644 index 0000000..f1d8058 --- /dev/null +++ b/app/views/report/index.html.erb @@ -0,0 +1,6 @@ +

Report index

+ +
    +
  • <%= link_to("Monthly algorithm metrics", report_algorithm_metrics_path) %>
    • +
  • <%= link_to("Aggregate algorithm metrics", report_algorithm_metrics_path(type: 'aggregate') ) %>
    • +
diff --git a/config/routes.rb b/config/routes.rb index 7239556..ac74994 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -34,6 +34,8 @@ get 'up' => 'rails/health#show', as: :rails_health_check get 'playground', to: 'static#playground' + get '/report', to: 'report#index' + get '/report/algorithm_metrics', to: 'report#algorithm_metrics' # Defines the root path route ("/") root to: 'static#index' diff --git a/test/helpers/metrics_helper_test.rb b/test/helpers/metrics_helper_test.rb new file mode 100644 index 0000000..574f5da --- /dev/null +++ b/test/helpers/metrics_helper_test.rb @@ -0,0 +1,29 @@ +require 'test_helper' + +class MetricsHelperTest < ActionView::TestCase + def metric + Metrics::Algorithms.new( + month: DateTime.now - 1.month, + doi: 1, + issn: 2, + isbn: 3, + pmid: 4, + unmatched: 79, + journal_exact: 5, + suggested_resource_exact: 6, + created_at: DateTime.now, + updated_at: DateTime.now + ) + end + test 'can calculate a percentage of matches for a record' do + assert_in_delta(21.0, percent_match(metric), 0.01) + end + + test 'can calculate sum of matches for a record' do + assert_in_delta(21.0, sum_matched(metric), 0.01) + end + + test 'can calculate sum of all events for a record' do + assert_in_delta(100.0, sum_total(metric), 0.01) + end +end From f131051128828b50e5e0bc11ce43411009afe36c Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Mon, 23 Sep 2024 13:43:37 -0400 Subject: [PATCH 4/4] algorithm metric controller tests --- app/views/report/algorithm_metrics.html.erb | 6 ++ test/controllers/report_controller_test.rb | 69 +++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/controllers/report_controller_test.rb diff --git a/app/views/report/algorithm_metrics.html.erb b/app/views/report/algorithm_metrics.html.erb index 5aac198..c6a0042 100644 --- a/app/views/report/algorithm_metrics.html.erb +++ b/app/views/report/algorithm_metrics.html.erb @@ -2,6 +2,12 @@

Percentage match is not accurate for Terms that match multiple algorithms. The actual percentage match will be lower than the reported value. At this time, the error is not believed to be significant based on the currently low volume of Terms that match multiple algorithms. This may change as we develop new algorithms to a point where we need to address this discrepancy.

+<% if params[:type] == 'aggregate' %> +

Aggregate Algorithm Metrics

+<% else %> +

Monthly Algorithm Metrics

+<% end %> + <% if params[:type] == 'aggregate' %> diff --git a/test/controllers/report_controller_test.rb b/test/controllers/report_controller_test.rb new file mode 100644 index 0000000..f2b4608 --- /dev/null +++ b/test/controllers/report_controller_test.rb @@ -0,0 +1,69 @@ +require 'test_helper' + +class ReportControllerTest < ActionDispatch::IntegrationTest + test 'report index is not accessible without authentication' do + get '/report' + + assert_redirected_to '/' + follow_redirect! + + assert_select 'div.alert', text: 'Please sign in to continue', count: 1 + end + + test 'report index is accessible to admins when authenticated' do + sign_in users(:admin) + + get '/report' + + assert_response :success + end + + test 'report index url is accessible to basic users when authenticated' do + sign_in users(:basic) + + get '/report' + + assert_response :success + end + + test 'algorithm metrics report is not accessible without authentication' do + get '/report/algorithm_metrics' + + assert_redirected_to '/' + follow_redirect! + + assert_select 'div.alert', text: 'Please sign in to continue', count: 1 + end + + test 'algorithm metrics report is accessible to admins when authenticated' do + sign_in users(:admin) + + get '/report/algorithm_metrics' + + assert_response :success + end + + test 'algorithm metrics report is accessible to basic users when authenticated' do + sign_in users(:basic) + + get '/report/algorithm_metrics' + + assert_response :success + end + + test 'algorithm metrics can show monthly data' do + sign_in users(:basic) + + get '/report/algorithm_metrics' + + assert_select 'h3', text: 'Monthly Algorithm Metrics', count: 1 + end + + test 'algorithm metrics can show aggregate data' do + sign_in users(:basic) + + get '/report/algorithm_metrics?type=aggregate' + + assert_select 'h3', text: 'Aggregate Algorithm Metrics', count: 1 + end +end