Improve federation regarding reshares and root post author

[jaywink]

When author of the root post receives a reshare to it, no participation is added to the root author on the reshare. This causes any comments on the reshare on remote pods not to be sent to the author. Adding a participation should subscribe to the reshare and thus bring added comments back to the author.

Did some live testing with develop production pods to make sure of this case. Basically:

• User A posts a post on pod A
• User B, who follows user A, reshares it on pod B
• User C, who follows user B, comments on the reshare of B on pod C

Expected:

• Pod A should receive the reshare and the comment
• Pod B should receive the reshare and the comment
• Pod C should receive the reshare and the comment (has authored it)

What happens:

• Pod A receives only the reshare, not the comment
• Pod B receives the reshare and the comment
• Pod C receives the reshare and the comment (has authored it)

[jaywink]

Also, the bug in action:

https://iliketoast.net/posts/775355 <-- my reshare of HQ post, has a comment from Sean
https://pod.diaspora.software/p/0746ec1050d10133bafa6c626dd55703 <-- my reshare on HQ, never got the comment

[jaywink]

Hmm actually I need to continue testing this patch tomorrow...

[jaywink]

Actually, this ended up not being so simple. I managed to (by moving the code to lib.postzord.receiver.local_batch.receive!) get the participations to be created, so for example a situation where:

• User A writes a post, doesn't follow anyone
• User B follows user A, reshares user A post
• User C follows user B, reshares user B reshare (ie user A post)

What happens now is that a participation is never created for user A on the reshared posts, which means that any replies will not arrive on user A pod. I managed to get that part working, ie participations are created to the root author always, independent of how far in the chain the resharing user is.

But, if user C comments on his/her reshare, the comment only comes to user B's pod, not user A's. I can't really figure out how to handle the situation, to make either user B pod relay the comment to user A or something else.

Anyway, I realized too that this changes quite a few things which maybe not everyone will like, though personally I would like. So, if this change was implemented, the original root author would always be subscribed to comments and likes to reshares. Of course, this is quite a big change in functionality, for me to the better..

What do people think - shall I continue working on fixing the relationships regarding participations in reshares?

If you are wondering what the problem is - it is basically this:

User A posts a post
User B reshares
User C reshares (from following user B)

--> all the reshares end up on user A pod, but none of the interactions to it. This is broken functionality where the post exists but not the participations. We should somehow fix it OR not deliver the reshare to the root author pod at all.

[jaywink]

OK I've added functionality now to also push comments made to reshares to the original author. This, with the first comment, will cause also a notification to the reshare author, unless the "notifications for non-contacts" bug is hit.

I can't figure out how to write the test so that I can first build a comment, then except it in the dispatcher and then trigger everything so that I can check that defer_build_and_dispatch gets the correct additional_subscribers. Any pointers towards how to rewrite the test a bit for that?

[jaywink]

For the changed functionality, would love some feedback whether it is acceptable. I guess it could be configurable ie "notify on all reshare activity" etc. The participation part could also be removed and only make comments federate better, I thought these are linked but not really in the end, so this PR kind of contains two changes. Can separate if wished.

Ping for opinions @Flaburgan @goobertron @svbergerem @jhass @denschub @AugierLe42e

[ghost]

OK I've added functionality now to also push comments made to reshares to the original author.

Do you think it is possible for the comments to be the same on all the reshares? Because doing this could cause the problem that otheer people answer on the original post to a comment that has been made on a reshare so the author of the comment does not know someone has answered him (because he is notified for the reshare and not the original post).

[jaywink]

Do you think it is possible for the comments to be the same on all the reshares?

Where the comments link hasn't been changed, the comment is just delivered to one extra place: the root authors pod.

[jaywink]

I think @Flaburgan approved in principle in IRL discussion here at the hackathon, the adding participation between root author and reshare posts. Correct me if understood wrong :) I think it adds value to users to be notified when their posts are interacted with.

[diaspora-code-review]

Use a guard clause instead of wrapping the code inside a conditional expression.

[jaywink]

Rebased, but still not really sure how to write the test. Any help appreciated.

