Merge branch 'main' into reindex-fileset-after-file-migration

app/assets/stylesheets/_bootstrap-default-overrides.scss

@@ -19,6 +19,9 @@ $warning: #565653 !default;
 $breadcrumb-active-color: #4c4c4c;
 $pagination-disabled-color: #4c4c4c;
 $blue: #0056B3 !default;
+// Adjusts link text to show as underlined and then not underlined on hover
+$link-decoration: underline !default;
+$link-hover-decoration: none !default;
 
 // Date picker on safari placeholder text was misaligned
 @media screen {

app/services/hyrax/lock_manager.rb

@@ -16,11 +16,11 @@ def initialize(time_to_live, retry_count, retry_delay)
 
     ##
     # Blocks until lock is acquired or timeout.
-    def lock(key)
+    def lock(key, ttl: @ttl, retry_count: @retry_count, retry_delay: @retry_delay)
       returned_from_block = nil
 
       pool.then do |conn|
-        client(conn).lock(key, @ttl) do |locked|
+        client(conn, retry_count: retry_count, retry_delay: retry_delay).lock(key, ttl) do |locked|
           raise UnableToAcquireLockError unless locked
           returned_from_block = yield
         end
@@ -31,16 +31,16 @@ def lock(key)
       Hyrax.logger.error(err.message)
       raise(ConnectionPool::TimeoutError,
             "Failed to acquire a lock from Redlock due to a Redis connection " \
-            "timeout: #{err}. If you are using Redis via `ConnectionPool` " \
-            "you may wish to increase the pool size.")
+              "timeout: #{err}. If you are using Redis via `ConnectionPool` " \
+              "you may wish to increase the pool size.")
     end
 
     private
 
     ##
     # @api_private
-    def client(conn)
-      Redlock::Client.new([conn], retry_count: @retry_count, retry_delay: @retry_delay)
+    def client(conn, retry_count:, retry_delay:)
+      Redlock::Client.new([conn], retry_count: retry_count, retry_delay: retry_delay)
     end
 
     ##

app/services/hyrax/lockable.rb

@@ -1,11 +1,12 @@
 # frozen_string_literal: true
+require 'forwardable'
+
 module Hyrax
   module Lockable
+    extend Forwardable
     extend ActiveSupport::Concern
 
