From f9edf81d6b3ee87da8c25d67ce3ccdb8ac3f738e Mon Sep 17 00:00:00 2001
From: Eric Pugh <epugh@opensourceconnections.com>
Date: Tue, 19 Sep 2023 18:23:55 -0400
Subject: [PATCH] Finally, proper book export/import

---
 .../api/v1/export/books_controller.rb         |   1 +
 .../api/v1/import/books_controller.rb         |  94 ++++++++++++
 app/views/api/v1/books/_book.json.jbuilder    |  26 +---
 .../api/v1/books/_judgements.json.jbuilder    |  10 --
 .../v1/books/_query_doc_pairs.json.jbuilder   |  12 --
 app/views/api/v1/books/create.json.jbuilder   |   2 +-
 app/views/api/v1/books/show.json.jbuilder     |   2 +-
 app/views/api/v1/books/update.json.jbuilder   |   2 +-
 .../api/v1/export/books/_book.json.jbuilder   |  22 +++
 .../v1/export/books/_judgements.json.jbuilder |   5 +
 .../books/_query_doc_pair.json.jbuilder       |  10 ++
 .../books/_selection_strategy.json.jbuilder   |   0
 .../api/v1/export/books/show.json.jbuilder    |   3 +
 config/routes.rb                              |   2 +
 .../api/v1/books_controller_test.rb           |  55 +------
 .../api/v1/export/books_controller_test.rb    |  42 +++++
 .../api/v1/import/books_controller_test.rb    | 145 ++++++++++++++++++
 test/fixtures/query_doc_pairs.yml             |   2 +-
 .../api/export_import_book_flow_test.rb       |  11 +-
 19 files changed, 339 insertions(+), 107 deletions(-)
 create mode 100644 app/controllers/api/v1/import/books_controller.rb
 delete mode 100644 app/views/api/v1/books/_judgements.json.jbuilder
 delete mode 100644 app/views/api/v1/books/_query_doc_pairs.json.jbuilder
 create mode 100644 app/views/api/v1/export/books/_book.json.jbuilder
 create mode 100644 app/views/api/v1/export/books/_judgements.json.jbuilder
 create mode 100644 app/views/api/v1/export/books/_query_doc_pair.json.jbuilder
 rename app/views/api/v1/{ => export}/books/_selection_strategy.json.jbuilder (100%)
 create mode 100644 app/views/api/v1/export/books/show.json.jbuilder
 create mode 100644 test/controllers/api/v1/export/books_controller_test.rb
 create mode 100644 test/controllers/api/v1/import/books_controller_test.rb

diff --git a/app/controllers/api/v1/export/books_controller.rb b/app/controllers/api/v1/export/books_controller.rb
index eef2db9e5..be40a73de 100644
--- a/app/controllers/api/v1/export/books_controller.rb
+++ b/app/controllers/api/v1/export/books_controller.rb
@@ -8,6 +8,7 @@ class BooksController < Api::ApiController
         before_action :check_book
 
         def show
+          respond_with @book
         end
       end
     end
