From b3df38207d0a719127f1b4397edf60d3dc2c05be Mon Sep 17 00:00:00 2001
From: Janell-Huyck <huyckjl@ucmail.uc.edu>
Date: Thu, 18 Jan 2024 17:08:36 -0500
Subject: [PATCH 1/5] Add more comprehensive testing for publications
 controller

---
 ...ications_controller_access_control_spec.rb |  22 +++
 .../publications_controller_create_spec.rb    |  83 +++++++++++
 .../publications_controller_destroy_spec.rb   | 126 ++++++++++++++++
 .../publications_controller_index_spec.rb     |  77 ++++++++++
 .../publications_controller_update_spec.rb    | 138 ++++++++++++++++++
 .../publications_controller_spec.rb           |  26 ----
 .../helpers/login_helpers_for_unit_tests.rb   |   6 +
 7 files changed, 452 insertions(+), 26 deletions(-)
 create mode 100644 spec/controllers/publications_controller/publications_controller_access_control_spec.rb
 create mode 100644 spec/controllers/publications_controller/publications_controller_create_spec.rb
 create mode 100644 spec/controllers/publications_controller/publications_controller_destroy_spec.rb
 create mode 100644 spec/controllers/publications_controller/publications_controller_index_spec.rb
 create mode 100644 spec/controllers/publications_controller/publications_controller_update_spec.rb
 delete mode 100644 spec/controllers/publications_controller_spec.rb

