diff --git a/lib/unread/read_mark.rb b/lib/unread/read_mark.rb index 4b66f3c..da3fbfe 100644 --- a/lib/unread/read_mark.rb +++ b/lib/unread/read_mark.rb @@ -1,5 +1,5 @@ class ReadMark < ActiveRecord::Base - belongs_to :readable, polymorphic: true + belongs_to :readable, polymorphic: true, inverse_of: :read_marks validates_presence_of :reader_id, :reader_type, :readable_type diff --git a/lib/unread/readable.rb b/lib/unread/readable.rb index 3e58b1d..8a66907 100644 --- a/lib/unread/readable.rb +++ b/lib/unread/readable.rb @@ -27,23 +27,40 @@ def mark_collection_as_read(collection, reader) if global_timestamp && global_timestamp >= timestamp # The object is implicitly marked as read, so there is nothing to do else - # This transaction is needed, so that parent transaction won't rollback even there's an error. - ReadMark.transaction(requires_new: true) do - begin - rm = obj.read_marks.where(reader_id: reader.id, reader_type: reader.class.base_class.name).first || obj.read_marks.build - rm.reader_id = reader.id - rm.reader_type = reader.class.base_class.name - rm.timestamp = timestamp - rm.save! - rescue ActiveRecord::RecordNotUnique - raise ActiveRecord::Rollback - end - end + mark_collection_item_as_read(obj, reader, timestamp) end end end end + def mark_collection_item_as_read(obj, reader, timestamp) + marking_proc = proc { + rm = obj.read_marks.find_or_initialize_by(reader: reader) + rm.timestamp = timestamp + rm.save! + } + + if using_postgresql? + # With PostgreSQL, a transaction is unusable after a unique constraint vialoation. + # To avoid this, nested transactions are required. + # http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Exception+handling+and+rolling+back + ReadMark.transaction(requires_new: true) do + begin + marking_proc.call + rescue ActiveRecord::RecordNotUnique + # The object is explicitly marked as read, so rollback the inner transaction + raise ActiveRecord::Rollback + end + end + else + begin + marking_proc.call + rescue ActiveRecord::RecordNotUnique + # The object is explicitly marked as read, so there is nothing to do + end + end + end + # A scope with all items accessable for the given reader # It's used in cleanup_read_marks! to support a filtered cleanup # Should be overriden if a reader doesn't have access to all items diff --git a/spec/unread/readable_spec.rb b/spec/unread/readable_spec.rb index 01cbf2b..b742b74 100644 --- a/spec/unread/readable_spec.rb +++ b/spec/unread/readable_spec.rb @@ -269,8 +269,8 @@ it "should mark the rest as read when the first record is not unique" do Email.mark_as_read! [ @email1 ], for: @reader - allow(@email1).to receive_message_chain("read_marks.build").and_return(@email1.read_marks.build) - allow(@email1).to receive_message_chain("read_marks.where").and_return([]) + allow(@email1).to receive_message_chain("read_marks.find_or_initialize_by") + .and_return(@email1.read_marks.build(reader: @reader)) expect do Email.mark_as_read! [ @email1, @email2 ], for: @reader @@ -295,7 +295,6 @@ expect(@email2.unread?(@reader)).to be_falsey end - it "should mark all objects as read" do Email.mark_as_read! :all, for: @reader @@ -330,6 +329,11 @@ Email.mark_as_read! :foo, :bar }.to raise_error(ArgumentError) end + + it "should work with STI readers" do + Email.mark_as_read! [ @email1 ], for: Customer.find(@sti_reader.id) + expect(@email1.unread?(@sti_reader)).to be_falsey + end end describe :cleanup_read_marks! do