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

Notify about a new Discussion #8

Closed
wants to merge 2 commits into from
Closed

Notify about a new Discussion #8

wants to merge 2 commits into from

Conversation

terabin
Copy link

@terabin terabin commented Sep 4, 2016

When a new discussion start, pusher don't do anything, a discussion started is a new post too, so, makes sense?

When a new discussion start, pusher don't do anything, a discussion started is a new post too, so, makes sense?
@terabin terabin changed the title Discussion push Notify about a new Discussion Sep 4, 2016
@tobyzerner
Copy link
Contributor

tobyzerner commented Sep 4, 2016

The PostWasPosted event is actually fired upon new discussion creation as well. You can see this happening in:

  1. StartDiscussionHandler, which dispatches the PostReply command
  2. PostReplyHandler, which creates a new post with CommentPost::reply
  3. CommentPost::reply which raises the PostWasPosted event.

However I suspect it's not resulting in a Pusher message because the discussion's startPost is only updated after the PostWasPosted event is fired, and thus the $event->post->isVisibleTo(new Guest) condition is failing. (Haven't confirmed this though.)

Not sure if it's worth doing some refactoring in core so that the discussion's startPost is updated before the PostWasPosted event is fired, or just to use your solution. I will keep thinking about it.

Thanks for the PR, btw!

@luceos luceos requested a review from franzliedke June 24, 2019 09:55
Copy link
Sponsor Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not opposed to handling these 2 separately. Passing the Posted event up through to CreateDiscussionController would be a pain. However, this needs to be updated to reflect changes to Flarum over the past few years.

@askvortsov1
Copy link
Sponsor Member

Closing as outdated. I'm also not sure that this is a problem any more. @terabin feel free to reopen if this is still a problem, and you're still interested in taking it on.

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