diff --git a/spec/controllers/publications_controller/publications_controller_access_control_spec.rb b/spec/controllers/publications_controller/publications_controller_access_control_spec.rb
new file mode 100644
index 00000000..7121b382
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_access_control_spec.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  it_behaves_like 'restricts non-logged-in users', {
+    'index' => :get,
+    'show' => :get,
+    'new' => :get,
+    'edit' => :get,
+    'create' => :post,
+    'update' => :put,
+    'destroy' => :delete
+  }
+end
diff --git a/spec/controllers/publications_controller/publications_controller_create_spec.rb b/spec/controllers/publications_controller/publications_controller_create_spec.rb
new file mode 100644
index 00000000..8d23721b
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_create_spec.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  let(:submitter_id) { submitter.id.to_s }
+  let(:another_submitter_id) { another_submitter.id.to_s }
+
+  let(:valid_attributes) do
+    { 'author_first_name' => %w[Test Person],
+      'author_last_name' => %w[Case 2],
+      'college_ids' => ['', '1', '2'],
+      'uc_department' => 'Dept',
+      'work_title' => 'WT',
+      'other_title' => 'OT',
+      'publisher' => 'Pub',
+      'city' => 'City',
+      'publication_date' => 'Today',
+      'url' => 'www.fake.com',
+      'doi' => 'doi:' }
+  end
+
+  let(:invalid_attributes) do
+    { 'author_first_name' => ['Bad'],
+      'author_last_name' => [''],
+      'college_ids' => [''],
+      'uc_department' => '',
+      'work_title' => '',
+      'other_title' => '',
+      'publisher' => '',
+      'city' => '',
+      'publication_date' => '',
+      'url' => '',
+      'doi' => '' }
+  end
+
+  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
+  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
+
+  describe 'POST #create' do
+    before do
+      already_created_book_by_submitter
+      login_as_submitter_of(already_created_book_by_submitter)
+    end
+
+    context 'with valid params' do
+      it 'creates a new Book' do
+        expect do
+          post :create, params: { book: valid_attributes }
+        end.to change(Book, :count).by(1)
+      end
+
+      it 'redirects to the publications path after creation' do
+        post :create, params: { book: valid_attributes }
+        expect(response).to redirect_to(publications_path)
+      end
+    end
+
+    context 'with invalid params' do
+      it 'does not create a new Book' do
+        expect do
+          post :create, params: { book: invalid_attributes }
+        end.not_to change(Book, :count)
+      end
+
+      it "redirects to the 'new' template with status 'unprocessable_entity'" do
+        post :create, params: { book: invalid_attributes }
+        expect(response).to render_template(:new)
+        expect(response.status).to eql 422
+      end
+    end
+  end
+end
diff --git a/spec/controllers/publications_controller/publications_controller_destroy_spec.rb b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
new file mode 100644
index 00000000..581a2eae
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  let(:submitter_id) { submitter.id.to_s }
+  let(:another_submitter_id) { another_submitter.id.to_s }
+
+  let(:valid_attributes) do
+    { 'author_first_name' => %w[Test Person],
+      'author_last_name' => %w[Case 2],
+      'college_ids' => ['', '1', '2'],
+      'uc_department' => 'Dept',
+      'work_title' => 'WT',
+      'other_title' => 'OT',
+      'publisher' => 'Pub',
+      'city' => 'City',
+      'publication_date' => 'Today',
+      'url' => 'www.fake.com',
+      'doi' => 'doi:' }
+  end
+
+  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
+  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
+
+  describe 'DELETE #destroy' do
+    before do
+      already_created_book_by_submitter
+      already_created_book_by_another_submitter
+    end
+
+    context 'when attempting to delete books as a submitter' do
+      before do
+        login_as_submitter_of(already_created_book_by_submitter)
+      end
+
+      context 'and the book belongs to the submitter' do
+        it 'destroys only the requested book' do
+          expect { Book.find(already_created_book_by_submitter.id) }.not_to raise_error
+          expect do
+            delete :destroy, params: { id: already_created_book_by_submitter.id }
+          end.to change(Book, :count).by(-1)
+          expect { Book.find(already_created_book_by_submitter.id) }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+
+        it 'redirects to the publications_path' do
+          delete :destroy, params: { id: already_created_book_by_submitter.id }
+          expect(response).to redirect_to(publications_path)
+        end
+
+        it 'displays a success flash notice' do
+          delete :destroy, params: { id: already_created_book_by_submitter.id }
+          expect(flash[:warning]).to eql 'Book was successfully destroyed.'
+        end
+      end
+
+      context 'and the book does not belong to the submitter' do
+        it 'does not destroy the requested book' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
+          expect do
+            delete :destroy, params: { id: already_created_book_by_another_submitter.id }
+          end.not_to change(Book, :count)
+          expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
+        end
+
+        it 'redirects to the publications_path' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          delete :destroy, params: { id: already_created_book_by_another_submitter.id }
+          expect(response).to redirect_to(publications_path)
+        end
+
+        it 'displays a flash alert' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          delete :destroy, params: { id: already_created_book_by_another_submitter.id }
+          expect(flash[:danger]).to eql 'You are not authorized to delete this publication.'
+        end
+      end
+    end
+
+    context 'when attempting to delete books as an admin' do
+      before do
+        login_as_admin
+      end
+
+      it 'destroys the requested books by either submitter' do
+        expect { Book.find(already_created_book_by_submitter.id) }.not_to raise_error
+        expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
+
+        expect do
+          delete :destroy, params: { id: already_created_book_by_submitter.id }
+        end.to change(Book, :count).by(-1)
+        expect do
+          delete :destroy, params: { id: already_created_book_by_another_submitter.id }
+        end.to change(Book, :count).by(-1)
+
+        expect do
+          Book.find(already_created_book_by_submitter.id)
+        end.to raise_error(ActiveRecord::RecordNotFound)
+        expect do
+          Book.find(already_created_book_by_another_submitter.id)
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'redirects to the publications_path' do
+        delete :destroy, params: { id: already_created_book_by_submitter.id }
+        expect(response).to redirect_to(publications_path)
+      end
+
+      it 'displays a success flash notice' do
+        delete :destroy, params: { id: already_created_book_by_submitter.id }
+        expect(flash[:warning]).to eql 'Book was successfully destroyed.'
+      end
+    end
+  end
+end
diff --git a/spec/controllers/publications_controller/publications_controller_index_spec.rb b/spec/controllers/publications_controller/publications_controller_index_spec.rb
new file mode 100644
index 00000000..b97605c2
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_index_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  let(:submitter_id) { submitter.id.to_s }
+  let(:another_submitter_id) { another_submitter.id.to_s }
+
+  let(:valid_attributes) do
+    { 'author_first_name' => %w[Test Person],
+      'author_last_name' => %w[Case 2],
+      'college_ids' => ['', '1', '2'],
+      'uc_department' => 'Dept',
+      'work_title' => 'WT',
+      'other_title' => 'OT',
+      'publisher' => 'Pub',
+      'city' => 'City',
+      'publication_date' => 'Today',
+      'url' => 'www.fake.com',
+      'doi' => 'doi:' }
+  end
+
+  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
+  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
+
+  # We have some custom behavior for the index action.  Firstly, it redirects
+  # the user to the publications path.  Then it calls a series of partials
+  # that are indexes of each of the publication types.
+  describe 'GET #index' do
+    before do
+      already_created_book_by_submitter
+      already_created_book_by_another_submitter
+    end
+
+    context 'when a user is logged in as an admin' do
+      before do
+        login_as_admin
+        get :index
+      end
+
+      it 'assigns all books to @books' do
+        expect(assigns(:books)).to include(already_created_book_by_submitter)
+        expect(assigns(:books)).to include(already_created_book_by_another_submitter)
+      end
+
+      it 'redirects to publications path' do
+        expect(response).to redirect_to(publications_path)
+      end
+    end
+
+    context 'when a user is logged in as a submitter' do
+      before do
+        login_as_submitter_of(already_created_book_by_submitter)
+        get :index
+      end
+
+      it 'assigns only the submitter\'s books to @books' do
+        expect(assigns(:books)).to eq([already_created_book_by_submitter])
+        expect(assigns(:books)).not_to include(already_created_book_by_another_submitter)
+      end
+
+      it 'redirects to publications path' do
+        expect(response).to redirect_to(publications_path)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/publications_controller/publications_controller_update_spec.rb b/spec/controllers/publications_controller/publications_controller_update_spec.rb
new file mode 100644
index 00000000..79eeba4a
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_update_spec.rb
@@ -0,0 +1,138 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  let(:submitter_id) { submitter.id.to_s }
+  let(:another_submitter_id) { another_submitter.id.to_s }
+
+  let(:valid_attributes) do
+    { 'author_first_name' => %w[Test Person],
+      'author_last_name' => %w[Case 2],
+      'college_ids' => ['', '1', '2'],
+      'uc_department' => 'Dept',
+      'work_title' => 'WT',
+      'other_title' => 'OT',
+      'publisher' => 'Pub',
+      'city' => 'City',
+      'publication_date' => 'Today',
+      'url' => 'www.fake.com',
+      'doi' => 'doi:' }
+  end
+
+  let(:new_attributes) do
+    { 'author_first_name' => %w[New Person],
+      'author_last_name' => %w[Person 2] }
+  end
+
+  let(:invalid_attributes) do
+    { 'author_first_name' => ['Bad'],
+      'author_last_name' => [''] }
+  end
+
+  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
+  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
+
+  describe 'PUT #update' do
+    before do
+      already_created_book_by_submitter
+      already_created_book_by_another_submitter
+    end
+
+    context 'when attempting to update a book as a submitter' do
+      before do
+        login_as_submitter_of(already_created_book_by_submitter)
+      end
+
+      context 'and the book belongs to the submitter' do
+        context 'and updating with valid params' do
+          it 'updates the requested book' do
+            put :update, params: { id: already_created_book_by_submitter.id, book: new_attributes }
+            already_created_book_by_submitter.reload
+            expect(already_created_book_by_submitter.author_first_name).to eql %w[New Person]
+            expect(already_created_book_by_submitter.author_last_name).to eql %w[Person 2]
+          end
+
+          it 'redirects to the book' do
+            put :update, params: { id: already_created_book_by_submitter.id, book: valid_attributes }
+            expect(response).to redirect_to(already_created_book_by_submitter)
+          end
+
+          it 'displays a success flash notice' do
+            put :update, params: { id: already_created_book_by_submitter.to_param, book: valid_attributes }
+            expect(flash[:success]).to eql 'Book was successfully updated.'
+          end
+        end
+
+        context 'and updating with invalid params' do
+          it 'does not update the requested book' do
+            put :update, params: { id: already_created_book_by_submitter.id, book: invalid_attributes }
+            already_created_book_by_submitter.reload
+            expect(already_created_book_by_submitter.author_first_name).to eql %w[Test Person]
+            expect(already_created_book_by_submitter.author_last_name).to eql %w[Case 2]
+          end
+
+          it "redirects to the 'edit' template with status 'unprocessable_entity'" do
+            put :update, params: { id: already_created_book_by_submitter.to_param, book: invalid_attributes }
+            expect(response).to render_template(:edit)
+            expect(response.status).to eql 422
+          end
+        end
+      end
+
+      context 'and the book does not belong to the submitter' do
+        it 'does not update the requested book' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
+          already_created_book_by_another_submitter.reload
+          expect(already_created_book_by_another_submitter.author_first_name).to eql %w[Test Person]
+          expect(already_created_book_by_another_submitter.author_last_name).to eql %w[Case 2]
+        end
+
+        it 'redirects to the publications_path' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
+          expect(response).to redirect_to(publications_path)
+        end
+
+        it 'displays a flash alert' do
+          skip 'waiting for related logic to be merged from PR # 323'
+          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
+          expect(flash[:danger]).to eql 'You are not authorized to edit this publication.'
+        end
+      end
+    end
+
+    context 'when attempting to update a book as an admin' do
+      before do
+        login_as_admin
+      end
+      it 'updates the requested book' do
+        put :update, params: { id: already_created_book_by_submitter.id, book: new_attributes }
+        already_created_book_by_submitter.reload
+        expect(already_created_book_by_submitter.author_first_name).to eql %w[New Person]
+        expect(already_created_book_by_submitter.author_last_name).to eql %w[Person 2]
+      end
+
+      it 'redirects to the book' do
+        put :update, params: { id: already_created_book_by_submitter.to_param, book: valid_attributes }
+        expect(response).to redirect_to(already_created_book_by_submitter)
+      end
+
+      it 'displays a success flash notice' do
+        put :update, params: { id: already_created_book_by_submitter.to_param, book: valid_attributes }
+        expect(flash[:success]).to eql 'Book was successfully updated.'
+      end
+    end
+  end
+end
diff --git a/spec/controllers/publications_controller_spec.rb b/spec/controllers/publications_controller_spec.rb
deleted file mode 100644
index a2938ba0..00000000
--- a/spec/controllers/publications_controller_spec.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe PublicationsController, type: :controller do
-  let(:valid_session) { { submitter_id: 1 } }
-
-  describe 'GET #index' do
-    before do
-      Submitter.create!(
-        first_name: 'First Name',
-        last_name: 'Last Name',
-        college: 2,
-        department: 'Department',
-        mailing_address: 'Mailing Address',
-        phone_number: '111-111-1111',
-        email_address: 'test@mail.uc.edu'
-      )
-    end
-
-    it 'returns a success response' do
-      get :index, session: valid_session
-      expect(response).to be_successful
-    end
-  end
-end
diff --git a/spec/support/helpers/login_helpers_for_unit_tests.rb b/spec/support/helpers/login_helpers_for_unit_tests.rb
index daf3caff..56d9c6dd 100644
--- a/spec/support/helpers/login_helpers_for_unit_tests.rb
+++ b/spec/support/helpers/login_helpers_for_unit_tests.rb
@@ -23,4 +23,10 @@ def extract_submitter_id_from(resource)
 
 def log_in_submitter(submitter_id)
   session[:submitter_id] = submitter_id
