Merge pull request #5004 from avalonmediasystem/develop

More bugfixes

app/assets/javascripts/move_section.js

@@ -14,7 +14,7 @@
  * ---  END LICENSE_HEADER BLOCK  ---
 */
 
-$('#show_move_modal').on('click', function(){
+$('.show_move_modal').on('click', function(){
   $('#move_modal').show();
   var id = $(this).data('id');
   // Set the URL for form POST action

app/controllers/application_controller.rb

@@ -58,7 +58,9 @@ def rewrite_v4_ids
   end
 
   def store_location
-    store_location_for(:user, request.url) unless request.xhr?
+    if should_store_return_url?
+      store_location_for(:user, request.url)
+    end
     if request.env["omniauth.params"].present? && request.env["omniauth.params"]["login_popup"].present?
       session[:previous_url] = root_path + "self_closing.html"
     end
@@ -160,7 +162,7 @@ def current_ability
     if request.format == :json
       head :unauthorized
     else
-      session[:previous_url] = request.fullpath unless request.xhr?
+      store_location_for(:user, request.fullpath) if should_store_return_url?
       render '/errors/restricted_pid', status: :unauthorized
     end
   end
@@ -199,7 +201,7 @@ def authenticate_user!(_opts = {})
     if request.format == :json
       head :unauthorized
     else
-      session[:previous_url] = request.fullpath unless request.xhr?
+      store_location_for(:user, request.fullpath) if should_store_return_url?
       render '/errors/restricted_pid', status: :unauthorized
     end
   end
@@ -239,4 +241,8 @@ def strip_zero_width_chars!(obj)
         obj
       end
     end
+
+    def should_store_return_url?
+      !(request.xhr? || request.format != "html" || request.path.start_with?("/users/") || request.path.end_with?("poster") || request.path.end_with?("thumbnail"))
+    end
 end

app/views/media_objects/_file_upload.html.erb

@@ -73,7 +73,7 @@ Unless required by applicable law or agreed to in writing, software distributed
                           method: :delete %>
                     </span>
                     <span>
-                      <button id="show_move_modal" class="btn btn-sm btn-outline" data-id="<%= section.id %>"
+                      <button class="btn btn-sm btn-outline show_move_modal" data-id="<%= section.id %>"
                         data-toggle="modal" data-target="#move_modal">
                         Move
                       </button>

spec/controllers/media_objects_controller_spec.rb

@@ -1086,7 +1086,7 @@
       context 'Before sign in' do
         it 'persists the current url on the session' do
           get 'show', params: { id: media_object.id }
-          expect(session[:previous_url]).to eql media_object_path(media_object)
+          expect(session[:user_return_to]).to eql media_object_path(media_object)
         end
       end
 

spec/factories/users.rb

@@ -44,6 +44,12 @@
     end
     factory :user_lti do
     end
+
+    trait :with_identity do
+      after(:create) do |user|
+        Identity.create!(email: user.email, password: user.password)
+      end
+    end
   end
 
   factory :cataloger, class: User  do

spec/features/login_redirect_spec.rb

@@ -0,0 +1,52 @@
+# Copyright 2011-2022, The Trustees of Indiana University and Northwestern
+#   University.  Licensed under the Apache License, Version 2.0 (the "License");
+#   you may not use this file except in compliance with the License.
+#
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software distributed
+#   under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+#   CONDITIONS OF ANY KIND, either express or implied. See the License for the
+#   specific language governing permissions and limitations under the License.
+# ---  END LICENSE_HEADER BLOCK  ---
+
+require 'rails_helper'
+
+describe 'Login Redirects' do
+  let(:user) { FactoryBot.create(:user, :with_identity) }
+
+  describe '/media_objects/:id' do
+    let(:media_object) { FactoryBot.create(:fully_searchable_media_object, master_files: [master_file]) }
+    let(:master_file) { FactoryBot.create(:master_file, :with_derivative) }
+
+    it 'redirects to item page' do
+      visit media_object_path(media_object)
+      visit hls_manifest_master_file_path(media_object.master_files.first, "high")
+      sign_in user
+      expect(page.current_path).to eq media_object_path(media_object)
+    end
+
+    context 'visiting item after accessing restricted page' do
+      it 'redirects to item page' do
+        visit playlists_path
+        visit media_object_path(media_object)
+        sign_in user
+        expect(page.current_path).to eq media_object_path(media_object)
+      end
+    end
+  end
+
+  describe '/collection/:id' do
+    let(:collection) { FactoryBot.create(:collection) }
+    let(:media_object) { FactoryBot.create(:fully_searchable_media_object, collection: collection) }
+
+    it 'redirects to collection page' do
+      visit collections_path(collection, format: :html)
+      visit poster_collection_path(collection)
+      sign_in user
+      expect(page.current_path).to eq collections_path(collection, format: :html)
+    end
+  end
+end

spec/rails_helper.rb

@@ -191,6 +191,7 @@
   config.include Warden::Test::Helpers,type: :feature
   config.include FixtureMacros, type: :controller
   config.include OptionalExample
+  config.include Features::SessionHelpers, type: :feature
 end
 
 FactoryBot::SyntaxRunner.class_eval do

spec/requests/redirect_spec.rb

@@ -1,11 +1,11 @@
 # Copyright 2011-2022, The Trustees of Indiana University and Northwestern
 #   University.  Licensed under the Apache License, Version 2.0 (the "License");
 #   you may not use this file except in compliance with the License.
-# 
+#
 # You may obtain a copy of the License at
-# 
+#
 # http://www.apache.org/licenses/LICENSE-2.0
-# 
+#
 # Unless required by applicable law or agreed to in writing, software distributed
 #   under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
 #   CONDITIONS OF ANY KIND, either express or implied. See the License for the
@@ -17,14 +17,12 @@
 describe 'redirect', type: :request do
   it 'stores url to redirect to when unauthorized and needing to authenticate (#authorize!)' do
     get '/admin/collections'
-    expect(request.env['rack.session']['previous_url']).to eq '/admin/collections'
     expect(request.env['rack.session']['user_return_to']).to eq '/admin/collections'
     expect(response).to render_template('errors/restricted_pid')
   end
 
   it 'stores url to redirect to when needing to authenticate (#authenticate_user!)' do
     get '/bookmarks'
-    expect(request.env['rack.session']['previous_url']).to eq '/bookmarks'
     expect(request.env['rack.session']['user_return_to']).to eq '/bookmarks'
     expect(response).to render_template('errors/restricted_pid')
   end

spec/support/features.rb

@@ -0,0 +1,4 @@
+require File.expand_path('../features/session_helpers', __FILE__)
+
+RSpec.configure do |config|
+end

spec/support/features/session_helpers.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+# Copied from Hyrax: spec/support/features/session_helpers.rb
+module Features
+  module SessionHelpers
+    def sign_in(who = :user)
+      user = who.is_a?(User) ? who : FactoryBot.build(:user).tap(&:save!)
+      visit '/users/sign_in'
+      within('div.omniauth-form form') do
+        fill_in 'Login', with: user.email
+        fill_in 'Password', with: user.password
+        click_on 'Connect'
+      end
+    end
+  end
+end