Skip to:
Content

bbPress.org

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#3205 closed defect (bug) (fixed)

Updating replies always deletes "bbp_reply_to"

Reported by: wpdennis's profile wpdennis Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.6 Priority: high
Severity: major Version: 2.2
Component: Component - Replies Keywords: dev-feedback
Cc: info@…

Description (last modified by netweb)

With threaded replies "bbp_reply_to" is deleted whenever users edit their own replies (except they are moderators).

This happens because the input field in form-reply.php is only available for moderators. For non moderators isset( $_REQUESTbbp_reply_to? ) is always false and therefore $reply_to will always be 0 in bbp_edit_reply_handler:

<?php
    $reply = $reply_id = $reply_to = $reply_author = $topic_id = $forum_id = 0;

    ....

    if ( isset( $_REQUEST['bbp_reply_to'] ) ) {
        $reply_to = bbp_validate_reply_to( $_REQUEST['bbp_reply_to'], $reply_id );
    }

Additionaly, if a user knows the code he can just set any id as bbp_reply_to, even without the capability. I think this would fix both issues:

<?php
    if ( isset( $_REQUEST['bbp_reply_to'] ) && current_user_can( 'moderate', $reply_id ) ) {
        $reply_to = bbp_validate_reply_to( $_REQUEST['bbp_reply_to'], $reply_id );
    } elseif ( bbp_thread_replies() ) {
      $reply_to = bbp_get_reply_to( $reply_id );
  }

Another route would be to not update reply_to in updates, except the user is a moderator. E.g. in bbp_update_reply:

<?php
// Only moderators can update the reply-to id
if ( current_user_can( 'moderate', $reply_id ) ) {
    bbp_update_reply_to( $reply_id, $reply_to );
}

This line can be removed because it runs twice and doesn't change anything at this point:

<?php
    $reply_to = bbp_validate_reply_to( $reply_to, $reply_id );

Change History (5)

#1 @netweb
7 years ago

  • Description modified (diff)

#2 @johnjamesjacoby
7 years ago

  • Milestone changed from Awaiting Review to 2.6
  • Owner set to johnjamesjacoby

#3 @johnjamesjacoby
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In 6826:

Replies: use existing reply_to value if none is passed.

This change fixes a bug that would cause the reply hierarchy to be broken when non-moderator users would edit their existing replies within the allotted editing period.

Props wpdennis. Fixes #3205.

#4 @johnjamesjacoby
7 years ago

  • Version set to 2.2

Thanks @wpdennis for this fix. Nice to see you again, too!

I opted to leave the extra call to bbp_validate_reply_to() in bbp_update_reply() because that function is called in other circumstances. You're right that when editing a single reply that it is validated twice. When called directly though (via the meta-box save or elsewhere) it still needs validation.

Great work on spotting this. It's been around for a while, and nobody's prioritized it.

#5 @wpdennis
7 years ago

Thanks @johnjamesjacoby and thanks for the very quick patch!

because that function is called in other circumstances.

bbp_validate_reply_to() is always called in bbp_update_reply_to() itself and I think because of this the call in bbp_update_reply() is always redudant and will never change anything.

But this isn't a real issue. It was just something I noticed because of this ticket.

Note: See TracTickets for help on using tickets.