+  session.delete(:admin)
+end
+
+def login_as_admin
+  session[:admin] = true
+  session.delete(:submitter_id)
 end

From ef924812a1314097f4cd9a05f21006310cd53cd2 Mon Sep 17 00:00:00 2001
From: Janell-Huyck <huyckjl@ucmail.uc.edu>
Date: Tue, 13 Feb 2024 16:48:26 -0500
Subject: [PATCH 2/5] Add test for show

---
 .../publications_controller_show_spec.rb      | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 spec/controllers/publications_controller/publications_controller_show_spec.rb

diff --git a/spec/controllers/publications_controller/publications_controller_show_spec.rb b/spec/controllers/publications_controller/publications_controller_show_spec.rb
new file mode 100644
index 00000000..7085ddf9
--- /dev/null
+++ b/spec/controllers/publications_controller/publications_controller_show_spec.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+# The PublicationsController is responsible for handling the requests for all
+# publications.  Each model inherits its CRUD actions from this controller.
+# We do not actually have a Publications model, so for testing purposes
+# we will use the Book model, which is a child of the Publication model.
+#
+require 'rails_helper'
+
+# We are calling BooksController, a "child" of PublicationsController, so we
+# need to define it here.
+RSpec.describe BooksController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  let(:submitter_id) { submitter.id.to_s }
+  let(:another_submitter_id) { another_submitter.id.to_s }
+
+  let(:valid_attributes) do
+    { 'author_first_name' => %w[Test Person],
+      'author_last_name' => %w[Case 2],
+      'college_ids' => ['', '1', '2'],
+      'uc_department' => 'Dept',
+      'work_title' => 'WT',
+      'other_title' => 'OT',
+      'publisher' => 'Pub',
+      'city' => 'City',
+      'publication_date' => 'Today',
+      'url' => 'www.fake.com',
+      'doi' => 'doi:' }
+  end
+
+  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
+  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
+
+  # Check to make sure that the correct @submitter is set when show is called
+  # and that the correct template is rendered when appropriate.
+  describe 'GET #show' do
+    before do
+      already_created_book_by_submitter
+      already_created_book_by_another_submitter
+    end
+
+    context 'when attempting to view books as a submitter' do
+      before do
+        login_as_submitter_of(already_created_book_by_submitter)
+      end
+
+      context 'and the book belongs to the submitter' do
+        it 'leaves @submitter set to the current submitter' do
+          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+          get :show, params: { id: already_created_book_by_submitter.id }
+          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+        end
+
+        it 'renders the show template' do
+          get :show, params: { id: already_created_book_by_submitter.id }
+          expect(response).to render_template('show')
+        end
+      end
+
+      context 'and the book does not belong to the submitter' do
+        it 'leaves @submitter set to the current submitter' do
+          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+          get :show, params: { id: already_created_book_by_another_submitter.id }
+          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+        end
+
+        it 'throws a 404 error' do
+          expect { get :show, params: { id: already_created_book_by_another_submitter.id } }
+            .to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
+    end
+
+    context 'when attempting to delete books as an admin' do
+      before do
+        login_as_admin
+      end
+
+      it 'sets @submitter to the submitter of the requested book' do
+        expect(assigns(:submitter)).to be_nil
+        get :show, params: { id: already_created_book_by_submitter.id }
+        expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+
+        get :show, params: { id: already_created_book_by_another_submitter.id }
+        expect(assigns(:submitter)).to eql already_created_book_by_another_submitter.submitter
+      end
+
+      it 'renders the show template' do
+        get :show, params: { id: already_created_book_by_submitter.id }
+        expect(response).to render_template('show')
+
+        get :show, params: { id: already_created_book_by_another_submitter.id }
+        expect(response).to render_template('show')
+      end
+    end
+  end
+end