Btw, it's awesome receiving notifications for other people commenting on my reshares. Would really love to get this in. Also the comment federation would be appreciated by many.

Anything I can do to move this?

[Flaburgan]

Yep +1 from my side.

[jhass]

This line is not needed.

[jaywink]

Thanks @jhass for the review. I still can't figure out this however, how to get @comment.save! (which I assume is the right way to go from here) to end up calling defer_build_and_post.

I assume I have to do some allow etc magic, but, as always, rspec got the better of me. I'll try reading up on it during the weekend, or if you have a quick hint, appreciated. Maybe the code doesn't work yet, but since the test doesn't even end up in that section (verified via debugger), difficult to say for sure.

[jhass]

Let's trace the call chain. Creating a public comment via the UI:

• https://github.com/diaspora/diaspora/blob/develop/app/controllers/comments_controller.rb#L15
• https://github.com/diaspora/diaspora/blob/develop/app/services/comment_service.rb#L15
• https://github.com/diaspora/diaspora/blob/develop/app/models/user/social_actions.rb#L4
• https://github.com/diaspora/diaspora/blob/develop/app/models/comment.rb#L94
• https://github.com/diaspora/diaspora/blob/develop/lib/federated/generator.rb#L11
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L50
• https://github.com/diaspora/diaspora/blob/develop/app/workers/deferred_dispatch.rb#L16

So no, save! doesn't trigger a callback that calls create!, rather it's called explicitly and is what calls save!. Let's pause here for a second and look at ...

Receiving a public comment via federation:

• https://github.com/diaspora/diaspora/blob/develop/app/controllers/publics_controller.rb#L24
• https://github.com/diaspora/diaspora/blob/develop/app/workers/receive_unencrypted_salmon.rb#L12
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver.rb#L14
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L24
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L54
• https://github.com/diaspora/diaspora/blob/develop/lib/diaspora/parser.rb#L13
• Effectively Comment.from_xml, from_xml is ROXML magic (the xml_attr stuff in the models). No callbacks.
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L59
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L67 (We're persisted to the DB now)
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L32-L33 (https://github.com/diaspora/diaspora/blob/develop/app/models/comment.rb#L10, https://github.com/diaspora/diaspora/blob/develop/lib/diaspora/relayable.rb#L43)
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/public.rb#L45
• https://github.com/diaspora/diaspora/blob/develop/lib/diaspora/relayable.rb#L89

Now we're at Postzord::Dispatcher.build(...).post again, so the code paths converge here.

• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L34
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L66
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L85
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L126
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/dispatcher.rb#L139
• https://github.com/diaspora/diaspora/blob/develop/app/workers/receive_local_batch.rb#L11-L12
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/local_batch.rb#L19
• https://github.com/diaspora/diaspora/blob/develop/lib/postzord/receiver/local_batch.rb#L36
• https://github.com/diaspora/diaspora/blob/develop/lib/diaspora/relayable.rb#L69 (the other codepath now, where we own the parent).

That should show all relevant "callback"/"hook" situations, the last part rather being for completeness.

It's probably fair if you just do the testcase for Federated::Generator directly, using a test subclass that returns stub (double) values. Or you argue that it's opaque behaviour of the CommentsController or CommentService even, though that's a bit on the far edge of unit testing.

[jaywink]

Thanks, now my head hurts ;) Will continue later with something..

[jaywink]

Hmmm odd thing with the tests. One of the builds passes (mysql 2.2), but all three others fail?

[jaywink]

Final stage:

• Send comment to reshare author when commenting on reshare. As posts are always delivered also to reshare root, comments should also be delivered to reshare root, for concistency.
• Add participation to root.author on receiving reshare. When author of the root post receives a reshare to it, no participation is added to the root author on the reshare. This causes any comments on the reshare on remote pods not to be sent to the author. Adding a participation should subscribe to the reshare and thus bring added comments back to the author.

I have nothing more to add to this so unless review gives anything and if tests pass, ready for mergy. @denschub - could this be included in tomorrows 0.5 freeze since it improves federation?

[denschub]

No objections, merged as 9a35a0d.

[TheRadialActive]

Thanks @jaywink for your work. It really saves me a lot of time :)