Don't allow more than 1000 bookmarks

Co-authored-by: Kevin Reiss <kevinreiss@users.noreply.github.com>
Co-authored-by: Max Kadel <maxkadel@users.noreply.github.com>

app/assets/javascripts/blacklight/blacklight.js

@@ -0,0 +1,12 @@
+// Customize which blacklight javascripts are imported
+// See: https://github.com/projectblacklight/blacklight/wiki/Using-Sprockets-to-compile-javascript-assets
+
+
+//= require 'blacklight/core.js'
+
+//= import 'blacklight/autocomplete.js'
+//= require 'blacklight/bookmark_toggle.js'
+//= require 'blacklight/button_focus.js'
+//= require 'blacklight/facet_load.js'
+//= require 'blacklight/modal.js'
+//= require 'blacklight/search_context.js'

app/assets/javascripts/checkbox_submit.js

@@ -0,0 +1,144 @@
+// From blacklight gem
+// The upstream file has changed substantially in Blacklight 8,
+// When we upgrade, we will need to update this file to match
+// the new API.
+
+/* A JQuery plugin (should this be implemented as a widget instead? not sure)
+   that will convert a "toggle" form, with single submit button to add/remove
+   something, like used for Bookmarks, into an AJAXy checkbox instead.
+
+   Apply to a form. Does require certain assumption about the form:
+    1) The same form 'action' href must be used for both ADD and REMOVE
+       actions, with the different being the hidden input name="_method"
+       being set to "put" or "delete" -- that's the Rails method to pretend
+       to be doing a certain HTTP verb. So same URL, PUT to add, DELETE
+       to remove. This plugin assumes that.
+
+       Plus, the form this is applied to should provide a data-doc-id
+       attribute (HTML5-style doc-*) that contains the id/primary key
+       of the object in question -- used by plugin for a unique value for
+       DOM id's.
+
+  Uses HTML for a checkbox compatible with Bootstrap 3.
+
+   Pass in options for your class name and labels:
+   $("form.something").blCheckboxSubmit({
+        //cssClass is added to elements added, plus used for id base
+        cssClass: "toggle_my_kinda_form",
+        error: function() {
+          #optional callback
+        },
+        success: function(after_success_check_state) {
+          #optional callback
+        }
+   });
+*/
+(function($) {
+  $.fn.blCheckboxSubmit = function(argOpts) {
+    this.each(function() {
+      var options = $.extend({}, $.fn.blCheckboxSubmit.defaults, argOpts);
+
+
+      var form = $(this);
+      form.children().hide();
+      //We're going to use the existing form to actually send our add/removes
+      //This works conveneintly because the exact same action href is used
+      //for both bookmarks/$doc_id.  But let's take out the irrelevant parts
+      //of the form to avoid any future confusion.
+      form.find('input[type=submit]').remove();
+
+      //View needs to set data-doc-id so we know a unique value
+      //for making DOM id
+      var uniqueId = form.attr('data-doc-id') || Math.random();
+      // if form is currently using method delete to change state,
+      // then checkbox is currently checked
+      var checked = (form.find('input[name=_method][value=delete]').length != 0);
+
+      var checkbox = $('<input type="checkbox">')
+        .addClass( options.cssClass )
+        .attr('id', options.cssClass + '_' + uniqueId);
+      var label = $('<label>')
+        .addClass( options.cssClass )
+        .attr('for', options.cssClass + '_' + uniqueId)
+        .attr('title', form.attr('title') || '');
+      var span = $('<span>');
+
+      label.append(checkbox);
+      label.append(' ');
+      label.append(span);
+
+      var checkboxDiv = $('<div class="checkbox" />')
+        .addClass(options.cssClass)
+        .append(label);
+
+      function updateStateFor(state) {
+          checkbox.prop('checked', state);
+          label.toggleClass('checked', state);
+          if (state) {
+             //Set the Rails hidden field that fakes an HTTP verb
+             //properly for current state action.
+             form.find('input[name=_method]').val('delete');
+             span.html(form.attr('data-present'));
+          } else {
+             form.find('input[name=_method]').val('put');
+             span.html(form.attr('data-absent'));
+          }
+        }
+
+      form.append(checkboxDiv);
+      updateStateFor(checked);
+
+      checkbox.click(function() {
+          span.html(form.attr('data-inprogress'));
+          label.attr('disabled', 'disabled');
+          checkbox.attr('disabled', 'disabled');
+
+          $.ajax({
+              url: form.attr('action'),
+              dataType: 'json',
+              type: form.attr('method').toUpperCase(),
+              data: form.serialize(),
+              error: function(data) {
+                 label.removeAttr('disabled');
+                 checkbox.removeAttr('disabled');
+                 options.error.call(form, data.responseJSON);
+                 updateStateFor(checked);
+                },
+              success: function(data, status, xhr) {
+                //if app isn't running at all, xhr annoyingly
+                //reports success with status 0.
+                if (xhr.status != 0) {
+                  checked = ! checked;
+                  updateStateFor(checked);
+                  label.removeAttr('disabled');
+                  checkbox.removeAttr('disabled');
+                  options.success.call(form, checked, xhr.responseJSON);
+                } else {
+                  label.removeAttr('disabled');
+                  checkbox.removeAttr('disabled');
+                  options.error.call(form, xhr.responseJSON);
+                }
+              }
+          });
+
+          return false;
+      }); //checkbox.click
+
+
+    }); //this.each
+    return this;
+  };
+
+$.fn.blCheckboxSubmit.defaults =  {
+          //cssClass is added to elements added, plus used for id base
+          cssClass: 'blCheckboxSubmit',
+          error: function(json) {
+            if (json && json.length && json[0]) {
+              alert(json[0]);
+            } else {
+              alert("Error");
+            }
+          },
+          success: function() {} //callback
+};
+})(jQuery);

