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

Fix notifications for non-contacts interactions #6498

Closed
Closed
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -98,6 +98,7 @@ def self.federated_class

def initialize(person, target, text)
@text = text
@dispatcher_opts = {additional_subscribers: target.comments_authors.where("id !=#{person.id}")}

This comment has been minimized.

Copy link
@jhass

jhass Oct 14, 2015

Member

where.not(id: person.id)

super(person, target)
end

@@ -9,6 +9,7 @@ Feature: Notifications
| email |
| bob@bob.bob |
| alice@alice.alice |
| carol@carol.carol |

Scenario: someone shares with me
When I sign in as "bob@bob.bob"
@@ -67,6 +68,80 @@ Feature: Notifications
Then I should see "commented on your post"
And I should have 1 email delivery

Scenario: unconnected user comments in reply to comment by another user who commented a post of someone who she shares with
Given "alice@alice.alice" has a public post with text "check this out!"
When I sign in as "bob@bob.bob"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great post, alice! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment"
When I sign out
And I sign in as "carol@carol.carol"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great comment, bob! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment:nth-child(2)"
When I sign out
And I sign in as "bob@bob.bob"
And I follow "Notifications" in the header
Then the notification dropdown should be visible
And I should see "also commented on"
And I should have 3 email delivery


Scenario: unconnected user comments in reply to my comment to her post
Given "alice@alice.alice" has a public post with text "check this out!"
When I sign in as "carol@carol.carol"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great post, alice! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment"
When I sign out
And I sign in as "alice@alice.alice"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great post, carol! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment:nth-child(2)"
When I sign out
And I sign in as "carol@carol.carol"
And I follow "Notifications" in the header
Then the notification dropdown should be visible
And I should see "also commented on"
And I should have 2 email delivery

Scenario: connected user comments in reply to my comment to an unconnected user's post
Given "alice@alice.alice" has a public post with text "check this out!"
And a user with email "bob@bob.bob" is connected with "carol@carol.carol"
When I sign in as "carol@carol.carol"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great post! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment"
When I sign out
And I sign in as "bob@bob.bob"
And I am on "alice@alice.alice"'s page
And I focus the comment field
And I fill in the following:
| text | great post! |
And I press "Comment"
Then I should see "less than a minute ago" within ".comment:nth-child(2)"
When I sign out
And I sign in as "carol@carol.carol"
And I follow "Notifications" in the header
Then the notification dropdown should be visible
And I should see "also commented on"
And I should have 3 email delivery

This comment has been minimized.

Copy link
@jhass

jhass Oct 13, 2015

Member

Aren't we missing one where Carol comments, Bob comments, Carol comments, Bob should have a notification?

This comment has been minimized.

Copy link
@cmrd-senya

cmrd-senya Oct 13, 2015

Author Member

You mean "Bob comments on Alice's post, Carol also comments on that post -> notification for Bob" that received notification fine before my patch? I'm fine implementing it.

This comment has been minimized.

Copy link
@jhass

jhass Oct 13, 2015

Member

I didn't read the surrounding cukes tbh, but yeah if it's not there already it might be a good addition here.

Scenario: someone mentioned me in their post
Given a user with email "bob@bob.bob" is connected with "alice@alice.alice"
And Alice has a post mentioning Bob
@@ -24,5 +24,8 @@ def update_comments_counter
update_all(:comments_count => self.comments.count)
end

def comments_authors
Person.where(id: comments.select(:author_id).distinct)
end
end
end
@@ -5,13 +5,14 @@ class Generator
def initialize(user, target)
@user = user
@target = target
@dispatcher_opts ||= {}
end

def create!(options={})
relayable = build(options)
if relayable.save!
logger.info "user:#{@user.id} dispatching #{relayable.class}:#{relayable.guid}"
Postzord::Dispatcher.defer_build_and_post(@user, relayable)
Postzord::Dispatcher.defer_build_and_post(@user, relayable, @dispatcher_opts)
relayable
end
end
@@ -61,5 +61,14 @@ def notify_users
@users.find_each do |user|
Notification.notify(user, @object, @object.author)
end
if @object.respond_to?(:target)
additional_subscriber = @object.target.author.owner
elsif @object.respond_to?(:post)
additional_subscriber = @object.post.author.owner
end

Notification.notify(additional_subscriber, @object, @object.author) if additional_subscriber &&
additional_subscriber != @object.author.owner &&
!@users.exists?(additional_subscriber.id)

This comment has been minimized.

Copy link
@jhass

jhass Oct 14, 2015

Member

Make this a regular if, don't add newlines to a modifer-if

This comment has been minimized.

Copy link
@cmrd-senya

cmrd-senya Oct 14, 2015

Author Member

Rubocop asked me of opposite :)

This comment has been minimized.

Copy link
@jhass

jhass Oct 14, 2015

Member

Meh, extract the condition to private method perhaps?

end
end
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.