diff --git a/app/controllers/api/v1/import/books_controller.rb b/app/controllers/api/v1/import/books_controller.rb
new file mode 100644
index 000000000..5eaf69ebb
--- /dev/null
+++ b/app/controllers/api/v1/import/books_controller.rb
@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+module Api
+  module V1
+    module Import
+      class BooksController < Api::ApiController
+        # before_action :find_book
+        # before_action :check_case
+
+        # rubocop:disable Metrics/MethodLength
+        # rubocop:disable Metrics/AbcSize
+        # rubocop:disable Metrics/CyclomaticComplexity
+        # rubocop:disable Metrics/PerceivedComplexity
+        # rubocop:disable Layout/LineLength
+        def create
+          team_id = params.require(:team_id)
+          params_to_use = book_params.to_h.deep_symbolize_keys
+
+          @book = Book.new
+
+          @book.team = Team.find(team_id)
+
+          scorer_name = params_to_use[:scorer][:name]
+          unless Scorer.exists?(name: scorer_name)
+            @book.errors.add(:base, "Scorer with name '#{scorer_name}' needs to be migrated over first.")
+          end
+
+          selection_strategy_name = params_to_use[:selection_strategy][:name]
+          unless SelectionStrategy.exists?(name: selection_strategy_name)
+            @book.errors.add(:selection_strategy,
+                             "Selection strategy with name '#{selection_strategy_name}' needs to be migrated over first.")
+          end
+
+          if params_to_use[:query_doc_pairs]
+            list_of_emails_of_users = []
+            params_to_use[:query_doc_pairs].each do |query_doc_pair|
+              next unless query_doc_pair[:judgements]
+
+              query_doc_pair[:judgements].each do |judgement|
+                list_of_emails_of_users << judgement[:user_email]
+              end
+            end
+            list_of_emails_of_users.uniq!
+            list_of_emails_of_users.each do |email|
+              unless User.exists?(email: email)
+                @book.errors.add(:base, "User with email '#{email}' needs to be migrated over first.")
+              end
+            end
+          end
+
+          unless @book.errors.empty?
+            render json: @book.errors, status: :bad_request
+            return
+          end
+
+          # passed first set of validations.
+          @book.name = params_to_use[:name]
+          @book.show_rank = params_to_use[:show_rank]
+          @book.support_implicit_judgements = params_to_use[:support_implicit_judgements]
+
+          @book.scorer = Scorer.find_by(name: scorer_name)
+          @book.selection_strategy = SelectionStrategy.find_by(name: selection_strategy_name)
+
+          params_to_use[:query_doc_pairs]&.each do |query_doc_pair|
+            qdp = @book.query_doc_pairs.build(query_doc_pair.except(:judgements))
+            next unless query_doc_pair[:judgements]
+
+            query_doc_pair[:judgements].each do |judgement|
+              judgement[:user] = User.find_by(email: judgement[:user_email])
+              qdp.judgements.build(judgement.except(:user_email))
+            end
+          end
+
+          if @book.save
+            respond_with @book
+          else
+            render json: @book.errors, status: :bad_request
+          end
+        end
+        # rubocop:enable Metrics/MethodLength
+        # rubocop:enable Metrics/AbcSize
+        # rubocop:enable Metrics/CyclomaticComplexity
+        # rubocop:enable Metrics/PerceivedComplexity
+        # rubocop:enable Layout/LineLength
+
+        private
+
+        def book_params
+          params.require(:book).permit!
+        end
+      end
+    end
+  end
+end
diff --git a/app/views/api/v1/books/_book.json.jbuilder b/app/views/api/v1/books/_book.json.jbuilder
index c91ba9533..d5c4bfc96 100644
--- a/app/views/api/v1/books/_book.json.jbuilder
+++ b/app/views/api/v1/books/_book.json.jbuilder
@@ -1,28 +1,4 @@
 # frozen_string_literal: true
 
-export = @export ||= false
-
 json.name        book.name
