Merge pull request #2710 from sandbergja/backport-bookmark-create-ret…

…urn-value Backport BookmarksController#create should not return success on invalid input
projectblacklight · May 7, 2022 · 2197870 · 2197870
2 parents 26edc70 + f7d6964
commit 2197870
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 4 deletions.
diff --git a/app/controllers/concerns/blacklight/bookmarks.rb b/app/controllers/concerns/blacklight/bookmarks.rb
@@ -75,12 +75,15 @@ def create
 
     current_or_guest_user.save! unless current_or_guest_user.persisted?
 
-    success = @bookmarks.all? do |bookmark|
-      current_or_guest_user.bookmarks.where(bookmark).exists? || current_or_guest_user.bookmarks.create(bookmark)
+    bookmarks_to_add = @bookmarks.reject { |bookmark| current_or_guest_user.bookmarks.where(bookmark).exists? }
+    success = ActiveRecord::Base.transaction do
+      current_or_guest_user.bookmarks.create!(bookmarks_to_add)
+    rescue ActiveRecord::RecordInvalid
+      false
     end
 
     if request.xhr?
-      success ? render(json: { bookmarks: { count: current_or_guest_user.bookmarks.count } }) : render(plain: "", status: "500")
+      success ? render(json: { bookmarks: { count: current_or_guest_user.bookmarks.count } }) : render(json: current_or_guest_user.errors.full_messages, status: "500")
     else
       if @bookmarks.any? && success
         flash[:notice] = I18n.t('blacklight.bookmarks.add.success', count: @bookmarks.length)

diff --git a/spec/controllers/bookmarks_controller_spec.rb b/spec/controllers/bookmarks_controller_spec.rb
@@ -24,7 +24,8 @@
       allow(@controller).to receive_message_chain(:current_or_guest_user, :existing_bookmark_for).and_return(false)
       allow(@controller).to receive_message_chain(:current_or_guest_user, :persisted?).and_return(true)
       allow(@controller).to receive_message_chain(:current_or_guest_user, :bookmarks, :where, :exists?).and_return(false)
-      allow(@controller).to receive_message_chain(:current_or_guest_user, :bookmarks, :create).and_return(false)
+      allow(@controller).to receive_message_chain(:current_or_guest_user, :bookmarks, :create!).and_raise(ActiveRecord::RecordInvalid)
+      allow(@controller).to receive_message_chain(:current_or_guest_user, :errors, :full_messages).and_return([1])
       put :update, xhr: true, params: { id: 'iamabooboo', format: :js }
       expect(response.code).to eq "500"
     end