Properly nest returning judgements for a book under the book route (#…

…1187)

* Properly nest returning judgements for a book under the /book/:book_id/judgements route

* Update test

* Simplify test

* Tweak rubocop rules

app/controllers/api/v1/books_controller.rb

@@ -4,7 +4,6 @@
 
 module Api
   module V1
-    # rubocop:disable Metrics/ClassLength
     class BooksController < Api::ApiController
       before_action :set_book, only: [ :show, :update, :destroy ]
       before_action :check_book, only: [ :show, :update, :destroy ]
@@ -27,63 +26,13 @@ def index
         respond_with @books
       end
 
-      # rubocop:disable Metrics/MethodLength
-      # rubocop:disable Metrics/AbcSize
-      # rubocop:disable Metrics/CyclomaticComplexity
-      # rubocop:disable Metrics/PerceivedComplexity
-      # rubocop:disable Metrics/BlockLength
       api :GET, '/api/books/:book_id',
           'Show the book with the given ID.'
       param :id, :number,
             desc: 'The ID of the requested book.', required: true
       def show
-        respond_to do |format|
-          format.json
-          format.csv do
-            @csv_array = []
-            csv_headers = %w[query docid]
-
-            # Only return rateable judgements, filter out the unrateable ones.
-            # unique_raters = @book.judgements.rateable.preload(:user).collect(&:user).uniq
-            # unique_raters = @book.judges.merge(Judgement.rateable)
-            unique_judge_ids = @book.query_doc_pairs.joins(:judgements)
-              .distinct.pluck(:user_id)
-
-            # this logic about using email versus name is kind of awful.  Think about user.full_name or user.identifier?
-            unique_judges = []
-            unique_judge_ids.each do |judge_id|
-              judge = User.find(judge_id) unless judge_id.nil?
-              unique_judges << judge
-              csv_headers << make_csv_safe(if judge.nil?
-                                             'anonymous'
-                                           else
-                                             judge.name.presence || judge.email
-                                           end)
-            end
-
-            @csv_array << csv_headers
-            query_doc_pairs = @book.query_doc_pairs.includes(:judgements)
-            query_doc_pairs.each do |qdp|
-              row = [ make_csv_safe(qdp.query_text), qdp.doc_id ]
-              unique_judges.each do |judge|
-                judgement = qdp.judgements.detect { |j| j.user == judge }
-                rating = judgement.nil? ? '' : judgement.rating
-
-                row.append rating
-              end
-              @csv_array << row
-            end
-
-            headers['Content-Disposition'] = "attachment; filename=\"book_#{@book.id}_export.csv\""
-            headers['Content-Type'] ||= 'text/csv'
-          end
-        end
+        respond_with @judgement
       end
-      # rubocop:enable Metrics/MethodLength
-      # rubocop:enable Metrics/AbcSize
-      # rubocop:enable Metrics/CyclomaticComplexity
-      # rubocop:enable Metrics/PerceivedComplexity
-      # rubocop:enable Metrics/BlockLength
 
       api :POST, '/api/books', 'Create a new book.'
       param_group :book_params
@@ -139,15 +88,6 @@ def set_book
       def check_book
         render json: { message: 'Book not found!' }, status: :not_found unless @book
       end
-
-      def make_csv_safe str
-        if %w[- = + @].include?(str[0])
-          " #{str}"
-        else
-          str
-        end
-      end
     end
-    # rubocop:enable Metrics/ClassLength
   end
 end

app/controllers/api/v1/judgements_controller.rb

@@ -2,6 +2,7 @@
 
 module Api
   module V1
+    # rubocop:disable Metrics/ClassLength
     class JudgementsController < Api::ApiController
       before_action :set_book
       before_action :check_book
@@ -29,11 +30,76 @@ class JudgementsController < Api::ApiController
         end
       end
 
+      # rubocop:disable Metrics/MethodLength
+      # rubocop:disable Metrics/AbcSize
+      # rubocop:disable Metrics/CyclomaticComplexity
+      # rubocop:disable Metrics/PerceivedComplexity
+      # rubocop:disable Metrics/BlockLength
       api :GET, '/api/books/:book_id/judgements',
-          'List all judgements for the book'
+          'List all judgements for the book.  When you request the .csv version we only return valid rated judgements.'
       param :book_id, :number,
             desc: 'The ID of the requested book.', required: true
       def index
