getting an activity permalink instead of a shortlink

[DJPaul]

Our existing bp_activity_get_permalink method returns the permalink for a single activity item, right? Actually, that's not true, because if you look at the function, it returns what I'd actually describe as a short link. For example:

bp_activity_get_permalink(567) will return http://example.com/activity/p/567/.
That URL will redirect to http://example.com/members/admin/activity/567/.

I think bp_activity_get_permalink should return the canonical permalink (the second link), not the first. It would be more intuitive, bring it inline with WP's get_permalink which I'd guess many people who extend BP are familiar with, and finally give us a way to get the canonical link for a piece of activity content without having to dig into $_SERVER.

If this proposed change is accepted, I suggest we rename bp_activity_get_permalink to bp_activity_get_shortlink, and build a real bp_activity_get_permalink.

[djpaul]

In 9061:

Tests: update activity tests to use bp_activity_get_permalink to get a link to an activity item rather than manually constructing a link from an activity ID.

See #5926

[djpaul]

In 9062:

Activity: when redirecting unauthenticated users away from a single activity permalink, make sure the value of the redirect_to parameter has been URL encoded.

Also improves the readability of this section of code by moving it out of the ternary operator.

See #5926

[djpaul]

In 9063:

Activity: refactor part of bp_get_activity_latest_update to use bp_activity_get_permalink to get a link to an activity item rather than manually constructing the link.

See #5926

[DJPaul]

5926.01.patch is what I'm suggesting. It renames bp_activity_get_permalink to bp_activity_get_shortlink, and bp_activity_get_permalink is re-built to return the canonical URL instead. I've swapped some existing use out for the new shortlink function places where this function is used (where I think people might care) -- purely for aesthetic backwards consistency, rather than any technical reason.

[DJPaul]

Have asked tw2113 for an 2nd opinion regarding phpDoc for the functions and actions.

[tw2113]

My VCS-fu must not be strong enough yet, because I'm having trouble getting the patch to apply cleanly to an uptodate copy of trunk. I figure worst case scenario, I type up the phpdocs for the hooks that can be applied manually after someone with some more experience gets the patch refreshed.

[tw2113]

To add to that, [9194] reworked some of the innards of bp_activity_get_permalink as well, for example.

[boonebgorges]

Replying to tw2113:

My VCS-fu must not be strong enough yet, because I'm having trouble getting the patch to apply cleanly to an uptodate copy of trunk. I figure worst case scenario, I type up the phpdocs for the hooks that can be applied manually after someone with some more experience gets the patch refreshed.

It does not apply cleanly, probably because of [9194]. If you're able to refresh it, it'd be helpful. (This is a manual process - you'll have to manually merge the chunks of the patch that do not cleanly apply. No VCS-fu involved.)

[tw2113]

My question at this point is do we keep imath's version of bp_activity_get_permalink for example, or do we need to discuss which direction the function goes going forward. I know Paul is proposing renaming it completely.

[DJPaul]

I don't think I'll have time to refresh the patch for 2.2, but I'll get it done for 2.3.

[DJPaul]

5926.02.patch is refreshed for current trunk. The new bp_activity_get_shortlink does duplicate most of bp_activity_get_permalink, but I am struggling to think of a nice way of avoiding the duplication.

[DJPaul]

I guess I could try adding another parameter to permalink() that sets whether to return the shortlink or not, and make shortlink() into a kind of wrapper.

[DJPaul]

I forgot about this. I'll commit to this for 2.4.

[DJPaul]

I have refreshed the patch for current trunk so it applies cleanly (the since version is out of date) but one of the new unit tests is failing, and I think it's something to do with our test runner's go_to function which is pretty gnarly (it might not handle redirects?), and I don't have time/desire to figure it out for 2.4.

[DJPaul]

This patch doesn't apply cleanly anymore, and last I knew, it was breaking the tests. Punting.

[DJPaul]

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see ​https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: ​https://make.wordpress.org/chat/