diff --git a/lib/generators/unread/migration/templates/migration.rb b/lib/generators/unread/migration/templates/migration.rb index ecdb1db..efe3413 100644 --- a/lib/generators/unread/migration/templates/migration.rb +++ b/lib/generators/unread/migration/templates/migration.rb @@ -6,7 +6,7 @@ def self.up t.datetime :timestamp end - add_index ReadMark, [:reader_id, :reader_type, :readable_type, :readable_id], name: 'read_marks_reader_readable_index' + add_index ReadMark, [:reader_id, :reader_type, :readable_type, :readable_id], name: 'read_marks_reader_readable_index', unique: true end def self.down diff --git a/lib/unread/readable.rb b/lib/unread/readable.rb index 66f651f..4a7ab64 100644 --- a/lib/unread/readable.rb +++ b/lib/unread/readable.rb @@ -27,11 +27,18 @@ def mark_array_as_read(array, reader) if global_timestamp && global_timestamp >= timestamp # The object is implicitly marked as read, so there is nothing to do else - 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! + # 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 end end end diff --git a/spec/unread/readable_spec.rb b/spec/unread/readable_spec.rb index a97416a..4fbc348 100644 --- a/spec/unread/readable_spec.rb +++ b/spec/unread/readable_spec.rb @@ -266,6 +266,20 @@ expect(@email2.unread?(@reader)).to be_falsey end + 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([]) + + expect do + Email.mark_as_read! [ @email1, @email2 ], for: @reader + end.to change(ReadMark, :count).by(1) + + expect(@email1.unread?(@reader)).to be_falsey + expect(@email2.unread?(@reader)).to be_falsey + end + it "should perform less queries if the objects are already read" do Email.mark_as_read! :all, :for => @reader