-json.book_id     book.id unless export
-json.show_rank   book.show_rank
-json.support_implicit_judgements book.support_implicit_judgements
-
-if export
-  if book.scorer.present?
-    json.scorer do
-      json.partial! 'api/v1/scorers/scorer', scorer: book.scorer, export: export
-    end
-  end
-
-  if book.selection_strategy.present?
-    json.selection_strategy do
-      json.partial! 'selection_strategy', selection_strategy: book.selection_strategy
-    end
-  end
-end
-
-json.query_doc_pairs do
-  json.array! book.query_doc_pairs,
-              partial: 'api/v1/books/query_doc_pairs', as: :query_doc_pair,
-              export: export
-end
+json.book_id     book.id
diff --git a/app/views/api/v1/books/_judgements.json.jbuilder b/app/views/api/v1/books/_judgements.json.jbuilder
deleted file mode 100644
index fa0f85148..000000000
--- a/app/views/api/v1/books/_judgements.json.jbuilder
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-json.judgement_id   judgement.id unless export
-json.rating         judgement.rating
-json.unrateable     judgement.unrateable
-json.user_id        judgement.user_id unless export
-if judgement.user && export
-  json.user_email judgement.user.email
-  json.user_name judgement.user.name
-end
diff --git a/app/views/api/v1/books/_query_doc_pairs.json.jbuilder b/app/views/api/v1/books/_query_doc_pairs.json.jbuilder
deleted file mode 100644
index f683c16b6..000000000
--- a/app/views/api/v1/books/_query_doc_pairs.json.jbuilder
+++ /dev/null
@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-
-json.query_doc_pair_id  query_doc_pair.id unless export
-json.position           query_doc_pair.position
-json.query              query_doc_pair.query_text
-json.doc_id             query_doc_pair.doc_id
-
-json.document_fields query_doc_pair.document_fields if export
-
-json.judgements do
-  json.array! query_doc_pair.judgements, partial: 'api/v1/books/judgements', as: :judgement, export: export
-end
diff --git a/app/views/api/v1/books/create.json.jbuilder b/app/views/api/v1/books/create.json.jbuilder
index 79d02b9eb..d0e4b27d4 100644
--- a/app/views/api/v1/books/create.json.jbuilder
+++ b/app/views/api/v1/books/create.json.jbuilder
@@ -1,3 +1,3 @@
 # frozen_string_literal: true
 
-json.partial! 'book', book: @book, export: false
+json.partial! 'book', book: @book
diff --git a/app/views/api/v1/books/show.json.jbuilder b/app/views/api/v1/books/show.json.jbuilder
index 79d02b9eb..d0e4b27d4 100644
--- a/app/views/api/v1/books/show.json.jbuilder
+++ b/app/views/api/v1/books/show.json.jbuilder
@@ -1,3 +1,3 @@
 # frozen_string_literal: true
 
-json.partial! 'book', book: @book, export: false
+json.partial! 'book', book: @book
diff --git a/app/views/api/v1/books/update.json.jbuilder b/app/views/api/v1/books/update.json.jbuilder
index 79d02b9eb..d0e4b27d4 100644
--- a/app/views/api/v1/books/update.json.jbuilder
+++ b/app/views/api/v1/books/update.json.jbuilder
@@ -1,3 +1,3 @@
 # frozen_string_literal: true
 
-json.partial! 'book', book: @book, export: false
+json.partial! 'book', book: @book
diff --git a/app/views/api/v1/export/books/_book.json.jbuilder b/app/views/api/v1/export/books/_book.json.jbuilder
new file mode 100644
index 000000000..73ffc874e
--- /dev/null
+++ b/app/views/api/v1/export/books/_book.json.jbuilder
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+json.name        book.name
+json.show_rank   book.show_rank
+json.support_implicit_judgements book.support_implicit_judgements
+
+if book.scorer.present?
+  json.scorer do
+    json.partial! 'api/v1/scorers/scorer', scorer: book.scorer, export: true
+  end
+end
+
+if book.selection_strategy.present?
+  json.selection_strategy do
+    json.partial! 'selection_strategy', selection_strategy: book.selection_strategy
+  end
+end
+
+json.query_doc_pairs do
+  json.array! book.query_doc_pairs,
+              partial: 'query_doc_pair', as: :query_doc_pair
+end
diff --git a/app/views/api/v1/export/books/_judgements.json.jbuilder b/app/views/api/v1/export/books/_judgements.json.jbuilder
new file mode 100644
index 000000000..f2c498106
--- /dev/null
+++ b/app/views/api/v1/export/books/_judgements.json.jbuilder
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+json.rating         judgement.rating
+json.unrateable     judgement.unrateable
+json.user_email judgement.user.email if judgement.user
diff --git a/app/views/api/v1/export/books/_query_doc_pair.json.jbuilder b/app/views/api/v1/export/books/_query_doc_pair.json.jbuilder
new file mode 100644
index 000000000..65866c4aa
--- /dev/null
+++ b/app/views/api/v1/export/books/_query_doc_pair.json.jbuilder
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+json.query_text      query_doc_pair.query_text
+json.doc_id          query_doc_pair.doc_id
+json.position        query_doc_pair.position
+json.document_fields query_doc_pair.document_fields
+
+json.judgements do
+  json.array! query_doc_pair.judgements, partial: 'judgements', as: :judgement
+end
diff --git a/app/views/api/v1/books/_selection_strategy.json.jbuilder b/app/views/api/v1/export/books/_selection_strategy.json.jbuilder
similarity index 100%
rename from app/views/api/v1/books/_selection_strategy.json.jbuilder
rename to app/views/api/v1/export/books/_selection_strategy.json.jbuilder
diff --git a/app/views/api/v1/export/books/show.json.jbuilder b/app/views/api/v1/export/books/show.json.jbuilder
new file mode 100644
index 000000000..d0e4b27d4
--- /dev/null
+++ b/app/views/api/v1/export/books/show.json.jbuilder
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+json.partial! 'book', book: @book
diff --git a/config/routes.rb b/config/routes.rb
index d3b89c473..0d54cd5d7 100755
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -174,6 +174,7 @@
 
       # Imports
       namespace :import do
