Comments: Update post title link to point at post permalink

[gwwar]

Currently it links to the comment permalink ("blog.blog/post/comment-page-1/#comment-123"), which is the only post url available. We should update this to point at the post permalink instead eg blog.blog/post/

cc/ @Automattic/lannister

[kwight]

Currently, the comment data we receive from an API call to sites/(site)/comments will not have the permalink of the comment's parent post. There is the post ID however, and we would know the site ID also; these two pieces of info would allow us to fetch the post itself for its URL.

There are a few ways to handle this, I'm not sure which is best.

1. Add the parent post URL to the WordPress.com API response of /site/(site)/comments. I think we can just rule this option out.
2. Use the data layer to add the URL into the comment item data in state. This makes it immediately available to the getSiteComments selector everywhere.
3. Pass the required posts data into CommentList through connect() (which in turn passes it through to CommentDetail). A method would need to be added to CommentList to inject the needed URL into each comment as it's passed to CommentDetail. This could also mean a new selector (maybe getSiteCommentsParentPosts()). I guess a simpler version is to do it all in mapStateToProps: get the posts, iterate over the comments and inject the URL, then pass the new comments into CommentList.
4. Use a method in CommentList that injects the URL into each comment object by fetching the single post data itself; it would accept comment, fetch the post data from state, inject the title, then return the new comment.
5. Use regex to strip the #comment-(id) part from the end of URL and call it a day. It would probably be fine, but feels fragile.

I'm thinking #2 is the best way to go about this; it takes the data we're getting from the API, and shapes it into what we need before saving it to our state. It seems like I would add the additional work here, but that looks suspicious and hacky, with the other handlers looking more simple; am I missing something?

[dmsnell]

Am I missing something or is this not easy? If we have the postId, which I think we should in the comment data, can't we just link into the post in the reader view?

[Copons]

Use regex to strip the #comment-(id) part from the end of URL and call it a day. It would probably be fine, but feels fragile.

Bad approach, but still, just wanted to point out that we'd also need to strip the /comment-page-(pageNumber) from the URL in case of paginated comments.

E.g. a post URL in one of my test sites:
https://testblog.wordpress.com/2016/08/11/blog-post/comment-page-1/#comment-2

[gwwar]

Use regex to strip the #comment-(id) part from the end of URL and call it a day. It would probably be fine, but feels fragile.

@kwight that should be an ok approach if we parse and reconstruct it using the url lib. It mirrors the node url api

[kwight]

Am I missing something or is this not easy?

Maybe it's easy for you, but it isn't clear for me; that's why I'm asking for guidance.

If we have the postId, which I think we should in the comment data, can't we just link into the post in the reader view?

We do have that post ID (and the site ID), but looking in state, that full post is not available anywhere I can see. (Not sure what you mean by the Reader view – why would the Reader be concerned about a user's post with comments on it? Are a user's posts supposed to be represented in state/reader somehwere?..)

I guess my main questions are:

1. Should that post be fetched and its data saved in /comments/items/(item)/(index)/post, so that it's available whenever getSiteComments is called? I think this would be done in the data layer, at this spot I mentioned in my previous comment?
2. If not, then the post needs to be fetched and saved to state somehow in CommentList; would Menus: Alignment issue with undo link in is-info notice after deleting menu #3 or Site Picker: Always show search if user has hidden site #4 above be better for that?

[dmsnell]

@kwight please don't let me sound like I'm talking down to you. I don't know the context of the problem so that's why I ask.

that full post is not available anywhere I can see…why would the Reader be concerned about a user's post with comments on it?

I should say that my interpretation of the issue description is that we have a link in the comment to the parent post and when we click on that we want to take the user to the post itself. If that's our goal then all we need are siteId and postId and we can generate a link to the post in the Reader view - "deep linking" if you will (except we're already in the app).

It won't matter if the post is in state already, which is the beauty of how our approach with Redux and declarative data dependencies work. Once that page renders it will load in the post.

This is where I was curious about the difficulty because if we would like to direct people to a Calypso view then the functionality is already there and we need perform no string operations.

[davewhitley]

Dennis brings up a good point. I assumed we would just link to the external site, but we should probably link to the Reader post view. It's hard to know what the user expects or what we want them to expect, when it comes to either linking to the site or Calypso.

[kwight]

we can generate a link to the post in the Reader view - "deep linking" if you will

Ah! Now I understand what you meant about the Reader; right, that makes sense. I had assumed like @drw158 that we were talking about an external link, but it sounds like we all agree a Reader view would be better, eg. http://calypso.localhost:3000/read/blogs/12754318/posts/2512. Working in #16162 .