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

find original post links across all previous posts in a reply chain, not just the first #217

Closed
snarfed opened this issue Jul 5, 2014 · 10 comments
Assignees
Labels

Comments

@snarfed
Copy link
Owner

snarfed commented Jul 5, 2014

right now, we're designed entirely for the post-and-comments model. we find the original post for a silo post, then send all responses to it.

however, comments themselves can have original posts. in practice, this happens most often on twitter, since replies (comments) are themselves full fledged tweets.

here's an example. @kartikprabhu's first two tweets in the chain both have original posts; the second with a link, the first with a rel-syndication link here.

right now, when there are multiple candidates like this, each response ends up attached to a single candidate, arbitrarily. we should ideally attach it to the original tweet and the tweet it's directly replying to, or maybe all possible candidates. not sure which.

for facebook, G+, and instagram, we don't perform original post discovery on comments at all, so responses always get sent to the original post. we could theoretically do the same thing for them too.

@snarfed
Copy link
Owner Author

snarfed commented Jul 5, 2014

@kartikprabhu, just fyi, i doubt i'll prioritize this in the near future. it'll take a bit of thought and work, but it's definitely doable. feel free to take a stab at it if you want!

@snarfed
Copy link
Owner Author

snarfed commented Jul 8, 2014

this actually also sometimes breaks twitter reply chains so that they stop getting sent at all. it happened to @lostfocus in this conversation. the first couple replies worked, but then @pfefferle's second reply got attached to dominik's first reply as its activity, instead of his original tweet, so it didn't get any original post links.

it looks like an easy heuristic fix for just this bug (ie not the full feature request) might be to attach the response to the earliest activity we can find, ie the one with the lowest id, and not the first one we see.

the full fix might not be too much more work though...

@snarfed snarfed added now and removed later labels Jul 8, 2014
@snarfed snarfed self-assigned this Jul 11, 2014
snarfed added a commit that referenced this issue Jul 12, 2014
just fyi @kylewm, @kartikprabhu. small diff but kinda big change to the data model!
snarfed added a commit that referenced this issue Jul 12, 2014
@snarfed
Copy link
Owner Author

snarfed commented Jul 12, 2014

@kartikprabhu, the new behavior for tweet reply chains is, for a given response, you'll get a webmention for each tweet (post) of yours that's a direct ancestor in the chain. (sorry, i know you wanted it to just go to the most recent one. i'm going with this instead for now, but i might reconsider in the future. we'll see!)

@snarfed snarfed closed this as completed Jul 12, 2014
@kartikprabhu
Copy link

The more I have thought about this, the more complicated it seems. I'll play around with this privately once I get activity-streams working on my own. I'll let you know of any (unlikely) bright ideas I stumble upon.

@snarfed
Copy link
Owner Author

snarfed commented Jul 14, 2014

reopening because the mf2 handlers don't get this quite right. handler (ie webmention source) urls all need a single root activity id. we currently just use the first, and then only include its original post url(s).

here's an example. jesse's reply tweet was sent both there and to the full chain, but its mf2 handler only links to the "committed fix" tweet, not the earlier one. propagate log here.

possible fix is to use Response.activities_json to populate original post links in handlers instead of the APIs. i guess that's #15, at least in part?

@snarfed snarfed reopened this Jul 14, 2014
@snarfed
Copy link
Owner Author

snarfed commented Jul 15, 2014

so...this is hard. 😖

@kartikprabhu
Copy link

yeah... I have been thinking of how to do this too but no ideas... I propose that bridgy go back to doing the simplest possible thing and leave the complexities to the webmention handlers.

@snarfed
Copy link
Owner Author

snarfed commented Jul 15, 2014

i actually see how i want to do it on the bridgy side now. it's definitely doable. and hopefully the new behavior should be harmless for webmention recipients, right? i don't think anyone's actually trying to do the single unified thread yet, so this should still work ok for everyone without any new de-duping or other logic.

snarfed added a commit that referenced this issue Jul 15, 2014
…tion source URLs

for #217. the mf2 handlers that handle our webmention source URLs always refetch the silo activity and response(s) and use that data alone. they don't use stored data in the datastore. they only use a single activity, so responses with multiple activities only get the original post URLs from a single activity.

ideally i'd switch the handlers to use the data we've already stored in the Response, but that's surprisingly harder than it sounds. with this, when a response has multiple activities, the mf2 handler source URLs still don't include all of the original post URLs, but we use the appropriate activity for each target so that they get the original post URLs they need.
@snarfed
Copy link
Owner Author

snarfed commented Jul 15, 2014

b75a01d

@snarfed
Copy link
Owner Author

snarfed commented Jul 15, 2014

confirmed kill! here's the test twitter thread. the "baz!" reply got sent to both the first post and the second post. here's the log.

closing (again) as fixed. phew. i need a 🍺.

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

No branches or pull requests

2 participants