From 6b58f1371ec4b6025e36841f60be5dc75cdfb932 Mon Sep 17 00:00:00 2001
From: Janell-Huyck <huyckjl@ucmail.uc.edu>
Date: Tue, 20 Feb 2024 13:30:18 -0500
Subject: [PATCH 3/5] WIP

---
 .ruby-gemset                                  |  2 +-
 .../publications_controller_destroy_spec.rb   |  3 -
 .../publications_controller_show_spec.rb      | 95 ++++++-------------
 3 files changed, 32 insertions(+), 68 deletions(-)

diff --git a/.ruby-gemset b/.ruby-gemset
index 971b5415..8d6c54aa 100644
--- a/.ruby-gemset
+++ b/.ruby-gemset
@@ -1 +1 @@
-aaec
+aaec
\ No newline at end of file
diff --git a/spec/controllers/publications_controller/publications_controller_destroy_spec.rb b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
index 581a2eae..1024111f 100644
--- a/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
@@ -66,7 +66,6 @@
 
       context 'and the book does not belong to the submitter' do
         it 'does not destroy the requested book' do
-          skip 'waiting for related logic to be merged from PR # 323'
           expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
           expect do
             delete :destroy, params: { id: already_created_book_by_another_submitter.id }
@@ -75,13 +74,11 @@
         end
 
         it 'redirects to the publications_path' do
