bbp_form_topic_subscribed() is wrong when editing a reply

[wpdennis]

If a user subscribes to a topic and then edits one of his replies in this topic, bbp_get_form_topic_subscribed() will not return 'checked' as it should in reply-form.php.

The reason is that bbp_get_global_post_field( 'post_author', 'raw' ) returns a string here:

<?php
elseif ( bbp_is_topic_edit() || bbp_is_reply_edit() ) {

        // Get current posts author
        $post_author = bbp_get_global_post_field( 'post_author', 'raw' );

        // Post author is not the current user
        if ( bbp_get_current_user_id() !== $post_author ) {
                $topic_subscribed = bbp_is_user_subscribed( $post_author, bbp_get_topic_id() );

                // Post author is the current user
        } else {
                $topic_subscribed = bbp_is_user_subscribed( bbp_get_current_user_id(), bbp_get_topic_id() );
        }
}

And in the following the checks won't work, most critically the array_search() here, where $user_id is still a string:

<?php
function bbp_is_object_of_user( $object_id = 0, $user_id = 0, $meta_key = '', $meta_type = 'post' ) {
        $user_ids = bbp_get_users_for_object( $object_id, $meta_key, $meta_type );
        $retval   = is_numeric( array_search( $user_id, $user_ids, true ) );

        // Filter & return
        return (bool) apply_filters( 'bbp_is_object_of_user', $retval, $object_id, $user_id, $meta_key, $meta_type );
}

---

Bugfix: Add cast and reduce unnecessary code in bbp_get_form_topic_subscribed():

<?php
elseif ( bbp_is_topic_edit() || bbp_is_reply_edit() ) {
        // Get current posts author
        $post_author = (int) bbp_get_global_post_field( 'post_author', 'raw' );
        $topic_subscribed = bbp_is_user_subscribed( $post_author, bbp_get_topic_id() );
}

It should be enough to always check the post_author. The check bbp_get_current_user_id() !== $post_author is unnecessary because it says:

• If the current user is not the author, take the author
• If the user is the author, take the user.

In the end it's always the author.

To make sure bbp_is_object_of_user() works in all instances, an explicit cast should be considered, too. Otherwise bugs like this are really sneaky.

<?php
function bbp_is_object_of_user( $object_id = 0, $user_id = 0, $meta_key = '', $meta_type = 'post' ) {
        $user_ids = bbp_get_users_for_object( $object_id, $meta_key, $meta_type );
        $retval   = is_numeric( array_search( (int) $user_id, $user_ids, true ) );

        // Filter & return
        return (bool) apply_filters( 'bbp_is_object_of_user', $retval, $object_id, $user_id, $meta_key, $meta_type );
}

[netweb]

Thanks @wpdennis

[johnjamesjacoby]

I believe the !== $post_author check exists so that Moderators can edit other people's replies and edit that author's subscription status instead of their own.

[johnjamesjacoby]

In 6828:

Engagements: enforce absint() on function parameters in engagements API.

Also fixes a bug causing an array_search() to erroneously fail.

See #3206.

[johnjamesjacoby]

In 6829:

Subscriptions: cast function results to (int) so strict comparisons pass.

This change fixes a bug where subscription checkboxes and UI elements could show/use incorrect values.

Props wpdennis. Fixes #3206.

[wpdennis]

Replying to johnjamesjacoby:

I believe the !== $post_author check exists so that Moderators can edit other people's replies and edit that author's subscription status instead of their own.

Exactly, it's always the post author. The code is just a very complex way of saying take always the post author no matter what:

<?php
if ( bbp_get_current_user_id() !== $post_author ) {
                $topic_subscribed = bbp_is_user_subscribed( $post_author, bbp_get_topic_id() );

                // Post author is the current user
        } else {
                $topic_subscribed = bbp_is_user_subscribed( bbp_get_current_user_id(), bbp_get_topic_id() );
        }

As I wrote above:

• If the current user is not the author, take the author
• If the user is the author, take the user (who is the author)

There is no way (and no reason) this complex snippet is performed with any other id than the post author id.

[johnjamesjacoby]

In 6830:

Subscriptions: simplify edit logic for getting checked() value.

Props wpdennis. See #3206.