Fix: column value getting overridden when multiple custom columns are added

[Nikschavan]

Fixes #1184.

The statement $out = $column_name; would get called even after the correct value is set to the variable for the first column.

I have changed this so that the default value will be used only if the variable out is still unchanged so that it retains the correct column value for all the columns.

Checklist

• Project documentation has been updated to reflect the changes in this pull request, if applicable.
• I have tested the changes in the local development environment (see contributing.md).
• I have added phpunit tests.

Release Changelog

• Fix: Column value getting overridden when multiple custom columns are added.

Release Checklist

• This pull request is to the master branch.
• Release version follows semantic versioning. Does it include breaking changes?
• Update changelog in readme.txt.
• Bump version in stream.php.
• Bump Stable tag in readme.txt.
• Bump version in classes/class-plugin.php.
• Draft a release on GitHub.

[Nikschavan]

Hi @kidunot89, @dero, @kasparsd 👋

Any chance to get a review on this?

[derekherman]

@kasparsd or @kidunot89 Correct me if I'm wrong here, but to me this seems like the problem is that the loop needs to exit if there is a value found and also that the default gets set if there isn't a value coming from the filters.

So the whole code block would change to:

if ( ! empty( $inserted_columns ) && is_array( $inserted_columns ) ) {
	foreach ( $inserted_columns as $column_title ) {

		/**
		 * If column title inserted via wp_stream_register_column_defaults ($column_title) exists
		 * among columns registered with get_columns ($column_name) and there is an action associated
		 * with this column, do the action
		 *
		 * Also, note that the action name must include the $column_title registered
		 * with wp_stream_register_column_defaults
		 */
		if ( $column_title === $column_name && has_filter( "wp_stream_insert_column_default_{$column_title}" ) ) {
			/**
			 * Allows for the addition of content under a specified column.
			 *
			 * @param object $record Contents of the row
			 *
			 * @return string
			 */
			$out = apply_filters( "wp_stream_insert_column_default_{$column_title}", $column_name, $record );
			break;
		}
	}
}

// Set default value.
if ( empty( $out ) ) {
	$out = $column_name;
}

[Nikschavan]

This should work as expected, I will update the PR

One question - If any column value is actually empty (for an example case where that column is not relevant for that log type) that gets set to $column_name everywhere, That is because of the last // Set default value. section. Is this an expected behavior or can that be removed?

[derekherman]

I’m not sure what the intended default behavior is. Is there a test for this @kidunot89

[kidunot89]

@derekherman @Nikschavan I don't believe so, so I've added some more changes. This should be all ready after somebody else reviews it.

[kidunot89]

@Nikschavan I'm sorry it took so long to get this reviewed 😞.

[kidunot89]

As @derekherman stated above the loop has to be exited once the value is found or it will just overwrite itself with the default value.

@Nikschavan You can replace the whole conditional started on Line 397 with @derekherman suggestion above and that should work.

[thefrosty]

While this was on the right track, the default filter is never called when creating a key => value pair of column(s).

So when filtering a new column, the $column_title could be Record ID, which is not a filterable value in L418.

Really this should have been:

Suggested change

	if ( $column_title === $column_name ) {
	/**
	* Allows for the addition of content under a specified column.
	*
	* @param object $record Contents of the row
	* @param string $out Column content.
	* @param object $record Record with row content.
	* @param string $column_name Column name.
	*
	* @return string
	*/
	$out = apply_filters( "wp_stream_insert_column_default_{$column_title}", $column_name, $record );
	} else {
	$out = $column_name;
	$out = apply_filters( "wp_stream_insert_column_default_{$column_title}", $out, $record, $column_name );
	foreach ( $inserted_columns as $column_key => $column_title ) {
	@@ -404,21 +405,20 @@ public function column_default( $item, $column_name ) {
	* Also, note that the action name must include the $column_title registered
	* with wp_stream_register_column_defaults
	*/
	if ( $column_title === $column_key ) {
	/**
	* Allows for the addition of content under a specified column.
	*
	* @param string $out Column content.
	* @param object $record Record with row content.
	* @param string $column_name Column name.
	*
	* @return string
	*/
	$out = apply_filters( "wp_stream_insert_column_default_{$column_key}", $out, $record, $column_name );