Poor Editing Template Logic

[mordauk]

A few weeks ago I was working on a bbPress site and ran into an interesting problem: the edit topic / reply pages didn't work. When attempting to edit a topic or reply, the page would simply reload without showing any of the editing controls.

At the time I was unable to discover what was causing this, but I've just now figured it out. in bbp-template-loader.php, there are functions for determining the template files that should be used for the edit templates. The edit topic template is defined with this

/**
 * Get the topic edit template
 *
 * @since bbPress (r3311)
 *
 * @uses bbp_get_topic_post_type()
 * @uses bbp_get_query_template()
 * @return string Path to template file
 */
function bbp_get_topic_edit_template() {
	$post_type = bbp_get_topic_post_type();
	$templates = array(
		'single-' . $post_type . '-edit.php', // Single Topic Edit
		'single-' . $post_type . '.php',      // Single Topic
	);
	return bbp_get_query_template( 'topic_edit', $templates );
}

bbPress first looks for single-topic-edit.php and then single-topic.php.

The problem I have with this is that it makes it impossible to have a single-topic.php file in the theme folder (unless that file has the edit controls) and still retain the ability to edit topics on the front end. If the theme also has single-topic-edit.php it will work fine, but not with just single-topic.php.

Since the single-{post type}.php is part of WP core, this seems like a really poor template name to include in the hierarchy for the edit view, especially when just above bbp_get_edit_topic_template() is this:

/**
 * Get the single topic template
 *
 * @since bbPress (r3922)
 *
 * @uses bbp_get_topic_post_type()
 * @uses bbp_get_query_template()
 * @return string Path to template file
 */
function bbp_get_single_topic_template() {
	$templates = array(
		'single-' . bbp_get_topic_post_type() . '.php'
	);
	return bbp_get_query_template( 'single_topic', $templates );
}

So bbPress is already planning to have support for single-topic.php, but it doesn't work fully with the single-topic.php also being used for the edit view.

I don't think the edit view should ever look for single-topic.php, since the presence of it and the lack of single-topic-edit.php will cause the edit view to fail.

Would replacing single-topic.php with something else cause any significant issues?

[anointed]

I posted this in another duplicate thread, but it might explain the problem I was having:

Running trunk as of today on WordPress 3.5beta1 bbPress 2x, buddypress 1.7

When I try to edit a group topic or reply nothing happens.

It's easiest to explain via a video: ​http://www.youtube.com/watch?v=xZ___b7gQGU&feature=plcp

hope it helps.

[mordauk]

Yep that's the exact same behavior that I see.

I've just confirmed that removing single-topic.php from the theme does fix the problem.

[johnjamesjacoby]

If memory serves, this is actually intentional for the reasons you've outlined here.

bbp_template_include_theme_supports() was intended to bypass this, and I think that's where the issue is.

The bbp_is_topic_edit() check is intentionally before the bbp_is_single_topic() check, and the -edit.php template is ahead of the non -edit.php template in the stack. The way it should work is:

• if the -edit.php file exists, use it
• if not, show something, and use the single-template.php to prevent a 404.

If you want to start looking around, bbp-template-loader.php is the place to start.

Note that the forums and replies templates work this same way.

[mordauk]

Replying to johnjamesjacoby:

• if the -edit.php file exists, use it
• if not, show something, and use the single-template.php to prevent a 404.

That makes sense, though it does seem problematic to me that a user is required to have single-topic-edit.php if they want to use single-topic.php. Obviously it's not a big deal to do that, but it does cause a serious case of head scratching.

I haven't dug deep enough yet, but what about updating the default theme-compatibility single views to include if( IS_EDIT ) conditionals? That way when a user copies the contents of the default file to single-TYPE.php, the type remains editable?

[mordauk]

Is there a plan for how to address this? I'll be happy to work on it, just not sure which way you want to go.

[johnjamesjacoby]

Replying to mordauk:

Is there a plan for how to address this? I'll be happy to work on it, just not sure which way you want to go.

Let's chat about how to reproduce this. I'm still fuzzy on exactly what the defect is.

[johnjamesjacoby]

(In [4324]) Theme Compat:

• Introduce bbp_get_template_stack() and bbp_register_template_stack().
• Used to provide an accurate hierarchy to individual template locations.
• Also enables an unlimited number of parent/child template relationships to be registered.
• See #1968.

[johnjamesjacoby]

Confirmed r4324 fixes this issue in trunk. The gist is, this new template stack approach preserves the hierarchy of template-names separate from the hierarchy of the stack. This allows us to walk the two separate arrays in step, without doing bulk loops for each file, potentially catching one out of order.

Closing as fixed.

[johnjamesjacoby]

(In [4325]) Template Tags:

• Check for edit when calling _is_ single forum/topic/reply.
• See #1968.

[johnjamesjacoby]

(In [4326]) Theme Compat:

• Reverse the look-up of bbp_locate_template() and check $template_names first.
• When a template is located, break out of both loops.
• See #1968.

[johnjamesjacoby]

(In [4327]) Theme Compat:

• Only iterate through bbp_get_template_stack() one time per call to bbp_locate_template().
• See #1968.

[johnjamesjacoby]

(In [4328]) Theme Compat:

• Remove single- fallback from forum/topic/reply/topic-tag edit templates.
• See #1968.