app/controllers/bookmarks_controller.rb

@@ -36,6 +36,36 @@ def index
     end
   end
 
+  def create
+    @bookmarks = if params[:bookmarks]
+                   permit_bookmarks[:bookmarks]
+                 else
+                   [{ document_id: params[:id], document_type: blacklight_config.document_model.to_s }]
+                 end
+
+    current_or_guest_user.save! unless current_or_guest_user.persisted?
+
+    bookmarks_to_add = @bookmarks.reject { |bookmark| current_or_guest_user.bookmarks.where(bookmark).exists? }
+    errors = []
+    ActiveRecord::Base.transaction do
+      bookmarks_to_add.each do |bookmark|
+        errors.concat current_or_guest_user.bookmarks.create(bookmark)&.errors&.full_messages
+      end
+    end
+
+    if request.xhr?
+      errors.empty? ? render(json: { bookmarks: { count: current_or_guest_user.bookmarks.count } }) : render(json: errors, status: "500")
+    else
+      if @bookmarks.any? && errors.empty?
+        flash[:notice] = I18n.t('blacklight.bookmarks.add.success', count: @bookmarks.length)
+      elsif @bookmarks.any?
+        flash[:error] = I18n.t('blacklight.bookmarks.add.failure', count: @bookmarks.length)
+      end
+
+      redirect_back fallback_location: bookmarks_path
+    end
+  end
+
   def print
     fetch_bookmarked_documents
     @url_gen_params = {}

app/models/bookmark.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class Bookmark < ApplicationRecord
+  belongs_to :user, polymorphic: true
+  belongs_to :document, polymorphic: true
+  validate :not_too_many_bookmarks, on: :create
+
+  def document
+    document_type.new document_type.unique_key => document_id
+  end
+
+  def document_type
+    value = super if defined?(super)
+    value &&= value.constantize
+    value || default_document_type
+  end
+
+  def default_document_type
+    SolrDocument
+  end
+
+  def not_too_many_bookmarks
+    return if user.bookmarks.count < Orangelight.config['bookmarks']['maximum']
+
+    errors.add(:user, "You have exceeded the maximum number of bookmarks! You can only save up to #{Orangelight.config['bookmarks']['maximum']} bookmarks")
+  end
+end