+        resources :books, only: [ :create ]
         resources :ratings, only: [ :create ]
         namespace :queries do
           resources :information_needs, only: [ :create ], param: :case_id
@@ -182,6 +183,7 @@
 
       # Exports
       namespace :export do
+        resources :books, only: [ :show ], param: :book_id
         resources :ratings, only: [ :show ], param: :case_id
         namespace :queries do
           resources :information_needs, only: [ :show ], param: :case_id
diff --git a/test/controllers/api/v1/books_controller_test.rb b/test/controllers/api/v1/books_controller_test.rb
index fc6ccf982..98afe8795 100644
--- a/test/controllers/api/v1/books_controller_test.rb
+++ b/test/controllers/api/v1/books_controller_test.rb
@@ -89,61 +89,10 @@ class BooksControllerTest < ActionController::TestCase
           assert_response :ok
 
           body = response.parsed_body
-          assert_not assigns(:export)
 
           assert_equal body['name'].size, book.name.size
-          assert_equal body['query_doc_pairs'][0]['query'], book.query_doc_pairs[0].query_text
-          assert_nil body['query_doc_pairs'][0]['document_fields']
-        end
-
-        test 'returns book info with export specified' do
-          get :show, params: { id: book.id, export: false }
-          assert_response :ok
-
-          body = response.parsed_body
-          assert_not assigns(:export)
-
-          assert_equal body['name'].size, book.name.size
-          assert_equal body['query_doc_pairs'][0]['query'], book.query_doc_pairs[0].query_text
-          assert_nil body['query_doc_pairs'][0]['document_fields']
-        end
-
-        test 'returns detailed query_doc_pair and judgement info' do
-          get :show, params: { id: book.id, export: true }
-          assert_response :ok
-
-          body = response.parsed_body
-          puts body
-          assert assigns(:export)
-
-          assert_equal body['name'].size, book.name.size
-          assert_equal body['query_doc_pairs'][0]['query'], book.query_doc_pairs[0].query_text
-          assert_equal body['query_doc_pairs'][0]['document_fields'], book.query_doc_pairs[0].document_fields
-        end
-      end
-
-      describe 'Exporting a book in json' do
-        let(:book)        { books(:james_bond_movies) }
-        let(:judgement)   { judgements(:jbm_qdp10_judgement) }
-        let(:doug)        { users(:doug) }
-        let(:random_user) { users(:random) }
-
-        test 'the AR object ids are replaced with names' do
-          get :show, params: { id: book.id, export: true }
-          assert_response :ok
-          body = response.parsed_body
-          require 'json'
-          puts JSON.pretty_generate(body)
-          assert_nil body['book_id']
-          assert_not_nil body['name']
-
-          assert_nil body['scorer_id']
-          assert_not_nil body['scorer']
-          assert_nil body['scorer']['scorer_id']
-          assert_empty body['scorer']['teams']
-
-          assert_nil body['selection_strategy_id']
-          assert_not_nil body['selection_strategy']
+          # assert_equal body['query_doc_pairs'][0]['query'], book.query_doc_pairs[0].query_text
+          # assert_nil body['query_doc_pairs'][0]['document_fields']
         end
       end
 
diff --git a/test/controllers/api/v1/export/books_controller_test.rb b/test/controllers/api/v1/export/books_controller_test.rb
new file mode 100644
index 000000000..f958063ad
--- /dev/null
+++ b/test/controllers/api/v1/export/books_controller_test.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+require 'csv'
+module Api
+  module V1
+    module Export
+      class BooksControllerTest < ActionController::TestCase
+        let(:doug) { users(:doug) }
+
+        before do
+          @controller = Api::V1::Export::BooksController.new
+
+          login_user doug
+        end
+
+        describe 'Exporting a book in json' do
+          let(:book)        { books(:james_bond_movies) }
+          let(:doug)        { users(:doug) }
+
+          test 'the AR object ids are replaced with names' do
+            get :show, params: { book_id: book.id }
+            assert_response :ok
+            body = response.parsed_body
+            require 'json'
+            puts JSON.pretty_generate(body)
+            assert_nil body['book_id']
+            assert_not_nil body['name']
+
+            assert_nil body['scorer_id']
+            assert_not_nil body['scorer']
+            assert_nil body['scorer']['scorer_id']
+            assert_empty body['scorer']['teams']
+
+            assert_nil body['selection_strategy_id']
+            assert_not_nil body['selection_strategy']
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/test/controllers/api/v1/import/books_controller_test.rb b/test/controllers/api/v1/import/books_controller_test.rb
new file mode 100644
index 000000000..c6256ebe4
--- /dev/null
+++ b/test/controllers/api/v1/import/books_controller_test.rb
@@ -0,0 +1,145 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+module Api
+  module V1
+    module Import
+      class BooksControllerTest < ActionController::TestCase
+        let(:team) { teams(:shared) }
+        let(:user) { users(:random) }
+        let(:doug) { users(:doug) }
+        let(:acase) { cases(:import_ratings_case) }
+        let(:query) { queries(:import_ratings_query) }
+        let(:book) { books(:james_bond_movies) }
+
+        before do
+          @controller = Api::V1::Import::BooksController.new
+
+          login_user user
+        end
+
+        describe '#create' do
+          test 'alerts when a team_id is not provided' do
+            data = {
+              name: 'test book',
+            }
+            assert_raises(ActionController::ParameterMissing) do
+              post :create, params: { book: data, format: :json }
+            end
+          end
+          test 'alerts when a user assocated with a judgement does not exist' do
+            data = {
+              name:               'test book',
+              scorer:             book.scorer.as_json(only: [ :name ]),
+              selection_strategy: book.selection_strategy.as_json(only: [ :name ]),
+              query_doc_pairs:    [
+                {
+                  query_text: 'dog', doc_id: '123', position: 1,
+                  judgements: [
+                    {
+                      rating:     1.0,
+                      unrateable: false,
+                      user_email: 'fakeuser@fake.com',
+                      user_name:  'Fake',
+                    },
+                    {
+                      rating:     2.0,
+                      unrateable: false,
+                      user_email: 'random@example.com',
+                      user_name:  'Random User',
+                    }
+                  ]
+                },
+                { query_text: 'dog', doc_id: '234' },
+                { query_text: 'dog', doc_id: '456',
+                  judgements: [
+                    {
+                      rating:     1.0,
+                      unrateable: false,
+                    }
+                  ] }
+              ],
+            }
+
+            post :create, params: { book: data, team_id: team.id, format: :json }
+
+            assert_response :bad_request
+
+            body = response.parsed_body
+
+            assert_includes body['base'], "User with email 'fakeuser@fake.com' needs to be migrated over first."
+            assert_nil Book.find_by(name: 'test book')
+          end
+
+          test 'alerts when a scorer associated with a book does not exist' do
+            data = {
+              name:               'test book',
+              scorer:             book.scorer.as_json(only: [ :name ]),
+              selection_strategy: {
+                name: 'fake selection',
+              },
+              query_doc_pairs:    [],
+            }
+
+            post :create, params: { book: data, team_id: team.id, format: :json }
+
+            assert_response :bad_request
+
+            body = response.parsed_body
+
+            assert_includes body['selection_strategy'],
+                            "Selection strategy with name 'fake selection' needs to be migrated over first."
+            assert_nil Book.find_by(name: 'test book')
+          end
+
+          test 'creates a new book' do
+            data = {
+              name:               'test book',
+              scorer:             book.scorer.as_json(only: [ :name ]),
+              selection_strategy: book.selection_strategy.as_json(only: [ :name ]),
+              query_doc_pairs:    [
+                {
+                  query_text: 'dog', doc_id: '123',
+                  judgements: [
+                    {
+                      rating:     1.0,
+                      unrateable: false,
+                      user_email: user.email,
+                    },
+                    {
+                      rating:     2.0,
+                      unrateable: false,
+                      user_email: doug.email,
+                    }
+                  ]
+                },
+                { query_text: 'dog', doc_id: '234' },
+                { query_text: 'dog', doc_id: '456',
+                  judgements: [
+                    {
+                      rating:     1.0,
+                      unrateable: false,
+                    }
+                  ] }
+              ],
+            }
+
+            post :create, params: { book: data, team_id: team.id, format: :json }
+
+            assert_response :created
+
+            @book = Book.find_by(name: 'test book')
+
+            assert_not_nil @book
+
+            assert_equal 3, @book.query_doc_pairs.count
+            assert_equal 3, @book.judgements.count
+
+            response.parsed_body
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/test/fixtures/query_doc_pairs.yml b/test/fixtures/query_doc_pairs.yml
index 863edacf8..a769a612e 100644
--- a/test/fixtures/query_doc_pairs.yml
+++ b/test/fixtures/query_doc_pairs.yml
@@ -82,7 +82,7 @@ jbm_qdp10:
   query_text: Action Movies
   position: 1
   doc_id: Moonraker
-  document_fields: MyTex
+  document_fields: '{"title":"Moonraker", "year":2023}'
   book: :james_bond_movies
   
 starwars_qdp1:
diff --git a/test/integration/api/export_import_book_flow_test.rb b/test/integration/api/export_import_book_flow_test.rb
index 40eabb523..34dff1f49 100644
--- a/test/integration/api/export_import_book_flow_test.rb
+++ b/test/integration/api/export_import_book_flow_test.rb
@@ -6,13 +6,14 @@ class ExportImportBookFlowTest < ActionDispatch::IntegrationTest
   include ActionMailer::TestHelper
 
   let(:book) { books(:james_bond_movies) }
+  let(:team) { teams(:shared) }
 
   test 'Export a complete book, and then modify the name, the scorer, and reimport it with same users' do
     post users_login_url params: { user: { email: 'doug@example.com', password: 'password' }, format: :json }
 
     # export the book
     Bullet.enable = false # we have extra nesting we don't care about
-    get api_book_url(book), params: { export: true }
+    get api_export_book_url(book)
     Bullet.enable = true
 
     assert_response :ok
@@ -21,10 +22,14 @@ class ExportImportBookFlowTest < ActionDispatch::IntegrationTest
 
     # Modify the book into a NEW book and import.
     response_json['name'] = 'New James Bond Movies'
-    response_json['scorer']['name'] = 'New James Bond Movies Scorer'
 
     puts JSON.pretty_generate(response_json)
 
-    post users_login_url params: { user: { email: 'doug@example.com', password: 'password' }, format: :json }
+    post api_import_books_url params: { team_id: team.id, book: response_json, format: :json }
+
+    puts response.parsed_body
+
+    new_book = Book.find(response.parsed_body['id'])
+    assert_not_nil(new_book)
   end
 end