-    def acquire_lock_for(lock_key, &block)
-      lock_manager.lock(lock_key, &block)
-    end
+    def_delegator :lock_manager, :lock, :acquire_lock_for
 
     def lock_manager
       @lock_manager ||= LockManager.new(

app/views/hyrax/pages/_form.html.erb

@@ -2,22 +2,22 @@
 <div class="card tabs">
 
   <ul class="nav nav-tabs" role="tablist">
-    <li class="nav-item">
+    <li class="nav-item" role="presentation">
       <a href="#about" role="tab" data-toggle="tab" class="nav-link active nav-safety-confirm">
         <%= t(:'hyrax.pages.tabs.about_page') %>
       </a>
     </li>
-    <li class="nav-item">
+    <li class="nav-item" role="presentation">
       <a href="#help" role="tab" data-toggle="tab" class="nav-link nav-safety-confirm">
         <%= t(:'hyrax.pages.tabs.help_page') %>
       </a>
     </li>
-    <li class="nav-item">
+    <li class="nav-item" role="presentation">
       <a href="#agreement" role="tab" data-toggle="tab" class="nav-link nav-safety-confirm">
         <%= t(:'hyrax.pages.tabs.agreement_page') %>
       </a>
     </li>
-    <li class="nav-item">
+    <li class="nav-item" role="presentation">
       <a href="#terms" role="tab" data-toggle="tab" class="nav-link nav-safety-confirm">
         <%= t(:'hyrax.pages.tabs.terms_page') %>
       </a>

app/views/hyrax/users/_user_row.html.erb

@@ -1,8 +1,11 @@
 <tr>
-  <td><%= link_to hyrax.user_path(user) do %>
-        <%= image_tag(user.avatar.url(:thumb), width: 30) if user.avatar.file %>
+   <td>
+     <% if user.avatar.file %>
+      <%= link_to hyrax.user_path(user) do %>
+        <%= image_tag(user.avatar.url(:thumb), width: 30, alt: user.name) %>
       <% end %>
-  </td>
+     <% end %>
+   </td>
    <td><%= link_to user.name, hyrax.user_path(user) %></td>
    <td><%= link_to user.user_key, hyrax.user_path(user) %></td>
    <td><%= user.department %></td>

spec/services/hyrax/lock_manager_spec.rb

@@ -1,14 +1,93 @@
 # frozen_string_literal: true
 RSpec.describe Hyrax::LockManager do
+  let(:ttl) { Hyrax.config.lock_time_to_live }
+  let(:retry_count) { Hyrax.config.lock_retry_count }
+  let(:retry_delay) { Hyrax.config.lock_retry_delay }
+
   subject do
-    described_class.new(Hyrax.config.lock_time_to_live,
-                        Hyrax.config.lock_retry_count,
-                        Hyrax.config.lock_retry_delay)
+    described_class.new(ttl, retry_count, retry_delay)
   end
 
   describe "lock", unless: ENV['TRAVIS'] do
     it "calls the block" do
       expect { |probe| subject.lock('foobar', &probe) }.to yield_with_no_args
     end
+
+    describe 'ttl' do
+      it "uses the configured ttl" do
+        client = instance_double(Redlock::Client)
+        allow(Redlock::Client).to receive(:new).and_return(client)
+
+        key = 'the key'
+        expect(client).to receive(:lock).with(key, ttl).and_yield(true)
+        subject.lock(key) { |_| }
+      end
+
+      it "accepts an optional ttl argument" do
+        new_ttl = 1000
+        expect(new_ttl).not_to eq(ttl) # just to be sure
+
+        client = instance_double(Redlock::Client)
+        allow(Redlock::Client).to receive(:new).and_return(client)
+
+        key = 'the key'
+        expect(client).to receive(:lock).with(key, new_ttl).and_yield(true)
+        subject.lock(key, ttl: new_ttl) { |_| }
+      end
+    end
+
+    describe 'retry_count' do
+      it "uses the configured retry_count" do
+        client = instance_double(Redlock::Client)
+        expect(Redlock::Client)
+          .to receive(:new)
+          .with(kind_of(Array), retry_count: retry_count, retry_delay: kind_of(Integer))
+          .and_return(client)
+
+        allow(client).to receive(:lock).and_yield(true)
+        subject.lock("a key") { |_| }
+      end
+
+      it "accepts an optional retry_count argument" do
+        new_retry_count = 11
+        expect(new_retry_count).not_to eq(retry_count) # just to be sure
+
+        client = instance_double(Redlock::Client)
+        expect(Redlock::Client)
+          .to receive(:new)
+          .with(kind_of(Array), retry_count: new_retry_count, retry_delay: retry_delay)
+          .and_return(client)
+
+        allow(client).to receive(:lock).and_yield(true)
+        subject.lock("a key", retry_count: new_retry_count) { |_| }
+      end
+    end
+
+    describe 'retry_delay' do
+      it "uses the configured retry_delay" do
+        client = instance_double(Redlock::Client)
+        expect(Redlock::Client)
+          .to receive(:new)
+          .with(kind_of(Array), retry_count: kind_of(Integer), retry_delay: retry_delay)
+          .and_return(client)
+
+        allow(client).to receive(:lock).and_yield(true)
+        subject.lock("a key") { |_| }
+      end
+
+      it "accepts an optional retry_delay argument" do
+        new_retry_delay = 11
+        expect(new_retry_delay).not_to eq(retry_delay) # just to be sure
+
+        client = instance_double(Redlock::Client)
+        expect(Redlock::Client)
+          .to receive(:new)
+          .with(kind_of(Array), retry_count: retry_count, retry_delay: new_retry_delay)
+          .and_return(client)
+
+        allow(client).to receive(:lock).and_yield(true)
+        subject.lock("a key", retry_delay: new_retry_delay) { |_| }
+      end
+    end
   end
 end

spec/services/hyrax/lockable_spec.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+RSpec.describe Hyrax::Lockable do
+  let(:lockable_class) { Class.new.include(described_class) }
+  let(:lock_manager) { instance_double(Hyrax::LockManager) }
+
+  subject { lockable_class.new }
+
+  describe "lock_manager" do
+    it "lazily creates a new lock manager with the default configuration" do
+      expect(Hyrax::LockManager).to receive(:new).once.with(
+        Hyrax.config.lock_time_to_live,
+        Hyrax.config.lock_retry_count,
+        Hyrax.config.lock_retry_delay
+      ).and_return(lock_manager)
+
+      expect(subject.lock_manager).to be(lock_manager)
+      expect(subject.lock_manager).to be(lock_manager)
+    end
+  end
+
+  describe "acquire_lock_for" do
+    before do
+      subject.instance_variable_set(:@lock_manager, lock_manager)
+    end
+
+    it 'acquires the lock' do
+      key = 'a key'
+      block = proc {}
+      expect(lock_manager).to receive(:lock).with(key, &block)
+      subject.acquire_lock_for(key, &block)
+    end
+
+    it 'accepts an optional ttl argument' do
+      key = 'a key'
+      new_ttl = 1000
+      block = proc {}
+      expect(lock_manager).to receive(:lock).with(key, ttl: new_ttl, &block)
+      subject.acquire_lock_for(key, ttl: new_ttl, &block)
+    end
+
+    it 'accepts an optional retry_count argument' do
+      key = 'a key'
+      new_retry_count = 11
+      block = proc {}
+      expect(lock_manager).to receive(:lock).with(key, retry_count: new_retry_count, &block)
+      subject.acquire_lock_for(key, retry_count: new_retry_count, &block)
+    end
+
+    it 'accepts an optional retry_delay argument' do
+      key = 'a key'
+      new_retry_delay = 11
+      block = proc {}
+      expect(lock_manager).to receive(:lock).with(key, retry_delay: new_retry_delay, &block)
+      subject.acquire_lock_for(key, retry_delay: new_retry_delay, &block)
+    end
+  end
+end