Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

in-persistence attribute of unread boolean #19

Closed
Ian-Choi opened this issue Mar 27, 2013 · 3 comments
Closed

in-persistence attribute of unread boolean #19

Ian-Choi opened this issue Mar 27, 2013 · 3 comments

Comments

@Ian-Choi
Copy link

Was looking at this method but behaved not as expected:
messages = Message.with_read_marks_for(current_user)

I was thinking if a method called with_unread_attr_for(reader) that would return all a list of readables with boolean attribute value indicating if it is unread by the reader. This might save us making additional calls to database to get the unread status of a list of readables.

Or am I missing something?

@ledermann
Copy link
Owner

with_read_marks_foris exactly for this use case: Getting a list of items including their read status with a minimal amount of database queries. Example:

In the controller:

@messages = Message.with_read_marks_for(current_user)

In the view:

- @messages.each do |message|
  - if message.unread?(current_user)
    = content_tag :strong, message.subject
  - else
    = message.subject

Hope this helps.

@Ian-Choi
Copy link
Author

I don't think it behave right. I am using PostgreSql.

irb(main):004:0> Ticket.with_read_marks_for(u).first.unread?(u)
Ticket Load (1.0ms) SELECT tickets., read_marks.id AS read_mark_id FROM "tickets" LEFT JOIN read_marks ON read_marks.readable_type = 'Ticket'
AND read_marks.readable_id = tickets.id
AND read_marks.user_id = 1
AND read_marks.timestamp >= tickets.updated_at WHERE "tickets"."is_destroyed" = 'f' ORDER BY "tickets"."id" ASC LIMIT 1
=> false
irb(main):005:0> Ticket.with_read_marks_for(u).last.unread?(u)
Ticket Load (1.1ms) SELECT tickets.
, read_marks.id AS read_mark_id FROM "tickets" LEFT JOIN read_marks ON read_marks.readable_type = 'Ticket'
AND read_marks.readable_id = tickets.id
AND read_marks.user_id = 1
AND read_marks.timestamp >= tickets.updated_at WHERE "tickets"."is_destroyed" = 'f' ORDER BY "tickets"."id" DESC LIMIT 1
ReadMark Load (1.4ms) SELECT "read_marks".* FROM "read_marks" WHERE "read_marks"."user_id" = $1 AND "read_marks"."readable_type" = 'Ticket' AND "read_marks"."readable_id" IS NULL ORDER BY "read_marks"."id" ASC LIMIT 1 [["user_id", 1]]
=> true

In the case it is ture, it queries the database twice. One for the list of readables and one for the read_marks.

I have a pull request. Please ignore it if I am wrong. Thanks.

@ledermann
Copy link
Owner

Yes, the database has to be touched twice in some cases. Please have a look at the code:
https://github.com/ledermann/unread/blob/master/lib/unread/readable.rb#L102
It's required to get the global timestamp if the message has no read marker.

Here is another example. You will see that the executed queries are optimized:

n = 10
Message.with_read_marks_for(user).last(n).map { |message| message.unread?(user) }

For this, only 2 queries are needed, not nor 2*nqueries.

Your PR #20 looks interesting, though. Maybe the 2 queries can be combined into a single one. I will check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants