ledermann / unread

Handle unread records and mark them as read with Ruby on Rails
MIT License
739 stars 121 forks source link

Add read_at #90

Closed aried3r closed 7 years ago

aried3r commented 7 years ago

This PR implements read_at so users know when a record was set to read by another.

Maybe this is something you want in this gem, maybe not. So please see this PR as a proposal :)

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-9.0%) to 91.031% when pulling e6abcd1a23d91c1c6f52e76b1bc06b91dd0fc605 on aried3r:ar/read_at into 3e54a30bfedf19a9a535b88767739a244a3670a3 on ledermann:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 99.552% when pulling 25083b0c80af47a1e3ac97d1f2359cecb613c1ae on aried3r:ar/read_at into 3e54a30bfedf19a9a535b88767739a244a3670a3 on ledermann:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 6e817e81084cad839847c26db24c97b6e66c64e0 on aried3r:ar/read_at into 3e54a30bfedf19a9a535b88767739a244a3670a3 on ledermann:master.

bgrzys commented 7 years ago

@aried3r Thanks for this PR, this is what I was looking for. I wonder why do we need new column instead of using existing timestamp field for this? At the first glance it looks to me that we can just use current time in mark_as_read!?

ledermann commented 7 years ago

Thanks for your contribution, but there is an important reason not to merge this PR: .cleanup_read_marks! deletes old (and unnecessary) read_marks, so the #read_at information gets lost. With this background, there is no way to store the date/time of reading in the read_marks table.