config/initializers/assets.rb

@@ -4,3 +4,7 @@
 
 # Version of your assets, change this if you want to expire all your assets.
 Rails.application.config.assets.version = '1.0'
+
+bl_dir = Bundler.rubygems.find_name('blacklight').first.full_gem_path
+assets_path = File.join(bl_dir, 'app', 'javascript')
+Rails.application.config.assets.paths << assets_path

config/orangelight.yml

@@ -1,4 +1,6 @@
 defaults: &defaults
+  bookmarks:
+    maximum: 1000
   events:
     server: 'amqp://localhost:5672'
     exchange: 'orangelight_events'
@@ -15,6 +17,8 @@ development:
 
 test:
   <<: *defaults
+  bookmarks:
+    maximum: 50
   feedback_form:
     to: 'test@princeton.edu'
     cc: 'test2w@princeton.edu, test3@princeton.edu'

spec/controllers/bookmarks_controller_spec.rb

@@ -43,4 +43,32 @@
       expect(body).not_to include(bad_value)
     end
   end
+
+  describe "create" do
+    let(:user) { FactoryBot.create(:user_with_many_bookmarks, bookmarks: 3) }
+    let(:controller) { described_class.new }
+    before do
+      user.save
+      allow(controller).to receive(:current_or_guest_user).and_return(user)
+      allow(controller).to receive(:params).and_return(
+        ActionController::Parameters.new({
+                                           bookmarks: [{ document_id: "12345", document_type: "SolrDocument" }],
+                                           format: :js
+                                         })
+      )
+      allow(controller).to receive(:redirect_back)
+      allow(controller.request).to receive(:xhr?).and_return(true)
+      allow(controller).to receive(:render)
+    end
+    it 'creates a bookmark' do
+      expect { controller.create }.to change { Bookmark.count }.by(1)
+    end
+    context 'when user already has the maximum number of bookmarks' do
+      let(:user) { FactoryBot.create(:user_with_many_bookmarks, bookmarks: 50) }
+      it 'does not create the bookmark' do
+        expect(Orangelight.config['bookmarks']['maximum']).to eq(50)
+        expect { controller.create }.not_to change { Bookmark.count }
+      end
+    end
+  end
 end

spec/factories/users.rb

@@ -41,5 +41,19 @@
       sequence(:uid) { srand.to_s[2..15] }
       username { 'Alma Patron' }
     end
+
+    factory :user_with_many_bookmarks do
+      transient do
+        bookmarks { 50 }
+      end
+      after(:create) do |user, evaluator|
+        (1..evaluator.bookmarks).each do |document_id|
+          bookmark = Bookmark.new
+          bookmark.user_id = user.id
+          bookmark.document_id = document_id
+          user.bookmarks << bookmark
+        end
+      end
+    end
   end
 end

spec/models/bookmarks_spec.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+describe Bookmark do
+  it 'does not allow user to create more than the maximum configured bookmarks' do
+    user = FactoryBot.create(:user_with_many_bookmarks,
+                             bookmarks: Orangelight.config['bookmarks']['maximum'])
+    expect do
+      bookmark = described_class.new(user:, document_id: '123')
+      bookmark.save!
+    end.to raise_error(ActiveRecord::RecordInvalid)
+  end
+end

spec/system/bookmarks_spec.rb

@@ -55,5 +55,17 @@
 
       expect(page).to have_content "History."
     end
+
+    context 'when user has more than the maximum number of bookmarks' do
+      let(:user) { FactoryBot.create(:user_with_many_bookmarks, bookmarks: 51) }
+      before do
+        stub_holding_locations
+      end
+      it 'still allows them to view the bookmarks page' do
+        login_as user
+        visit "/bookmarks"
+        expect(page).to have_xpath '//h1[text() = "Bookmarks"]'
+      end
+    end
   end
 end