unread icon indicating copy to clipboard operation
unread copied to clipboard

Utilizing current time instead of matching reference timestamp field

Open etcook opened this issue 6 years ago • 6 comments

What's the argument for utilizing matching the timestamp of the read_mark with the reference timestamp field? Why not just set the current time?

I was in the middle of building a derivative read report by left joining the data, and was surprised to see the timestamp field matched every single created_at. My intention was to build an "aging" report that showed how long it took to read an object.

Based on your response, if you'd like, I'll submit a PR to set it to current time instead.

etcook avatar Apr 17 '18 05:04 etcook

Thanks for your feedback, but I don't see what's wrong with the timestamps. The single timestamp (read_marks.readable_type IS NOT NULL) is the time when the readable was read by the user. The global timestamp (read_marks.readable_type IS NULL) is the time where the latest mark_as_read! :all, for: @reader was made.

What kind of improvement do you see?

ledermann avatar Apr 17 '18 08:04 ledermann

That's the behavior I expected, but I don't think that's what it's doing. All of my read_marks have the exact same time as created_at (which I'm using as a reference).

https://github.com/ledermann/unread/blob/7513518ed297f80a30405bb38e2d9cbf963aa8bc/lib/unread/readable.rb#L137

etcook avatar Apr 17 '18 08:04 etcook

You are right, not the reading time is stored, it's the reference time of the readable. Sorry - it's five year old code :)

Using the current time feels better for me. But beware, the GarbageCollector has to be changed/rewritten for this. This line needs the current implementation: https://github.com/ledermann/unread/blob/v0.10.1/lib/unread/garbage_collector.rb#L44

Nevertheless, I'm open for a PR.

ledermann avatar Apr 17 '18 10:04 ledermann

Two notes about your "aging report":

  • In the database there is no difference between "the user as read the individual readable" and "the user has not read it, but has marked multiple items as read in one step".
  • After the garbage collection, old read_marks are deleted and therefore the time of reading is lost.

ledermann avatar Apr 17 '18 10:04 ledermann

@ledermann Thank you for the quick response (and the gem). Luckily in this circumstance, I'm neither calling garbage collection nor letting them read multiple items / mark all read. I'll take a look at the gc code and submit a pr in a few. Thanks!

etcook avatar Apr 18 '18 16:04 etcook

Hi! I assume that the PR never existed, right? Any tips on what to change in the gem to get the read time instead the created_at timestamp?

deikka avatar Nov 11 '19 12:11 deikka