From 5196aa2ca3bea3c76db7dce74c23f96a94f4e3be Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sat, 18 Jan 2025 10:01:03 -0500 Subject: [PATCH 1/4] Properly nest returning judgements for a book under the /book/:book_id/judgements route --- app/controllers/api/v1/books_controller.rb | 62 +-- .../api/v1/judgements_controller.rb | 77 +++- app/models/user.rb | 6 +- .../show.csv.erb => judgements/index.csv.erb} | 0 app/views/books/show.html.erb | 4 +- config/routes.rb | 432 +----------------- .../api/v1/books_controller_test.rb | 30 -- .../api/v1/judgements_controller_test.rb | 31 +- test/models/user_test.rb | 17 + 9 files changed, 134 insertions(+), 525 deletions(-) rename app/views/api/v1/{books/show.csv.erb => judgements/index.csv.erb} (100%) diff --git a/app/controllers/api/v1/books_controller.rb b/app/controllers/api/v1/books_controller.rb index 5ccc8c719..7600f0d70 100644 --- a/app/controllers/api/v1/books_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/judgements_controller.rb b/app/controllers/api/v1/judgements_controller.rb index 107439df8..ae9f287b5 100644 --- a/app/controllers/api/v1/judgements_controller.rb +++ b/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.require(:judgement).permit(: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 diff --git a/app/models/user.rb b/app/models/user.rb index 853d55b1f..1d9d2cf5f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -257,7 +257,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 diff --git a/app/views/api/v1/books/show.csv.erb b/app/views/api/v1/judgements/index.csv.erb similarity index 100% rename from app/views/api/v1/books/show.csv.erb rename to app/views/api/v1/judgements/index.csv.erb diff --git a/app/views/books/show.html.erb b/app/views/books/show.html.erb index e385fe9b3..070fccb5b 100644 --- a/app/views/books/show.html.erb +++ b/app/views/books/show.html.erb @@ -165,7 +165,7 @@ This book consists of <%= @book.query_doc_pairs.count %> query document pairs an

There are a number of ways to export the Book data.