Merge pull request #174 from MITLibraries/tco-126-confirmation-scope

Add option to confirm only categorized terms

Author: matt-bernhardt

app/controllers/term_controller.rb

@@ -3,8 +3,16 @@
 class TermController < ApplicationController
   include Pagy::Backend
 
-  # This provides the list of terms that are awaiting confirmation.
+  # This provides the list of terms that are awaiting confirmation. By default this shows only terms which have been
+  # categorized automatically. Adding `type=all` to the querystring will show _all_ terms which the user has not yet
+  # confirmed.
   def unconfirmed
-    @pagy, @records = pagy(Term.user_unconfirmed)
+    terms = if params[:show] == 'all'
+              authorize! :confirm_uncategorized, Term
+              Term.user_unconfirmed
+            else
+              Term.categorized.user_unconfirmed
+            end
+    @pagy, @records = pagy(terms)
   end
 end

app/models/ability.rb

@@ -40,6 +40,7 @@ def initialize(user)
     # Rules for admins
     return unless user.admin?
 
+    can :confirm_uncategorized, Term
     can :manage, :all
   end
 end

app/models/term.rb

@@ -24,6 +24,7 @@ class Term < ApplicationRecord
   before_save :register_fingerprint
   after_destroy :check_fingerprint_count
 
+  scope :categorized, -> { where.associated(:categorizations).distinct }
   scope :user_confirmed, -> { where.associated(:confirmations).distinct }
   scope :user_unconfirmed, -> { where.missing(:confirmations).distinct }
 

app/views/term/unconfirmed.html.erb

@@ -3,6 +3,13 @@
 <p>The terms listed here have not yet been reviewed by a person and placed into a category. Clicking on any term will
 take you to the form for submitting this information.</p>
 
+<% if can? :confirm_uncategorized, Term %>
+<p class="wrap-filters">View:
+  <a class="btn button-small button-secondary" href="<%= terms_unconfirmed_path %>">Categorized terms</a>
+  <a class="btn button-small button-secondary" href="<%= terms_unconfirmed_path(show: 'all') %>">All terms</a>
+</p>
+<% end %>
+
 <p><%== pagy_info(@pagy) %></p>
 
 <ul class="list-unbulleted">

test/controllers/term_controller_test.rb

@@ -25,4 +25,52 @@ class TermControllerTest < ActionDispatch::IntegrationTest
 
     assert_response :success
   end
+
+  test 'basic users see only one confirmation option' do
+    sign_in users(:basic)
+    get terms_unconfirmed_path
+
+    assert_select 'p.wrap-filters', text: "View:\n  Categorized terms\n  All terms", count: 0
+  end
+
+  test 'admin users see two confirmation options' do
+    sign_in users(:admin)
+    get terms_unconfirmed_path
+
+    assert_select 'p.wrap-filters', text: "View:\n  Categorized terms\n  All terms", count: 1
+  end
+
+  test 'basic users cannot access the confirmation option for uncategorized terms' do
+    sign_in users(:basic)
+    get terms_unconfirmed_path(show: 'all')
+
+    assert_redirected_to '/'
+    follow_redirect!
+
+    assert_select 'div.alert', text: 'Not authorized.', count: 1
+  end
+
+  test 'admin users can access the confirmation option for uncategorized terms' do
+    sign_in users(:admin)
+    get terms_unconfirmed_path(show: 'all')
+
+    assert_response :success
+  end
+
+  test 'confirmation index can show two different sets of terms for admin users' do
+    sign_in users(:admin)
+    get terms_unconfirmed_path
+
+    # default_pagy will be something like "Displaying 10 items"
+    default_pagy = response.parsed_body.xpath('//main//span').first.text
+    default_pagy_count = default_pagy.split.second.to_i
+
+    get terms_unconfirmed_path(show: 'all')
+
+    # The '?type=all' route asks for more records, so the count should be higher
+    all_pagy = response.parsed_body.xpath('//main//span').first.text
+    all_pagy_count = all_pagy.split.second.to_i
+
+    assert_operator all_pagy_count, :>, default_pagy_count
+  end
 end

test/models/term_test.rb

@@ -299,6 +299,31 @@ class TermTest < ActiveSupport::TestCase
     end
   end
 
+  test 'categorized scope returns an active record relation' do
+    assert_kind_of ActiveRecord::Relation, Term.categorized
+  end
+
+  test 'categorized scope accounts for terms with multiple categorizations' do
+    categorized_count = Term.categorized.count
+    t = terms('doi')
+    orig_categorization_count = t.categorizations.count
+
+    # term has been categorized already
+    assert_operator 1, :<=, orig_categorization_count
+
+    new_record = {
+      term: t,
+      category: categories('navigational'),
+      confidence: 0.5,
+      detector_version: '1'
+    }
+    Categorization.create!(new_record)
+
+    # The term has gained a category, but the categorized scope has not changed size.
+    assert_operator orig_categorization_count, :<, t.categorizations.count
+    assert_equal categorized_count, Term.categorized.count
+  end
+
   test 'user_confirmed scope returns an active record relation' do
     assert_kind_of ActiveRecord::Relation, Term.user_confirmed
   end