-          skip 'waiting for related logic to be merged from PR # 323'
           delete :destroy, params: { id: already_created_book_by_another_submitter.id }
           expect(response).to redirect_to(publications_path)
         end
 
         it 'displays a flash alert' do
-          skip 'waiting for related logic to be merged from PR # 323'
           delete :destroy, params: { id: already_created_book_by_another_submitter.id }
           expect(flash[:danger]).to eql 'You are not authorized to delete this publication.'
         end
diff --git a/spec/controllers/publications_controller/publications_controller_show_spec.rb b/spec/controllers/publications_controller/publications_controller_show_spec.rb
index 7085ddf9..473c042f 100644
--- a/spec/controllers/publications_controller/publications_controller_show_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_show_spec.rb
@@ -11,88 +11,55 @@
 # need to define it here.
 RSpec.describe BooksController, type: :controller do
   let(:submitter) { FactoryBot.create(:submitter) }
-  let(:another_submitter) { FactoryBot.create(:submitter) }
+  let(:book) { FactoryBot.create(:book, submitter_id: submitter.id.to_s) }
 
-  let(:submitter_id) { submitter.id.to_s }
-  let(:another_submitter_id) { another_submitter.id.to_s }
-
-  let(:valid_attributes) do
-    { 'author_first_name' => %w[Test Person],
-      'author_last_name' => %w[Case 2],
-      'college_ids' => ['', '1', '2'],
-      'uc_department' => 'Dept',
-      'work_title' => 'WT',
-      'other_title' => 'OT',
-      'publisher' => 'Pub',
-      'city' => 'City',
-      'publication_date' => 'Today',
-      'url' => 'www.fake.com',
-      'doi' => 'doi:' }
+  before do
+    book
   end
 
-  let(:already_created_book_by_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id:)) }
-  let(:already_created_book_by_another_submitter) { FactoryBot.create(:book, valid_attributes.merge(submitter_id: another_submitter_id)) }
-
-  # Check to make sure that the correct @submitter is set when show is called
-  # and that the correct template is rendered when appropriate.
   describe 'GET #show' do
