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

Private message notifications don't appear in the UI after 1 or 2 messages #2321

Closed
jjoe opened this Issue Nov 5, 2011 · 15 comments

Comments

Projects
None yet
7 participants
@jjoe

jjoe commented Nov 5, 2011

I don't notice some replies until I see them in my email.
I am referring to PM's private messages.

@Zoulou

This comment has been minimized.

Zoulou commented Nov 10, 2011

I can confirm this has been happening to me for about, oh, 2-3 months now.

@ghost

This comment has been minimized.

ghost commented Dec 21, 2011

i can confirm that...really annoying...

@sarahmei

This comment has been minimized.

Member

sarahmei commented Jan 9, 2012

Thanks for reporting this. I edited the title - it sounds like you're getting email notifications, but no notifications in the UI?

@jjoe

This comment has been minimized.

jjoe commented Jan 9, 2012

Yes. I think I only get notified (on d*) of the first entry on the thread. It's hard to confirm since I don't do much private messaging. I will send you a test message. We can respond to each other 2 or 3 times to see if we get notifications.

@Zoulou

This comment has been minimized.

Zoulou commented Jan 9, 2012

Thanks for looking into this, Sarah, and for responding to us. Feel free to test this bug out using my diasporg account as well.

@jaywink

This comment has been minimized.

Contributor

jaywink commented Sep 1, 2012

Yeah only the first post brings up a notification for me :P Replies don't show up

@prellele

This comment has been minimized.

Contributor

prellele commented Oct 11, 2012

can confirm

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 6, 2012

Please assign to 0.0.2 milestone if possible - I think this fix is pretty urgent and am willing to work on fixing in within a few days

@jhass

This comment has been minimized.

Member

jhass commented Nov 6, 2012

okay cool, remember @prellele's PR for some specs for this ;)

@prellele

This comment has been minimized.

Contributor

prellele commented Nov 9, 2012

Some more information for this issue:

Notifications are completly working for the author of the conversation.
For all recipients the first update(creating message) of every recipient triggers the notifications.
(as far as I know)

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 10, 2012

OK so I bounced some messages back and forth between me and a test user on my own pod. Conversation was started by test user and as expected I stopped receiving the new message notification in the UI after the first message, but the test user always got it.

From the production.log output it's pretty clear that every time test user sends me a message, the output is pretty much identical, except this is missing:

event=push route=local sender=jaywink@iliketoast.net recipients=5 payload_type=Message

(this above is when I reply to test user)

Also of course is missing the local receive part which happens when test user receives my message:

receiving local batch for #<Message id: 2, conversation_id: 1, author_id: 1, guid: "ef222553b81fcff7", text: "dsfddsfds", created_at: "2012-11-09 22:45:42", updated_at: "2012-11-09 22:45:42", author_signature: "Cd5Tvs78C5s51lGZsJ+YrkGRklea7wLXOrb0/mEGpbiUaVysc47...", parent_author_signature: nil>
{:event=>:sign_with_key, :status=>:complete}
receiving local batch completed for #<Message id: 2, conversation_id: 1, author_id: 1, guid: "ef222553b81fcff7", text: "dsfddsfds", created_at: "2012-11-09 22:45:42", updated_at: "2012-11-09 22:45:42", author_signature: "Cd5Tvs78C5s51lGZsJ+YrkGRklea7wLXOrb0/mEGpbiUaVysc47...", parent_author_signature: nil>

So no local dispatch is done is the problem? If so, the problem is probably in this call:

Postzord::Dispatcher.build(current_user, message).post

So after trying out various things I went the traditional way of adding a log printout to every decision point that the code can get to (best way to debug! :)). What I noticed was that the path separated totally at this point: https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L75

So, due to @sender.owns?(@object.parent) when posting as conversation starter, it ends up taking the self.notify_local_users(local_people) path. When posting as conversation recipient, it takes the self.deliver_to_local(local_people) route (and this generates a notification to conversation starter).

I don't have the knowledge to really say what is the difference with these, anyone? If I force it to always take the route that generates a notification to the conversation starter, when posting as conversation starter it gets notified itself of the reply. If I post as conversation recipient, a nice little loop is triggered and I have to kill the server :)

Anyone who actually knows Ruby want to figure out what is actually wrong and the best way to fix it? :D

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 10, 2012

Also, the reason I guess the first notification is delivered to the recipient is that the first message is a Conversation object and thus falls down to self.deliver_to_local(local_people).

event=push route=local sender=testdude@iliketoast.net recipient=jaywink@iliketoast.net payload_type=Conversation

I don't really understand the whole logic of the dispatcher, what is supposed to do what. If this is hard to fix in a clean way, maybe better remove the 0.0.2 milestone and rewrite it in a sane way (unless of course the code makes perfect sense but I don't just get it :)). Though a quick fix just to get these notifications would be awesome if someone can figure out what quick fix needs to be added.

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 10, 2012

Also, it seems this issue as entirely only when recipient of message is on the same pod - at least some testing between pods generates notifications on both ends. Will continue debugging everything past self.notify_local_users(local_people) - assuming the Dispatcher logic is ok.

(for once federation works better than local traffic haha)

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 10, 2012

I tried to fix this in the dispatcher by making sure that a correct event is dispatched, but it really didn't come out very clean. Unwanted side effects and I'm thinking lots of rewriting of the dispatcher logic to make it simpler would need to be done there, afaik.

So I went the easy way. As whatever happens notification.notify will be called, I put an increase_unread method there. It seems to work, though only tested a few times. The tests by @prellele fail still - will look at that. Also need to rebase and verify again in my own pod, but no time now, will continue tomorrow with a pull.

Anyway, if someone wants to comment, here are the small changes: jaywink@c700b86

@jaywink

This comment has been minimized.

Contributor

jaywink commented Nov 11, 2012

Fix in pull #3727

DeadSuperHero added a commit that referenced this issue Nov 19, 2012

Merge pull request #3727 from jaywink/feature/2321-fix-message-notifi…
…cations

Fix notifications for private messages between local users, fixes #2321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment