Skip to:
Content

bbPress.org

Opened 2 months ago

#3612 new regression

Strange Testing Scenario with bbp_hide_forum

Reported by: sirlouen's profile SirLouen Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Component - Forums Keywords: dev-feedback 2nd-opinion needs-patch
Cc:

Description

The Unit Test

<?php
        // Hidden category.
        $c = $this->factory->forum->create( array(
                'post_title' => 'Hidden Category',
        ) );

        bbp_hide_forum( $c );
        $category = bbp_get_forum_permalink( $c );
        $this->assertSame( 'http://' . WP_TESTS_DOMAIN . '/?forum=hidden-category', $category );

It is currently failing, according to the test result, the forum is not receiving a hidden-category pretty name, but a forum number.

After deeper inspection, I observed that in this commit:

GH: https://github.com/bbpress/bbPress/commit/aa1b10ec5905d956d307b331ab0bd9d84904f1d0
Trac: https://bbpress.trac.wordpress.org/changeset/6585/

The call to function bbp_clean_post_cache was modified to the clean_post_cache and the bbp_clean_post_cache moved as a callback to the action hook of clean_post_cache

Firstly, I could not really reproduce this issue, in a real testing environment. If I create a new forum, with the "Hidden" property, it does receive a pretty name correctly. So I'm not 100% sure how this Unit Test is triggering the issue and if it's actually possible to reproduce it on the site.

Secondly, I've observed that if I switch back the bbp_clean_post_cache in the bbp_hide_forum function, the test passes, but 4 errors pop in

Personally, I cannot grasp which is the logic of doing a clean_post_cache in this function exactly.

I see 2 optional solutions:

  1. Since I don't really see if just removing clean_post_cache could make sense, I don't really understand either if switching back to the bbp_clean_post_cache
  2. Considering I'm completely unable to reproduce on the site something equivalent to the Unit Test, I'm not really sure either, if this Unit Test makes sense, of if it should be improved more according to the web behavior.

So, in this case, I don't really know which of the two solutions (or maybe a 3rd I'm overlooking), could make more sense.
Personally, I would rather opt with the most conservative one, which is leaving the function, but the one that actually works with the Unit Test bbp_clean_post_cache.

Change History (0)

Note: See TracTickets for help on using tickets.