Apply filter to query args Fix #355

[faishal]

Add stream_query_args filter with callback WP_Stream_Settings::remove_excluded_record_filter will fix issue.

[westonruter]

@faishal it might be better to use array_fill here:

$record__in_format = '(' . join( ',' array_fill( 0, count( $record__in ), '%d' ) ) . ')';

[faishal]

@westonruter Yes it will be better, I will do that changes.

[fjarrett]

@faishal You make an excellent point about applying the filter by default, and I think it's a great idea.

What do you think about adding a hide_excluded argument to the query? And making it true by default.

Then it can be overridden easily like:

stream_query( array( 'hide_excluded' => false ) );

[faishal]

@fjarrett That will be good idea. /five

[faishal]

All suggested changes are done

[fjarrett]

@faishal This is causing some warnings to appear for me.

Warning: array_fill(): Number of elements must be positive in /srv/www/wordpress-trunk/wp-content/plugins/stream/includes/query.php on line 187
Warning: join(): Invalid arguments passed in /srv/www/wordpress-trunk/wp-content/plugins/stream/includes/query.php on line 187

[faishal]

@fjarrett sorry my bad ! Fixed by checking empty array before use array_fill

[fjarrett]

@faishal While testing this, I've been running into some problems with other Stream extensions, namely our Data Exporter project. /cc @lukecarbis

Before we merge this I think we will be forced to expand the scope of this ticket even more and introduce an option for this, which will force users to opt-in to hiding previous records.

Then the default for any Stream query will be whatever this setting is.

What do you think?

[faishal]

@fjarrett Yeah, That will be really good option for user to choose whether they want to hide previous records or not. /five

[lukecarbis]

Silly questions:

What if I want to hide all evidence of Frankie Jarrett's existence by hiding his previous records, but I also want to stop recording changes from the Comments connector without hiding previous entries?

If I have not chosen to hide previous records, how do I know when the current exclude settings took effect?

[fjarrett]

@lukecarbis @faishal

1. You're right, this could be a possible scenario, however, I don't see it being a common use-case. When this type of granular control is desired I think it would be best to allow someone to do it with a custom hook. Or another alternative could be creating a totally separate extension that would handle fine-grained exclusion/visibility control.

2. You'll know they took effect because new records won't appear for those actions.

But the general idea of leaving the Visibility choice up to the user will help them accomplish 1 of 2 objectives:

1. Exclude this type of activity from now on.
2. Exclude this type of activity from now on and hide all previous mention of this activity.

[faishal]

@fjarrett done

[lukecarbis]

I noticed that the Stream filters don't respect the exclude settings.

For example, I set the author exclude to Luke Carbis and set Hide Previous Records to true. Then I view the Stream page (which does not show any of my activity). However, when I view the Authors filter dropdown, I can select the author Luke Carbis. When I filter using this setting, I can see all my activity.

Is this the intended behavior?

[fjarrett]

@lukecarbis Good catch! If the visibility is set to Hide Previous Records then I think excluded items should be omitted from the dropdown filters entirely, as originally discussed in #223

/cc @faishal

[lukecarbis]

@fjarrett @faishal Not only excluded from the dropdown filters, but also excluded from results (because I suppose it's possible to manually type an excluded user ID as a $_GET parameter).

[faishal]

@lukecarbis realy good catch. I am working on it.