+        respond_to do |format|
+          format.json do
+            @judgements = @book.judgements
+          end
+          format.csv do
+            @csv_array = []
+            csv_headers = %w[query docid]
+
+            # can't use more efficient quering because we need to include judgements where the user_id is nil.
+            # Maybe someday we require a user for all judgements?  Even if the user has the name 'Anonymous'?
+            # Only return rateable judgements, filter out the unrateable ones.
+            # unique_judge_ids = @book.query_doc_pairs.joins(:judgements).rateable
+            #  .distinct.pluck(:user_id)
+            # unique_judge_ids = @book.judgements.rateable.select(:user_id).distinct.pluck(:user_id)
+            unique_judge_ids = @book.query_doc_pairs.joins(:judgements)
+              .distinct.pluck(:user_id)
+
+            # unique_judges = User.where(id: @book.judgements.select(:user_id).distinct)
+            # unique_judges.each do |judge|
+            #  csv_headers << make_csv_safe(judge.fullname)
+            # end
+            unique_judges = []
+            unique_judge_ids.each do |judge_id|
+              judge = User.find(judge_id) unless judge_id.nil?
+              unique_judges << judge
+              csv_headers << make_csv_safe(if judge.nil?
+                                             'Anonymous'
+                                           else
+                                             judge.fullname
+                                           end)
+            end
+
+            @csv_array << csv_headers
+            query_doc_pairs = @book.query_doc_pairs
+              .joins(:judgements)
+              .where.not(judgements: { rating: nil })
+              .includes(:judgements)
+            query_doc_pairs.each do |qdp|
+              row = [ make_csv_safe(qdp.query_text), qdp.doc_id ]
+              unique_judges.each do |judge|
+                judgement = qdp.judgements.detect { |j| j.user == judge }
+                rating = judgement.nil? ? '' : judgement.rating
+
+                row.append rating
+              end
+              @csv_array << row
+            end
+
+            headers['Content-Disposition'] = "attachment; filename=\"book_#{@book.id}_judgements.csv\""
+            headers['Content-Type'] ||= 'text/csv'
+          end
+        end
+      end
+      # rubocop:enable Metrics/MethodLength
+      # rubocop:enable Metrics/AbcSize
+      # rubocop:enable Metrics/CyclomaticComplexity
+      # rubocop:enable Metrics/PerceivedComplexity
+      # rubocop:enable Metrics/BlockLength
+
+      def indexold
         @judgements = @book.judgements
 
         respond_with @judgements
@@ -108,6 +174,14 @@ def destroy
 
       private
 
+      def make_csv_safe str
+        if %w[- = + @].include?(str[0])
+          " #{str}"
+        else
+          str
+        end
+      end
+
       def judgement_params
         params.expect(judgement: [ :rating, :unrateable, :query_doc_pair_id, :user_id, :explanation ])
       end
@@ -120,5 +194,6 @@ def check_judgement
         render json: { message: 'Query Doc Pair not found!' }, status: :not_found unless @judgement
       end
     end
+    # rubocop:enable Metrics/ClassLength
   end
 end

app/models/user.rb

@@ -271,7 +271,11 @@ def unlock
   end
 
   def fullname
-    name.blank? ? email : name.titleize
+    if name.blank?
+      email.presence || 'Anonymous'
+    else
+      name.titleize
+    end
   end
 
   def after_database_authentication

app/views/books/show.html.erb

@@ -165,7 +165,7 @@ This book consists of <%= @book.query_doc_pairs.count %> query document pairs an
     <p class="card-text">There are a number of ways to export the Book data.</p>
 
     <ul class="list-group list-group-flush">
-      <li class="list-group-item">Export just the <%= link_to 'judgement data', api_book_path(@book, format: :csv) %> in CSV format.  This will take a bit but you get a CSV file in your browser.</li>
+      <li class="list-group-item">Export just the <%= link_to 'judgement data', api_book_judgements_path(@book, format: :csv) %> in CSV format.  This will take a bit but you will download a CSV file, useful for your own analysis.</li>
 
       <li class="list-group-item">Export the entire Book in JSON format.
         <% if @book.export_job %>
@@ -182,7 +182,7 @@ This book consists of <%= @book.query_doc_pairs.count %> query document pairs an
       <li class="list-group-item">
         <p>
           Reference these judgements from a notebook or another system you can use these <%= link_to 'Quepid APIs', apipie_apipie_path %> endpoints:
-          <code><%= api_book_path(@book, format: :csv) %></code> or <code><%= api_export_book_path(@book) %></code>
+          <code><%= api_book_judgements_path(@book, format: :csv) %></code> or <code><%= api_export_book_path(@book) %></code>
         </p>
 
       </li>

config/routes.rb

@@ -386,7 +386,6 @@
 
 # rubocop:enable Layout/LineLength
 
-# rubocop:disable Layout/LineLength
 # rubocop:disable Metrics/BlockLength
 Rails.application.routes.draw do
   # Define your application routes per the DSL in https://guides.rubyonrails.org/routing.html