-    before do
-      already_created_book_by_submitter
-      already_created_book_by_another_submitter
-    end
-
-    context 'when attempting to view books as a submitter' do
+    context 'as an admin user' do
       before do
-        login_as_submitter_of(already_created_book_by_submitter)
+        session[:admin] = true
+        get :show, params: { id: book.id }
       end
 
-      context 'and the book belongs to the submitter' do
-        it 'leaves @submitter set to the current submitter' do
-          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
-          get :show, params: { id: already_created_book_by_submitter.id }
-          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
-        end
+      it 'sets @submitter correctly' do
+        expect(assigns(:submitter).id.to_s).to eq(book.submitter_id)
+      end
 
-        it 'renders the show template' do
-          get :show, params: { id: already_created_book_by_submitter.id }
-          expect(response).to render_template('show')
-        end
+      it 'renders the show template' do
+        expect(response).to render_template(:show)
       end
 
-      context 'and the book does not belong to the submitter' do
-        it 'leaves @submitter set to the current submitter' do
-          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
-          get :show, params: { id: already_created_book_by_another_submitter.id }
-          expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
+      context 'when the book does not exist' do
+        before do
+          session[:admin] = true
         end
 
-        it 'throws a 404 error' do
-          expect { get :show, params: { id: already_created_book_by_another_submitter.id } }
-            .to raise_error(ActiveRecord::RecordNotFound)
+        it 'responds with a 404 not found' do
+          expect { get :show, params: { id: 'nonexistent' } }.to raise_error(ActiveRecord::RecordNotFound)
         end
       end
     end
 
-    context 'when attempting to delete books as an admin' do
-      before do
-        login_as_admin
-      end
-
-      it 'sets @submitter to the submitter of the requested book' do
-        expect(assigns(:submitter)).to be_nil
-        get :show, params: { id: already_created_book_by_submitter.id }
-        expect(assigns(:submitter)).to eql already_created_book_by_submitter.submitter
-
-        get :show, params: { id: already_created_book_by_another_submitter.id }
-        expect(assigns(:submitter)).to eql already_created_book_by_another_submitter.submitter
-      end
+    context 'as a submitter' do
+      context 'who owns the book' do
+        before do
+          session[:admin] = nil
+          book
+          login_as_submitter_of(book)
+        end
 
-      it 'renders the show template' do
-        get :show, params: { id: already_created_book_by_submitter.id }
-        expect(response).to render_template('show')
+        it 'does not set @submitter' do
+          get :show, params: { id: book.id }
+          expect(assigns(:submitter)).to be_nil
+        end
 
-        get :show, params: { id: already_created_book_by_another_submitter.id }
-        expect(response).to render_template('show')
+        it 'renders the show template' do
+          get :show, params: { id: book.id }
+          expect(response).to render_template(:show)
+        end
       end
     end
   end

From b4b6bab32fd0a4d734498142aa0e59bcfe4b7e15 Mon Sep 17 00:00:00 2001
From: Janell-Huyck <huyckjl@ucmail.uc.edu>
Date: Wed, 21 Feb 2024 14:02:40 -0500
Subject: [PATCH 4/5] Fix tests, increase 'show' resilience

---
 app/controllers/publications_controller.rb    |  7 +++++-
 .../publications_controller_destroy_spec.rb   | 18 ++++-----------
 .../publications_controller_index_spec.rb     |  2 +-
 .../publications_controller_update_spec.rb    | 23 ++++++-------------
 .../book_page_access_spec.rb                  |  2 +-
 .../submitter_page_access_spec.rb             |  2 +-
 .../access_authorization_for_feature_tests.rb |  2 +-
 .../helpers/login_helpers_for_unit_tests.rb   |  2 +-
 8 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/app/controllers/publications_controller.rb b/app/controllers/publications_controller.rb
index e6fb96a3..ca93fccf 100644
--- a/app/controllers/publications_controller.rb
+++ b/app/controllers/publications_controller.rb
@@ -111,7 +111,12 @@ def index
   end
 
   def show
-    @submitter = helpers.find_submitter(instance_variable_get("@#{controller_name.singularize}").id) if session[:admin]
+    resource = controller_name.singularize
+    resource_instance = instance_variable_get("@#{resource}")
+
+    raise ActiveRecord::RecordNotFound unless resource_instance
+
+    @submitter = helpers.find_submitter(resource_instance.id) if session[:admin]
   end
 
   def new
diff --git a/spec/controllers/publications_controller/publications_controller_destroy_spec.rb b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
index 1024111f..78cbb414 100644
--- a/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_destroy_spec.rb
@@ -65,29 +65,21 @@
       end
 
       context 'and the book does not belong to the submitter' do
