Use id-based bookmark path instead of access id / noid

refs #567

app/helpers/application_helper.rb

@@ -61,6 +61,12 @@ def document_thumbnail(document, image_options = {})
     image_tag url, image_options if url.present?
   end
 
+  # Gives the bookmarks path by document id instead of access id
+  # Use as an alternative to bookmark_path
+  def bookmarks_id_path(document)
+    Pathname.new('/').join("bookmarks", document.id).to_s
+  end
+
   private
 
     # Generate the URL for the configuration for the UV

app/views/catalog/_bookmark_control.html.erb

@@ -0,0 +1,27 @@
+<% if current_or_guest_user %>
+  <%-
+  # Note these two forms are pretty similar but for different :methods, classes, and labels.
+  # but it was simpler to leave them seperate instead of DRYing them, got confusing trying that.
+  # the data-doc-id attribute is used by our JS that converts to a checkbox/label.
+  #
+  # Local override to use the actual document id for the form path as opposed to
+  # the noid id. This simple change makes all the existing bookmark
+  # functionality work rather than overriding functionality in several other
+  # places to stay consistent with the noid.
+  -%>
+  <% unless bookmarked? document %>
+
+      <%= form_tag( bookmarks_id_path(document), :method => :put, :class => "bookmark_toggle", "data-doc-id" => document.id, :'data-present' => t('blacklight.search.bookmarks.present'), :'data-absent' => t('blacklight.search.bookmarks.absent'), :'data-inprogress' => t('blacklight.search.bookmarks.inprogress')) do %>
+        <%= submit_tag(t('blacklight.bookmarks.add.button'), :id => "bookmark_toggle_#{document.id.to_s.parameterize}", :class => "bookmark_add btn btn-default") %>
+      <% end %>
+
+  <% else %>
+
+      <%= form_tag( bookmarks_id_path(document), :method => :delete, :class => "bookmark_toggle", "data-doc-id" => document.id, :'data-present' => t('blacklight.search.bookmarks.present'), :'data-absent' => t('blacklight.search.bookmarks.absent'), :'data-inprogress' => t('blacklight.search.bookmarks.inprogress')) do %>
+        <%= submit_tag(t('blacklight.bookmarks.remove.button'), :id => "bookmark_toggle_#{document.id.to_s.parameterize}", :class => "bookmark_remove btn btn-default") %>
+      <% end %>
+
+  <% end %>
+<% else %>
+  &nbsp;
+<% end %>

spec/features/bookmarks_spec.rb

@@ -0,0 +1,45 @@
+require 'rails_helper'
+
+RSpec.describe 'Bookmarks', type: :feature, js: true do
+  let(:exhibit) { FactoryBot.create(:exhibit, title: 'Exhibit Title', slug: 'exhibit-title') }
+  let(:admin) { FactoryBot.create(:exhibit_admin, exhibit: exhibit) }
+  let(:id) { "67890" }
+  let(:id2) { "12345" }
+  let(:access_id) { '1r66j4408' }
+  let(:access_id2) { '2r66j4408' }
+  let(:document) { SolrDocument.new(id: id) }
+  let(:document2) { SolrDocument.new(id: id2) }
+
+  before do
+    sign_in admin
+    Spotlight::SolrDocumentSidecar.create!(
+      document: document2, exhibit: exhibit,
+      data: { "access_identifier_ssim" => [access_id2] }
+    )
+    Spotlight::SolrDocumentSidecar.create!(
+      document: document, exhibit: exhibit,
+      data: { "access_identifier_ssim" => [access_id] }
+    )
+
+    document.make_public! exhibit
+    document2.make_public! exhibit
+    document.reindex
+    document2.reindex
+    Blacklight.default_index.connection.commit
+  end
+
+  context "when checking the bookmark box" do
+    it "Adds bookmarks to the bookmark page" do
+      visit "/catalog?search_field=all_fields&q="
+      bookmark_box = "toggle_bookmark_#{id}"
+      check bookmark_box
+      expect(page).to have_content "In Bookmarks"
+
+      visit "/bookmarks"
+      expect(page).to have_content "1 entry found"
+
+      visit "/catalog?search_field=all_fields&q="
+      expect(page).to have_content "In Bookmarks"
+    end
+  end
+end