@@ -431,9 +430,9 @@
   end
 
   # let's encrypt verification (can be removed in the future)
-  get '.well-known/acme-challenge/9IWOgATbRmEtWKsOOJQ-E4-lrIT9tHsHv_9bl5Zt6fI', to: proc { [ 200, {}, [ '9IWOgATbRmEtWKsOOJQ-E4-lrIT9tHsHv_9bl5Zt6fI.fDzklrX7i2PRMRsPtxEvo2yRZDSfy2LO3t--NfWfgaA' ] ] }
-  # rubocop:enable Layout/LineLength
-
+  get '.well-known/acme-challenge/9IWOgATbRmEtWKsOOJQ-E4-lrIT9tHsHv_9bl5Zt6fI', to: proc {
+    [ 200, {}, [ '9IWOgATbRmEtWKsOOJQ-E4-lrIT9tHsHv_9bl5Zt6fI.fDzklrX7i2PRMRsPtxEvo2yRZDSfy2LO3t--NfWfgaA' ] ]
+  }
   post 'users/login' => 'sessions#create' # , #defaults: { format: :json
   post 'users/signup' => 'users/signups#create'
 

test/controllers/api/v1/books_controller_test.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'test_helper'
-require 'csv'
 module Api
   module V1
     class BooksControllerTest < ActionController::TestCase
@@ -95,35 +94,6 @@ class BooksControllerTest < ActionController::TestCase
           # assert_nil body['query_doc_pairs'][0]['document_fields']
         end
       end
-
-      describe 'Exporting a book in basic csv format' do
-        let(:book)        { books(:james_bond_movies) }
-        let(:judgement)   { judgements(:jbm_qdp10_judgement) }
-        let(:doug)        { users(:doug) }
-        let(:random_user) { users(:random) }
-
-        test 'returns book w/ query doc pairs and judgement info' do
-          get :show, params: { id: book.id, format: :csv }
-
-          assert_response :ok
-          csv = CSV.parse(response.body, headers: true)
-          assert_equal 'Best Bond Ever', csv[0]['query']
-          assert_equal 'GeorgeLazenby', csv[0]['docid']
-          assert_equal '3.0', csv[3]['Doug Turnbull']
-
-          assert_not_includes csv.headers, 'Unknown'
-        end
-
-        test 'handles a rating that is not associated with a user, and adds Unknown' do
-          judgement.user = nil
-          judgement.save!
-          get :show, params: { id: book.id, format: :csv }
-
-          assert_response :ok
-          csv = CSV.parse(response.body, headers: true)
-          assert_includes csv.headers, 'anonymous'
-        end
-      end
     end
   end
 end

test/controllers/api/v1/judgements_controller_test.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require 'test_helper'
-
+require 'csv'
 module Api
   module V1
     class JudgementsControllerTest < ActionController::TestCase
@@ -36,6 +36,35 @@ class JudgementsControllerTest < ActionController::TestCase
           end
         end
       end
+
+      describe 'Listing judgements for a book in basic csv format' do
+        let(:book)        { books(:james_bond_movies) }
+        let(:judgement)   { judgements(:jbm_qdp10_judgement) }
+        let(:doug)        { users(:doug) }
+        let(:random_user) { users(:random) }
+
+        test 'returns book w/ query doc pairs and judgement info' do
+          get :index, params: { book_id: book.id, format: :csv }
+
+          assert_response :ok
+          csv = CSV.parse(response.body, headers: true)
+
+          assert_not_nil csv[0]['query']
+          assert_not_nil csv[0]['docid']
+
+          assert_not_includes csv.headers, 'Unknown'
+        end
+
+        test 'handles a rating that is not associated with a user, and adds Unknown' do
+          judgement.user = nil
+          judgement.save!
+          get :index, params: { book_id: book.id, format: :csv }
+
+          assert_response :ok
+          csv = CSV.parse(response.body, headers: true)
+          assert_includes csv.headers, 'Anonymous'
+        end
+      end
     end
   end
 end

test/models/user_test.rb

@@ -337,6 +337,23 @@ class UserTest < ActiveSupport::TestCase
       assert user.valid?
     end
   end
+
+  describe 'The full name of the user' do
+    it 'uses the name if possible' do
+      user = User.new(name: 'bob')
+      assert 'bob', user.fullname
+    end
+
+    it 'uses the email if no name' do
+      user = User.new(email: '[email protected]')
+      assert '[email protected]', user.fullname
+    end
+
+    it 'handles it when we got nothing!' do
+      user = User.new
+      assert 'Anonymous', user.fullname
+    end
+  end
 end
 
 # rubocop:enable Layout/LineLength