-        it 'does not destroy the requested book' do
+        it 'raises a 404 error and does not destroy the requested book' do
           expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
+          initial_book_count = Book.count
           expect do
             delete :destroy, params: { id: already_created_book_by_another_submitter.id }
-          end.not_to change(Book, :count)
+          end.to raise_error(ActiveRecord::RecordNotFound)
+          expect(Book.count).to eql initial_book_count
           expect { Book.find(already_created_book_by_another_submitter.id) }.not_to raise_error
         end
-
-        it 'redirects to the publications_path' do
-          delete :destroy, params: { id: already_created_book_by_another_submitter.id }
-          expect(response).to redirect_to(publications_path)
-        end
-
-        it 'displays a flash alert' do
-          delete :destroy, params: { id: already_created_book_by_another_submitter.id }
-          expect(flash[:danger]).to eql 'You are not authorized to delete this publication.'
-        end
       end
     end
 
     context 'when attempting to delete books as an admin' do
       before do
-        login_as_admin
+        login_as_admin_unit_test
       end
 
       it 'destroys the requested books by either submitter' do
diff --git a/spec/controllers/publications_controller/publications_controller_index_spec.rb b/spec/controllers/publications_controller/publications_controller_index_spec.rb
index b97605c2..6efaf9e8 100644
--- a/spec/controllers/publications_controller/publications_controller_index_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_index_spec.rb
@@ -44,7 +44,7 @@
 
     context 'when a user is logged in as an admin' do
       before do
-        login_as_admin
+        login_as_admin_unit_test
         get :index
       end
 
diff --git a/spec/controllers/publications_controller/publications_controller_update_spec.rb b/spec/controllers/publications_controller/publications_controller_update_spec.rb
index 79eeba4a..fb18cb13 100644
--- a/spec/controllers/publications_controller/publications_controller_update_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_update_spec.rb
@@ -91,31 +91,22 @@
       end
 
       context 'and the book does not belong to the submitter' do
-        it 'does not update the requested book' do
-          skip 'waiting for related logic to be merged from PR # 323'
-          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
+        it 'does not update the requested book and raises a 404 error' do
+          expect {
+            put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }}
+            .to raise_error(ActiveRecord::RecordNotFound)
+
           already_created_book_by_another_submitter.reload
+
           expect(already_created_book_by_another_submitter.author_first_name).to eql %w[Test Person]
           expect(already_created_book_by_another_submitter.author_last_name).to eql %w[Case 2]
         end
-
-        it 'redirects to the publications_path' do
-          skip 'waiting for related logic to be merged from PR # 323'
-          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
-          expect(response).to redirect_to(publications_path)
-        end
-
-        it 'displays a flash alert' do
-          skip 'waiting for related logic to be merged from PR # 323'
-          put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
-          expect(flash[:danger]).to eql 'You are not authorized to edit this publication.'
-        end
       end
     end
 
     context 'when attempting to update a book as an admin' do
       before do
-        login_as_admin
+        login_as_admin_unit_test
       end
       it 'updates the requested book' do
         put :update, params: { id: already_created_book_by_submitter.id, book: new_attributes }
diff --git a/spec/features/specific_page_access/book_page_access_spec.rb b/spec/features/specific_page_access/book_page_access_spec.rb
index 96d3cd18..f9a3fc2d 100644
--- a/spec/features/specific_page_access/book_page_access_spec.rb
+++ b/spec/features/specific_page_access/book_page_access_spec.rb
@@ -18,7 +18,7 @@
 
   context 'when admin is logged in' do
     before do
-      login_as_admin
+      login_as_admin_feature_test
       create_book_as_new_submitter
       click_on("I'm Finished")
     end
diff --git a/spec/features/specific_page_access/submitter_page_access_spec.rb b/spec/features/specific_page_access/submitter_page_access_spec.rb
index c2c8d5e8..dcbcee97 100644
--- a/spec/features/specific_page_access/submitter_page_access_spec.rb
+++ b/spec/features/specific_page_access/submitter_page_access_spec.rb
@@ -18,7 +18,7 @@
 
   context 'when admin is logged in' do
     before do
-      login_as_admin
+      login_as_admin_feature_test
     end
 
     it 'allows access to the submitter profile page' do
diff --git a/spec/support/helpers/access_authorization_for_feature_tests.rb b/spec/support/helpers/access_authorization_for_feature_tests.rb
index 48b7c85a..1fd44114 100644
--- a/spec/support/helpers/access_authorization_for_feature_tests.rb
+++ b/spec/support/helpers/access_authorization_for_feature_tests.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-def login_as_admin
+def login_as_admin_feature_test
   visit manage_path
   fill_in('username', with: ENV.fetch('ADMIN_USERNAME', nil))
   fill_in('password', with: ENV.fetch('ADMIN_PASSWORD', nil))
