Refactor concern for clarity

app/controllers/concerns/submitter_owned_content_guard.rb → app/controllers/concerns/submitter_ownership_guard.rb

@@ -9,53 +9,46 @@
 # controllers that don't require submitter authentication (e.g. pages, errors, etc.).
 #
 # "Index" is skipped because it has its own logic to determine what can be shown.
-module SubmitterOwnedContentGuard
+module SubmitterOwnershipGuard
   extend ActiveSupport::Concern
 
+  PAGES_VIEWABLE_BY_ALL = %w[errors pages].freeze
+
   included do
-    before_action :verify_user_access, except: %i[index new create login validate finished]
+    before_action :verify_user_ownership, only: %i[show edit update destroy]
   end
 
-  pages_not_requiring_submitter_ownership = %w[errors pages]
-
-  def verify_user_access
-    return if user_is_admin? || no_submitter_linked_to_page?
-
+  def verify_user_ownership
+    return if user_is_admin? || page_viewable_by_all?
 
-    deny_access unless current_submitter_is_creator?
+    deny_access unless current_submitter_is_owner?
   end
 
   private
 
-  def user_is_admin?
-    session[:admin]
-  end
-
-  def no_submitter_linked_to_page?
-    pages_not_requiring_submitter_ownership.include?(controller_name)
+  def page_viewable_by_all?
+    PAGES_VIEWABLE_BY_ALL.include?(controller_name)
   end
 
-  def handle_submitter_special_case
-    submitter = Submitter.find(params[:id])
-    unauthorized_access if session[:submitter_id] != submitter.id
-  end
+  def current_submitter_is_owner?
+    return submitter_owns_profile? if controller_name == 'submitters'
 
-  def resource_class
-    controller_name.classify.constantize
+    submitter_owns_publication?
   end
 
-  def current_submitter_is_creator?
+  def submitter_owns_profile?
+    submitter = Submitter.find(params[:id])
 
-    if controller_name == 'submitters'
-      handle_submitter_special_case
-      return
-    end
-
-    return unless resource_class.method_defined?(:submitter_id)
+    logged_in_submitter_id == submitter.id.to_s
+  end
 
-    resource = resource_class.find(params[:id])
+  def submitter_owns_publication?
+    publication = publication_class.find(params[:id])
+    publication && (logged_in_submitter_id == publication.submitter_id.to_s)
+  end
 
-    resource && (session[:submitter_id].to_s == resource.submitter_id.to_s)
+  def publication_class
+    controller_name.classify.constantize
   end
 
   def deny_access

app/controllers/publications_controller.rb

@@ -26,7 +26,7 @@
 #
 
 class PublicationsController < ApplicationController
-  include SubmitterOwnedContentGuard
+  include SubmitterOwnershipGuard
 
   before_action :set_object, only: %i[show edit update destroy]
   before_action :signed_in, only: %i[index show edit update destroy]

app/controllers/submitters_controller.rb

@@ -1,9 +1,11 @@
 # frozen_string_literal: true
 
 class SubmittersController < ApplicationController
+  include SubmitterOwnershipGuard
+
   skip_before_action :require_authenticated_user, only: %i[new create finished]
 
-  before_action :set_submitter, only: %i[show edit update destroy]
+  before_action :set_submitter, only: %i[show edit update]
 
   # GET /submitters/1
   # GET /submitters/1.json

spec/controllers/concerns/submitter_owned_content_guard_spec.rb → spec/controllers/concerns/submitter_ownership_guard/submitter_owned_publications_spec.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
-
 RSpec.describe BooksController, type: :controller do
   let(:submitter) { FactoryBot.create(:submitter) }
   let(:another_submitter) { FactoryBot.create(:submitter) }

spec/controllers/concerns/submitter_ownership_guard/submitter_profile_ownership_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe SubmittersController, type: :controller do
+  let(:submitter) { FactoryBot.create(:submitter) }
+  let(:another_submitter) { FactoryBot.create(:submitter) }
+
+  before do
+    submitter
+    another_submitter
+  end
+
+  # There are no Submitters index or destroy actions, so we cannot test them.
+  # The SubmittersController is tested in spec/controllers/submitters_controller_spec.rb.
+
+  describe '#submitter_owned_content_guard' do
+    context 'when admin is logged in' do
+      before { login_as_admin }
+
+      it 'allows access to show' do
+        get :show, params: { id: submitter.id }
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'allows access to edit' do
+        get :edit, params: { id: submitter.id }
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context 'when submitter owns the resource' do
+      before { login_as_submitter_of(submitter) }
+
+      it 'allows access to show' do
+        get :show, params: { id: submitter.id }
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'allows access to edit' do
+        get :edit, params: { id: submitter.id }
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context 'when another submitter is logged in' do
+      before { login_as_submitter_of(another_submitter) }
+
+      it 'restricts access to show' do
+        expect { get :show, params: { id: submitter.id } }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'restricts access to edit' do
+        expect { get :edit, params: { id: submitter.id } }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context 'when no user is logged in' do
+      # Not tested because this scenario is handled by the UserAuthentication concern.
+    end
+  end
+end

spec/support/helpers/login_helpers_for_unit_tests.rb

@@ -7,6 +7,18 @@ def login_as_submitter_of(resource)
   log_in_submitter(submitter_id)
 end
 
+def login_as_admin
+  session[:admin] = true
+end
+
+def user_is_admin?
+  session[:admin]
+end
+
+def logged_in_submitter_id
+  session[:submitter_id].to_s
+end
+
 private
 
 def extract_submitter_id_from(resource)