diff --git a/spec/support/helpers/login_helpers_for_unit_tests.rb b/spec/support/helpers/login_helpers_for_unit_tests.rb
index 56d9c6dd..57f3241c 100644
--- a/spec/support/helpers/login_helpers_for_unit_tests.rb
+++ b/spec/support/helpers/login_helpers_for_unit_tests.rb
@@ -26,7 +26,7 @@ def log_in_submitter(submitter_id)
   session.delete(:admin)
 end
 
-def login_as_admin
+def login_as_admin_unit_test
   session[:admin] = true
   session.delete(:submitter_id)
 end

From 926f982dbb5da5240526557a17967f55be459dec Mon Sep 17 00:00:00 2001
From: Janell-Huyck <huyckjl@ucmail.uc.edu>
Date: Wed, 21 Feb 2024 14:56:58 -0500
Subject: [PATCH 5/5] Expand publications controller tests, clarify #show

---
 .ruby-gemset                                  |  2 +-
 app/controllers/publications_controller.rb    |  4 +++-
 .../publications_controller_show_spec.rb      | 21 +++++++++++++------
 .../publications_controller_update_spec.rb    |  5 +++--
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/.ruby-gemset b/.ruby-gemset
index 8d6c54aa..971b5415 100644
--- a/.ruby-gemset
+++ b/.ruby-gemset
@@ -1 +1 @@
-aaec
\ No newline at end of file
+aaec
diff --git a/app/controllers/publications_controller.rb b/app/controllers/publications_controller.rb
index ca93fccf..deec866b 100644
--- a/app/controllers/publications_controller.rb
+++ b/app/controllers/publications_controller.rb
@@ -111,12 +111,14 @@ def index
   end
 
   def show
+    return unless session[:admin]
+
     resource = controller_name.singularize
     resource_instance = instance_variable_get("@#{resource}")
 
     raise ActiveRecord::RecordNotFound unless resource_instance
 
-    @submitter = helpers.find_submitter(resource_instance.id) if session[:admin]
+    @submitter = helpers.find_submitter(resource_instance.id)
   end
 
   def new
diff --git a/spec/controllers/publications_controller/publications_controller_show_spec.rb b/spec/controllers/publications_controller/publications_controller_show_spec.rb
index 473c042f..5dba89cd 100644
--- a/spec/controllers/publications_controller/publications_controller_show_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_show_spec.rb
@@ -11,7 +11,9 @@
 # need to define it here.
 RSpec.describe BooksController, type: :controller do
   let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
   let(:book) { FactoryBot.create(:book, submitter_id: submitter.id.to_s) }
+  let(:another_book) { FactoryBot.create(:book, submitter_id: another_submitter.id.to_s) }
 
   before do
     book
@@ -44,13 +46,13 @@
     end
 
     context 'as a submitter' do
-      context 'who owns the book' do
-        before do
-          session[:admin] = nil
-          book
-          login_as_submitter_of(book)
-        end
+      before do
+        session[:admin] = nil
+        book
+        login_as_submitter_of(book)
+      end
 
+      context 'who owns the book' do
         it 'does not set @submitter' do
           get :show, params: { id: book.id }
           expect(assigns(:submitter)).to be_nil
@@ -61,6 +63,13 @@
           expect(response).to render_template(:show)
         end
       end
+
+      context 'who does not own the book' do
+        it 'raises a 404 not found error and does not set submitter' do
+          expect { get :show, params: { id: another_book.id } }.to raise_error(ActiveRecord::RecordNotFound)
+          expect(assigns(:submitter)).to be_nil
+        end
+      end
     end
   end
 end
diff --git a/spec/controllers/publications_controller/publications_controller_update_spec.rb b/spec/controllers/publications_controller/publications_controller_update_spec.rb
index fb18cb13..8083d18d 100644
--- a/spec/controllers/publications_controller/publications_controller_update_spec.rb
+++ b/spec/controllers/publications_controller/publications_controller_update_spec.rb
@@ -92,8 +92,9 @@
 
       context 'and the book does not belong to the submitter' do
         it 'does not update the requested book and raises a 404 error' do
-          expect {
-            put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }}
+          expect do
+            put :update, params: { id: already_created_book_by_another_submitter.id, book: new_attributes }
+          end
             .to raise_error(ActiveRecord::RecordNotFound)
 
           already_created_book_by_another_submitter.reload