Skip to:
Content

Opened 3 years ago

Last modified 4 weeks ago

#3460 new enhancement

Support custom post types in activity stream

Reported by: boonebgorges Owned by: boonebgorges
Milestone: 2.1 Priority: low
Severity: normal Version: 1.2.8
Component: Activity Keywords: 2nd-opinion has-patch needs-testing
Cc: trisha@…, karmatosed@…, hugoashmore@…, math.viet@…, sascha@…

Description

Right now, custom post types are not tracked in the activity stream - we do a check to make sure that 'post' == $post->post_type before posting activity items.

This is fine for the blogs component, but we should have a more general solution for post types.

I suggest a solution with two parts:

  • default support for all CPTs. That is, we hook to save_post and catch everything that is *not* 'post', and we do it regardless of whether the BP Blogs component is enabled. We can get the singular term for the post type, which we'll need to guess at the "Boone edited the x..." activity action, out of the post type labels.
  • Have an easy way to turn off support for activity tracking of post types. Though generally I know we like to avoid admin screens, I think that an all-purpose Activity admin panel would be useful in allowing admins to toggle activity tracking on *any* kind of activity. The list of activity types would be populated by all of our core activity types (activity_update, joined_group, etc), and also by the create/edit actions of all registered post types.

At the very least, we will need filters to allow the latter to happen with plugins. But I think that this would be a popular enough feature (going by the 80% rule) that we should consider an options panel.

Probably we would also want to include support in the activity filter dropdown for the create/edit actions for registered post type that has activity posting disabled.

Once we get into the next dev cycle, I can work up a rough sketch of what I'm suggesting, in the form of a plugin. (at least the CPT stuff, anyway) I'd love to get feedback first, though.

Attachments (17)

activity screen.png (69.9 KB) - added by trishasalas 11 months ago.
cpt-settings.png (99.6 KB) - added by karmatosed 11 months ago.
cpt-settings-redux.png (117.8 KB) - added by Prometheus Fire 11 months ago.
custom post type settings with text option
cpt-settings-reworkone.jpg (200.8 KB) - added by karmatosed 11 months ago.
cpt-settings-reworktwo.jpg (207.6 KB) - added by karmatosed 11 months ago.
3460.png (66.5 KB) - added by boonebgorges 11 months ago.
cpt-settings-reworkthree.png (221.0 KB) - added by karmatosed 11 months ago.
3460.2.png (97.9 KB) - added by karmatosed 11 months ago.
cpt-added-to-bp-activity.diff (2.9 KB) - added by trishasalas 11 months ago.
3460.diff (12.7 KB) - added by imath 5 months ago.
3460.2.diff (18.5 KB) - added by imath 5 months ago.
3460.3.diff (25.2 KB) - added by imath 5 months ago.
blogs-settings-section.png (42.0 KB) - added by imath 5 months ago.
3460.04.diff (30.5 KB) - added by imath 7 weeks ago.
3460.05.patch (42.9 KB) - added by imath 7 weeks ago.
cpt recco.jpeg (21.4 KB) - added by Prometheus Fire 7 weeks ago.
UI for CPT on multisite subsites
3460.06.diff (39.3 KB) - added by imath 5 weeks ago.

Download all attachments as: .zip

Change History (90)

comment:1 anointed3 years ago

Sbrajesh provided me some sample code in order to integrate my custom post-types into the activity stream, along with their comments.

*He may remember building the 'sermons' activity snippet for bp for me last year, which I now use on multiple types of post types, throughout many sites.

While it works it is not as elegant as a full api, which I would love to see more info on.

I'm mostly posting this reply to request that in the mean time, comments are NOT turned off for the activity stream when they come from a custom post-type. I posted the request in the related ticket, but as it's closed, I'm not sure anyone would see it.

This would be a HUGE regression for all of my sites.

comment:2 boonebgorges2 years ago

  • Keywords 1.7-early added
  • Milestone changed from 1.6 to Future Release

I still want to do this, but I want to do it right so I'm punting.

comment:3 boonebgorges21 months ago

  • Keywords 1.7-early removed
  • Milestone changed from Future Release to 1.7
  • Priority changed from normal to low

I think this will be easy to do, so I'm putting in the 1.7 milestone, but I think it's also appropriate for a plugin, so I'm leaving open the possibility of punting :)

comment:4 johnjamesjacoby17 months ago

  • Milestone changed from 1.7 to 1.8

No patch. Punting to 1.8.

comment:5 trishasalas11 months ago

  • Cc trisha@… added

I've been looking at this recently since I have a client who could benefit from it. (and I think I can help with it)

The only problem with default for all cpt's is that in some themes/plugins you have additional cpt's that you may not want to show up in the activity stream (sliders, etc). I have a mockup I'm including that isn't ideal but could maybe start a discussion about how to handle this situation.

I can see that the code to pull the data in to populate this would be kind of clunky (maybe the plugin idea is what is needed here?) As someone who also manages a site, to have the all or none option would be frustrating to me and I'd go to hacking code anyway.

Last edited 11 months ago by trishasalas (previous) (diff)

comment:6 boonebgorges11 months ago

The only problem with default for all cpt's is that in some themes/plugins you have additional cpt's that you may not want to show up in the activity stream (sliders, etc)

Yeah. I initially suggested that all CPTs would be turned on by default, but I'm willing to be persuaded that the default should be *off*.

to have the all or none option would be frustrating to me and I'd go to hacking code anyway.

Yes, this is good feedback. I think we should either: (a) flesh out our existing APIs just enough to make this easy to do in a plugin (probably just adding another hook or two), or (b) build a full-fledged admin panel where site admins can fiddle with the settings. I don't think we should do anything in between, because for most sites, "all or none" will do more harm than good.

comment:7 karmatosed11 months ago

  • Cc karmatosed@… added

comment:8 karmatosed11 months ago

I'm not sold this interface shown in activity screen.png is needed it feels over complicated and a half measure. That section is now rarely used by users so it feels like putting something down back of sofa because can't handle it better. I also think the UI suggested is a bit confused:

  1. Should the comments be per CPT?
  2. What happens with 10... even more CPT? That seems a clunky UI for that many.

Thinking around the idea it could be any of these options:

  1. If CPT a tab opens for a full fledges admin panel setting - no half measure stuck below activity have it as it's own tab for CPT. This seems to then allow a better UI with easier picking.
  2. A function flag you can turn on / off in theme post types - bit like post types
  3. It's safe to assume if you are using a plugin/theme that has a CPT you want it on. Therefore having all on seems to make sense. Perhaps turning off is done in theme or plugin?

I personally don't think most users would be frustrated by all on.

comment:9 boonebgorges11 months ago

Ha, I didn't see that trishasalas had attached a mockup before submitting my comment :)

I disagree with karmatosed that it's "over complicated and a half measure". I think it's pared down about as minimal as it can get - at least, the first part of it is. I'm not sure I understand what the "comments" bit means, though - does toggling that option mean that comments on the CPTs will be put into the activity stream? I'm on the fence about this. I don't think we should offer per-CPT toggles for posts but not for their comments.

I'm going to push back against karmatosed on not liking the activity settings section. It's true that the section is rarely used by users, but this setting will also be rarely used by users, and to my mind it makes good sense to put this kind of setting there. As for her alternative suggestions:

If CPT a tab opens for a full fledges admin panel setting

This is problematic in a couple ways. First, it introduces a large amount of bloat. An installation could have a half-dozen or more CPTs provided by various plugins. A tab for each, with nothing but a single toggle, seems like a waste of space. More importantly, many plugins that register CPTs already have their own settings screens, and it seems more confusing for us to create another screen than to put it in a list like trishasalas suggests.

A function flag you can turn on / off in theme post types - bit like post types

Yes, I imagine we will have a code-level way to turn things on or off regardless of the UI. What I'm suggesting is that a UI is also necessary if we're going to implement the feature.

It's safe to assume if you are using a plugin/theme that has a CPT you want it on.

I disagree. There are many plugins that use CPTs for storing utility-style data, in which case you definitely don't want it on. We could look at something like the 'public' setting for the post type to make some guesses about this, I suppose. More importantly, many plugins already support the BP activity stream natively. If we turn everything on by default, then there will be double posts for those CPTs. So, while it's possible that the feature will be missed by some if we default to "off", it's likely that we will screw up some sites in a more serious way (either through double posts or through irrelevant data posted to the stream) if defaulted to on. It's a decision that's going to have to be made independently on every BP installation, which is why I think a UI is non-negotiable.

Actually, the more I talk about it, the more I wonder whether there's going to be an elegant way for us to handle this in core at all. But I encourage you all to prove me wrong through continuing this helpful discussion :)

comment:10 karmatosed11 months ago

@boonebgorges: lol fair dos thought it was that you'd seen mockup.

I've done a response png https://buddypress.trac.wordpress.org/attachment/ticket/3460/cpt-settings.png. I think it refines the idea a bit and works for multiple types. I'm not sold on it as the right place so much but happy to be wrong if it's useful great. Ultimately, if it works and people can find it then yay.

I feel (my own solutions included) it's not as clever as could be. I also am apparently not being clever as have no better suggestion :) It certainly needs pondering and kicking around a bit.

Last edited 11 months ago by karmatosed (previous) (diff)

karmatosed11 months ago

comment:11 trishasalas11 months ago

The comments section at the bottom was my "Oh yeah, need to think about comments" moment. Not very well thought out and it shows :) I like @karmatosed's solution is this particular instance.

I'd like to work on this from the code side first and then see if we can come up with that 'clever' that @karmatosed mentions. I feel pretty strongly that it should be in core (if at all possible) as cpt's are used so often and in such an integral fashion for many sites.

@boonegorges is bp-blogs-functions.php the only place where post type is checked?

If you think this is a valid direction, I'll move ahead with a the Admin panel option keeping in mind the possibility of a plugins if this doesn't flesh out well.

comment:12 trishasalas11 months ago

First step

// Don't record this if it's not a post
	if ( in_array( $post->post_type, apply_filters( 'bp_blogs_record_all_post_types', array( 'page', 'revision', 'nav_menu_item', 'attachment' ) ) ) )
		return false;

This allows all with the exception of WordPress default post_types. Tested on my dev blog and it works for just the one cpt i tried. I also added a page and it was filtered out. No surprise as it's fairly straight-forward code.

@boonegorges, if this isn't a good approach or if there is a better approach please let me know.

My next step is to add 'wrote a new cpt' in the stream rather than the default 'post'. After that I'll tackle the admin...and try to make it 'clever'. I'll admit, the only idea I've got right now is check boxes...but I'm thinking :)

http://trishacod.es/activity/

comment:13 boonebgorges11 months ago

This allows all with the exception of WordPress default post_types.

Yes, this is a good starting point, though the name of your filter sounds like the opposite of what it does (it's a blacklist, not a whitelist).

When you start working with the "wrote a new..." language, be sure to check out the 'labels' that are stored in the post type object, eg get_post_type_object( 'page' ). I think you might be able to put something together using those labels.

Prometheus Fire11 months ago

custom post type settings with text option

comment:14 Prometheus Fire11 months ago

I admit that I breathed a sigh of relief when I saw that this ticket was being discussed again. This is something I've been struggling with for two years now.

I like UI concepts and am going to push for @karmatosed's version of the UI, simply because it feels more flexible. I added an attachment which modified her's a little bit to accommodate the input of a custom text string for the activity record. The argument being, if you are going to allow CPTs in the activity stream, being able to define the text is an valuable part of it. I currently use 4 CPTs in my activity stream and I found that the most important aspect of it when setting it up was writing custom text. From a UI point of view, perhaps the Allow comments and Text input stuff could be greyed out until Show in activity is selected.

Additionally, some of the feedback I've received from my users about the impact my CPTs have had on the activity stream is that I have no way to filter them (it's just something I've never worked out how to do). They've reported that it clutters the activity stream and makes it hard to use and understand. As this ticket moves forward, I think its important to remember that when a CPT is added to the stream, it is also added to the dropdown filter so that they can be sorted out from the rest of the stream records. Similar to how BBPress topics and replies show up there. Since I've never been able to figure out how to do this, CPTs are lumped in with blog posts and cannot be filtered on their own.

comment:15 karmatosed11 months ago

I'm not really sold on that UI it feels like adding clunk with text and an input box - click to edit is a fairly standard UI now we can utilise that for ease and less clunky :) I'd see the case probably as being one where you have the default and that becomes the class name with underscores of course.

There are 2 places you'd want to customise the text: 1 where it outputs with 'x did this custom post type' and the other being the unique class name on the li for styling.

We have 2 options to do this:

  1. Make the name of the custom post type as the default you click to edit if want to change display name - simplier method.

https://buddypress.trac.wordpress.org/attachment/ticket/3460/cpt-settings-reworkone.jpg

  1. Add an input box for label - do without text and have some descriptive text, still default to custom post type.

https://buddypress.trac.wordpress.org/attachment/ticket/3460/cpt-settings-reworktwo.jpg

I'd probably say 1. is the way forward.

comment:16 trishasalas11 months ago

The only additional thought I've had about UI is something "jquery-ui-ish" using accordion or something like that but I think that could be going overboard :) I'll leave the UI to @karmatosed. I do like sticking with existing UI, creates a cohesiveness that works.

On the textboxes...I don't think they will be needed. With @boonegorges direction I'm working on a way to display the cpt names in the activity stream. That will also spill over to being able to filter them with the dropdown as we do everything else.

comment:17 boonebgorges11 months ago

I'll leave the details of the implementation to the more UX-savvy, but I think that karmatosed's reworkone and reworktwo don't totally capture the issue that's being raised by Promethius Fire.

When CPTs are registered, they have a set of labels with them. For example:

register_post_type( 'idea', array(
    // ...
    'labels' => array(
        'name' => 'Ideas',
        'singular_name' => 'Idea',
        // ...
    ),
    // ....
) );

So, if all we are trying to do in terms of customization is to grab the name, we could just as easily do

$post_type = get_post_type_object( 'idea' );
$singular_name = $post_type->labels['singular_name'];

and then swap it into the skeleton action sentence like this:

$action = sprintf( '%s posted a new %s', $user_link, $singular_name );

In other words: we can already automate this. Adding a UI just to override singular_name seems like it will only address the edge case where the user doesn't like the singular_name that the CPT was registered with.

I (and, I think, Prometheus Fire) am more concerned with the case where you don't like the "posted a new" phrasing. Maybe, in the case of 'ideas', you want "x thought up a new idea". I have a feeling that this is going to be the type of customizations that site owners want to make. Unfortunately, allowing this kind of customization is much more complicated in terms of UX. See 3460.png.

My impression (take it for what it is) is that *either* we provide this level of customization, or we don't provide any customization at all (just the toggles). Doing something in between will create extra UX for very limited benefit.

boonebgorges11 months ago

comment:18 karmatosed11 months ago

https://buddypress.trac.wordpress.org/attachment/ticket/3460/cpt-settings-reworkthree.png is an attempt to combine the ideas. I'm probably in the overcomplicating camp though, if there is the ability to automate I feel that should be done and I'm not sure we're not creating extra UX for limited benefit. Although, it kind of works if we have it in this section.

comment:19 Prometheus Fire11 months ago

I think rework three is a good compromise. One of the things we need to remember that whole point of adding CPTs to the activity stream is to create a customized internet environment that caters to the niche (whatever that niche is).

@boonebgorges said: "I... am more concerned with the case where you don't like the "posted a new" phrasing. Maybe, in the case of 'ideas', you want "x thought up a new idea". I have a feeling that this is going to be the type of customizations that site owners want to make."

This is exactly true and is the whole focal point of using CPTs in the first place. Efforts on customization through the UX are not wasted and add to the value of this enhancement (and I think 3460.png and reworkthree are on the right path for it).

You might consider that to prevent over complicating the whole thing, if custom post types are not registered or detected, the interface itself does not display. That way, a novice BP site owner, who may not be interested in (or even know about) CPTs isn't going to be bothered, but as soon as one is registered, the interface appears and provides options.

Last edited 11 months ago by Prometheus Fire (previous) (diff)

comment:20 johnjamesjacoby11 months ago

Like the direction for this so far. Some thoughts:

  • Boone's method of assuming that some post_type settings like 'public' and 'show_ui' mean we should expose a UI for activity streams, makes a lot of sense.
  • Reads like we're putting the cart before the horse, mocking a UI for something that doesn't have an intended architecture yet.
  • What if we build a settings page for the existing activity stream actions first, so they can be toggled off and on. This will force us to take an inventory of our existing actions, their approaches, and allow us to build out the improvements we need to support custom post types as part of the native Activity Streams component, outside of the Blogs component (which is becoming increasingly less relevant as we trim things out of it.)
  • bp_activity_set_action() (and $bp->activity->actions) give us everything we need to create a UI for each of the registered actions. All we need is an option to toggle them on and off, and check for those options in bp_activity_add() to noop them if they're turned off.

comment:21 boonebgorges11 months ago

Reads like we're putting the cart before the horse, mocking a UI for something that doesn't have an intended architecture yet.

Maybe. We're definitely not at the point of deciding the specifics of the UI. However, I think this is a case where coming up with at least the sketch of a UI will help us get clear on what problems the feature is meant to solve, and that knowledge will drive some of the technical infrastructure. (Also, as you've mentioned, at least some of the infrastructure is already in place, such as $bp->activity->actions and related functions.)

which is becoming increasingly less relevant as we trim things out of it

I hope this is a side note about the long term and not a part of the scope of this ticket :) I for one am fond of the Blogs component :)

if we build a settings page for the existing activity stream actions first

bp_activity_set_action() (and $bp->activity->actions) give us everything we need to create a UI for each of the registered actions

Yes, though just to be clear, we only have "everything we need" for the *registered* actions, and the point of this ticket is explicitly for actions that *aren't* registered.

Those who are thinking about UX on this ticket: Could we see some additional mockups/wireframes with some of the expanded ideas JJJ raises, specifically toggles for existing activity actions? This would probably get its own panel. If we can all look at a shared set of mockups, maybe we can make a collective decision about how to strategize the short (1.8) and medium (1.9?) term goals.

karmatosed11 months ago

comment:22 trishasalas11 months ago

To be honest, I have envisioned this as having a separate settings panel from the beginning.

I like the idea of toggling all but do see the more immediate need of allowing cpt's in the activity stream. It is a point of frustration for many site owners. The proposed UI addresses just this component fairly elegantly, I think.

I'm wondering about having just this part (cpt's) for 1.8 and adding the full control panel for 1.9

comment:23 karmatosed11 months ago

I took liberty of responding to your wireframe Boone with one so we can use that thread on Balsamiq maybe? It's not saved so people can respond with proposed versions and promote their version as will.

https://buddypress.trac.wordpress.org/attachment/ticket/3460/3460.2.png is what I have so far. Of course, it's not got all options but should kick off the idea.

I used the settings style that WordPress uses for now as seems sensible.

Last edited 11 months ago by karmatosed (previous) (diff)

comment:24 trishasalas11 months ago

Success (all cpt's are showing in the activity stream with the appropriate label)

Here's the code I added/modified:

//Block all WordPress default post types with the exception of 'post'.  
	if ( in_array( $post->post_type, apply_filters( 'bp_blogs_block_wp_default_post_types', array( 'page', 'revision', 'nav_menu_item', 'attachment' ) ) ) )
		return false;

	$is_blog_public = apply_filters( 'bp_is_blog_public', (int)get_blog_option( $blog_id, 'blog_public' ) );

	if ( 'publish' == $post->post_status && empty( $post->post_password ) ) {
		if ( $is_blog_public || !is_multisite() ) {

			// Record this in activity streams
			$post_permalink = get_permalink( $post_id );

			// What is a post type?
			$post_type = get_post_type_object( $post->post_type );

			if ( is_multisite() )
				$activity_action  = sprintf( __( '%1$s wrote a new %4$s %2$s, on the site %3$s', 'buddypress' ), bp_core_get_userlink( (int) $post->post_author ), '<a href="' . $post_permalink . '">' . $post->post_title . '</a>', '<a href="' . get_blog_option( $blog_id, 'home' ) . '">' . get_blog_option( $blog_id, 'blogname' ) . '</a>', $post_type->labels->singular_name);

			else

				//Add the post type name to the activity sentence
				 $activity_action  = sprintf( __( '%1$s wrote a new %2$s, %3$s', 'buddypress' ), bp_core_get_userlink( (int) $post->post_author ), $post_type->labels->singular_name, '<a href="' . $post_permalink . '">' . $post->post_title . '</a>');

I am getting a seemingly unrelated error in debug in wp-includes/user.php lines 1065 - 1067. I suspect that's my sloppiness but I wanted to mention it.

Now on to the action sentence :)

comment:26 boonebgorges11 months ago

Yup, this is looking like a good start - that's just the technique I was thinking of for creating the action strings.

Are you familiar with using svn and creating diffs? It'll make it a bit easier that pasting right into Trac, and it'll make it much easier when it comes time to be sharing code between different folks. The codex's page is pretty good: http://codex.wordpress.org/Using_Subversion

comment:27 trishasalas11 months ago

Thanks, Boone. I've been using Versions to check the code out but hadn't made a .diff file yet. I found command line easier here :) Adding the same code as a .diff

Last edited 11 months ago by trishasalas (previous) (diff)

comment:28 trishasalas11 months ago

Here's my latest code update after talking to you this afternoon, Boone. (huge help, thanks). I'm going to look at this later because I have some errands to run but this looked important enough to post asap. See my code block for comments

<?php

// I Did this on bp-templates/bp-legacy/buddypress/activity/index.php on line 74

?>
<li id="activity-filter-select" class="last">
				<label for="activity-filter-by"><?php _e( 'Show:', 'buddypress' ); ?></label>
				<select id="activity-filter-by">

				<!-- I added this -->
					<?php bp_activity_filter_links() ?>

				<!-- /end I added this -->

				</select>
			</li>
<?php

// Then I Got This:  WARNING: wp-includes/wp-db.php:990 - Missing argument 2 for wpdb::prepare(), called in /var/www/vhosts/trishacod.es/httpdocs/wp-content/plugins/buddypress/bp-activity/bp-activity-classes.php on line 620 and defined
// This is line 620

return $wpdb->get_col( $wpdb->prepare( "SELECT DISTINCT component FROM {$bp->activity->table_name} ORDER BY component ASC" ));

// I googled and found this: http://make.wordpress.org/core/2012/12/12/php-warning-missing-argument-2-for-wpdb-prepare/ (looks important to me)

?>
Last edited 11 months ago by trishasalas (previous) (diff)

comment:29 boonebgorges11 months ago

In 7149:

Remove incorrect $wpdb->prepare() use in BP_Activity_Activity::get_recorded_components()

Also adds docblock

See #3460

comment:30 boonebgorges11 months ago

Thanks for the report, trishasalas. The $wpdb->prepare() notice is a small bug, which I just fixed in r7149. Carry on!

comment:31 trishasalas11 months ago

var_dump() for bp_activity_filter_links returns NULL but when I query the database directly and var_dump like so:

global $wpdb, $bp;

		$type = $wpdb->get_col("SELECT DISTINCT type FROM {$bp->activity->table_name} ORDER BY type ASC");
		print_r($type);

I get appropriate results. What am I doing wrong here? I've tried and read just about everything I can think of at the moment.

comment:32 boonebgorges11 months ago

What am I doing wrong here?

It's possible that you're not doing anything wrong, and that there's a bug. You'll have to debug bp_activity_filter_links() more closely. Note where that function calls BP_Activity_Activity::get_recorded_components(), and then trace to see what's going on in the rest of the function.

comment:33 hnla11 months ago

  • Cc hugoashmore@… added

comment:34 boonebgorges11 months ago

  • Milestone changed from 1.8 to 1.9

comment:35 trishasalas10 months ago

I have the custom post types added to the Activity Filter dropdown menu. The original menu items are currently still hard-coded. I believe that will require a separate function and will work on that as well. I am posting now as I have the items I want to exclude from type hard coded into the sql query. I tried a regex and NOT LIKE but wasn't able to get close enough...thought someone might have a more elegant solution.

Here is my function:

function bp_get_activity_filter_links( $args = false ) {
		global $wpdb;
		global $bp;

		// Fetch the names of components that have activity recorded in the DB
		$add_cpts = $wpdb->get_col( "SELECT DISTINCT type FROM {$bp->activity->table_name} WHERE type != 'activity_update'
	AND `type` != 'new_blog_post' AND `type` != 'joined_group' AND `type` != 'Post'", 0); 

		if ( empty( $add_cpts ) )
			return false;

		foreach ( $add_cpts as $add_cpt ) {

			echo '<option value=' . $add_cpt . '>' . $add_cpt . '</option>';

	        }
	}

You can see a working example here: http://trishacod.es/activity/

comment:36 trishasalas8 months ago

I actually, shocking have this working...with one caveat that I hope isn't huge. Because of the way bbPress is hooked in I'm getting topics and replies recorded twice in the database. I'm hoping one of you guys can help me filter properly.

Next steps for me are to create what is needed to make the rest of the option values dynamic on bp-templates>bp-legacy>buddypress>activity>index.php. After that I'll work on the admin settings screen.

That brings me to an important question, should I/we put this on hold and help @karmatosed finish the template pack with a bang and save this for 2.0? I'd really like to see that templatepack be a star for 1.9

@boonegorges I'm going to upload the diff tomorrow (Sunday)...still not quick as I'd like to be with svn and command line. I really only made changes to the blogs-functions.php page and the activity index page in the templates.

comment:37 trishasalas8 months ago

  • Keywords dev-feedback added

comment:38 trishasalas8 months ago

  • Keywords dev-feedback removed

comment:39 boonebgorges8 months ago

Looking forward to seeing what you've got, trishasalas!

should I/we put this on hold and help @karmatosed finish the template pack with a bang and save this for 2.0?

Yes, I think this ticket is secondary in importance for 1.9. Thanks for all your work!

comment:40 boonebgorges5 months ago

  • Milestone changed from 1.9 to Future Release

Clearing the milestone. Let's see where we can get with this one next time around.

comment:41 trishasalas5 months ago

Picking this up again. I'd really like to have at least part of this done for 2.0. Can we discuss minimum needed to put into core?

comment:42 imath5 months ago

Hi,

I've tried to build a patch. I should have read the thread before following blindly my idea! So this patch won't reply to the need to "catch" all post types registered. When building it i had in mind, plugin authors that wish to use the BuddyPress Activity component.
When registering a custom post type, we define various args for the function register_post_type(). So my idea is to extend in a way these args by letting plugins define a "bp_tracking" argument and a new value to the "supports" arg ("bp_tracking"). For instance, the patch could be used this way in a plugin:

$labels = array(
	'name' => __( 'Post type name', 'plugin_domain' ),
	/** etc... **/
);

$tracking_args = array( 
	'type_post'                           => 'new_blog_post', // post activity type
	'activity_permalink_post_callback'    => 'callback_function_to_build_permalink_for_post',
	'activity_action_post_callback'       => 'callback_function_to_build_action_for_post',
	'type_comment'                        => 'new_blog_comment', // comment activity type
	'activity_action_comment_callback'    => 'callback_function_to_build_action_for_comment',
);

$args_file = array(
	'label'	            => __( 'Post type label', 'plugin_domain' ),
	'labels'            => $labels_file,
	'supports'          => array( 'title', 'editor', 'author', 'bp_tracking' ),
	'bp_tracking'       => $tracking_args
);

The first thing the patch is doing is to extend the post_type "post" to include these arguments, i guess it can be done with other post_types.. Then i've modified the two functions bp_blogs_record_post() and bp_blogs_record_comment() so that they can play with these new arguments..

I hope the diff 3460.diff attached will help you to find the best solution.

imath5 months ago

comment:43 follow-up: trishasalas5 months ago

@imath, I really like your solution, is there a way to account for existing post types with this method do you think?

The first part of the patch I submitted will allow all post types through but doesn't address comments at all. I've been working on creating the labels we need for filtering but I'm wondering now if this might be a better solution?

comment:44 in reply to: ↑ 43 imath5 months ago

  • Keywords has-patch 2nd-opinion added

Replying to trishasalas:

@imath, I really like your solution, is there a way to account for existing post types with this method do you think?

Thanks for your feedback and yes there is. If you look at the function bp_blogs_extend_initial_post_types() we could loop in the different post types to add a support for "bp_tracking". But it would need to first deal with the post_type "post" to have a default behavior to rely on for the bp_tracking array of callback action, and then simply add support for the others (except for the attachment post type maybe..).

But i really think, it's not a so good idea. Because, it will risk to reverse the logic ! Some users might be unsatisfied that every post type is now tracked into the activity stream...

I've edited the patch for 3460.2.diff. I've added to it the work you've done with singular names, so now it's really easy to add a support to the site tracking component for a post type. You simply need to add a few characters in the functions.php of your theme, let's take the post type page as an example :
add_post_type_support( 'page', 'bp_tracking' );

And every page you will post or comment on will be tracked.

I've also modified the activity filter options, so that when a post type has a "bp_tracking" support, then the different dropdowns on front or in back end have the post_type filter options.

Finally, i've adapted the disable activity comment feature that we can set in BuddyPress settings to also apply it to post types that has a support for bp_tracking.

What everyone is thinking about it ?

imath5 months ago

comment:45 follow-up: trishasalas5 months ago

@imath, this is great! I do have to say though I'm on the fence about the lack of ui. Adding the code to functions.php is intuitive for most developers but will we 'lose' some who don't understand? (Maybe I could write a Codex doc about this if we decide to implement in this way ;)

Although, @karmatosed's points above about the settings page are valid as well, since it is mainly developers who will look for that settings page is it needed if we have this (very easy to implement) bit of code for the theme functions file.

This method also nicely addresses the tracking of the cpt's that a developer would like to track without the worry of grabbing unnecessary/unwanted cpt's.

There were a few notices/errors in the code that I didn't have time to look closely at:
Trying to get property of non-object on lines 44 and 45 of wp-content/plugins/buddypress/bp-blogs/bp-blogs-activity.php

And then after I registered post types: undefined index: "post type name" (same location as above)

comment:46 in reply to: ↑ 45 imath5 months ago

  • Keywords has-patch removed

Replying to trishasalas:

There were a few notices/errors in the code that I didn't have time to look closely at:
Trying to get property of non-object on lines 44 and 45 of wp-content/plugins/buddypress/bp-blogs/bp-blogs-activity.php

And then after I registered post types: undefined index: "post type name" (same location as above)

That's problematic. I haven't noticed it because i've tested with the "page" post type and my "buddydrive_file" post type. The buddydrive one is hooking init at a 9 priority (i've noticed using 10 the taxonomy wasn't available if using ajax). I've just check with the default priority (10) which most will use, and you're right about the errors, the post type arguments are not defined :(

I need to keep exploring ;)

comment:47 imath5 months ago

I really need to explore more, i've made it work by modifying the bp-core-actions this way :
add_action( 'init', 'bp_init', 11 );
bp_init hooking init later, which frightens me a bit, i don't know if it could cause regressions elsewhere :( ..

Moreover i wonder what's happening in a network, for instance if a custom post type is set on a child blog but not on the network.. For sure the activity filter for that particular post type won't be available :(

It really needs a deeper exploration!! But i'll be back ;)

comment:48 follow-up: bartom345 months ago

@iMath: I want to integrate a custom post type called "beer" in my activity stream.

Do you advice me to follow your 3460.2.diff patch integrate it ?
Do I edit the core file of BP ?

Thanks for your help,
Tom

comment:49 trishasalas5 months ago

Hi Tom,

I hope you don't mind if I chime in here. We're getting close, but the diff you mentioned still has a few minor bugs to work out. I wouldn't edit core files just yet, instead check this link out and see if it helps: http://buddypress.org/support/topic/getting-custom-post-types-to-record-activity/

For future reference, you'll probably get a quicker response over in the forums as they are monitored more regularly than trac tickets for requests like this (http://buddypress.org/support/) I just happened to have my email opened and saw this come in :)

Best,
Trisha

comment:50 in reply to: ↑ 48 imath5 months ago

Replying to bartom34:

@iMath: I want to integrate a custom post type called "beer" in my activity stream.

Do you advice me to follow your 3460.2.diff patch integrate it ?

Hi @bartom34,

@trishasalas is right. If your goal is to use this diff on a live website. I strongly advise you to avoid using it, because
1/ It's still work in progress
2/ It may not be the best solution

If your goal is to test this on a local dev site in order to contribute to the subject, then you can try it. But it will only work for built in WordPress post type such as the page one or post types that are hooking init at a priority under 10. To make sure all post type are "catched", you will need to edit the core file bp-core-actions.php to change the priority of the bp_init hook for this one :
add_action( 'init', 'bp_init', 11 );
But doing so, checking that changing this priority has no impact on all the BuddyPress features is needed.

Again don't use it on a live website and follow @trishasalas advices

comment:51 imath5 months ago

  • Cc math.viet@… added
  • Keywords has-patch added

Hi,

In France we often say "Jamais 2 sans 3", i guess it applies to this patch! So this is my third suggestion of patch for this feature.
I've built this version of the patch in WordPress 3.8-beta1 / BuddyPress 1.9-beta1, multisite on.

1/ I've added a new hook to bp-core/bp-core-actions.php :
add_action( 'init', 'bp_catch_post_types', 100 ); very late to maximize the post type catching and i thought it was a safest way than modifying the priority of bp_init

2/ I've created a new section settings in BuddyPress settings Administration screen
you'll find a screenshot in the attached files (blogs-settings-section.png). This new section allows the community administrator to activate the tracking for the different post types that have been catched.

  • if on a multisite config :
    • the post type is in a plugin activated on the network : then it can be activated from the BuddyPress Site Tracking settings and all the post types will be tracked for the root and the child blogs.
    • the post type is only activated on any of the child blogs but not on the root blog : it cannot be activated in the BuddyPress Site Tracking settings, so it won't be tracked
    • the post type is activated on the root blog : then it can be activated from the BuddyPress Site Tracking settings and all the post types will be tracked for the root and if the same plugin has been activated in any of the child blogs, then they will also be tracked.
  • if on a regular config :
    • All the post types activated from the BuddyPress site tracking will be tracked.

To avoid the attachment post type, i'm using get_post_types( array( 'show_in_nav_menus' => true ), 'objects' ), it means that if the argument 'show_in_nav_menus' for the post type is set to false, it won't be tracked. This argument if not defined when registering a post type defaults to the value of the 'public' argument. Avoiding the attachment post type can be done in another way, but i think it's good then to use something like get_post_types( array( 'public' => true ), 'objects' )...

3/ i've added a filter to let a child blog eventually remove a trackable post type for its particular case :
apply_filters( 'bp_blogs_post_type_trackable', bp_get_option( 'bp-enabled-post-types', array() ), $blog_id )

Finally the community administrator decides what post types can be tracked, plugins cannot force using the 'bp_tracking' support. But plugins can still include arguments to override the default way of formatting activities. For instance a plugin can include in its post type arguments the "bp_tracking" array :

	$args_post_type = array(
		'label'	            => __( 'Random' ),
		'labels'            => array( /*the different labels */ ),
		'public'            => true,
		'bp_tracking'       => array( 
			'type_post'                           => 'new_random',
			'activity_permalink_post_callback'    => 'random_activity_permalink_callback',
			'activity_action_post_callback'       => 'random_activity_action_random_callback',
			'type_comment'                        => 'new_random_comment',
			'activity_action_comment_callback'    => 'random_activity_action_comment_callback',
		),
		'supports'          => array( 'title', 'editor', 'author' ),
	);

imath5 months ago

comment:52 follow-up: Prometheus Fire3 months ago

@imath - maybe you could rethink this:

the post type is only activated on any of the child blogs but not on the root blog : it cannot be activated in the BuddyPress Site Tracking settings, so it won't be tracked

In multisite, I've encountered several use cases where I use a CPT on a subsite, but not on the main site. This is often the case in niche networks. I've built out subsites with their own set of CPTs but to draw people to the subsite, I needed them tracked by the activity stream on the mainsite. I think the ability to track all CPTs on the network would be a key part of this feature. Having on/off switches for all available CPTs I think is important for this.

comment:53 in reply to: ↑ 52 imath3 months ago

Replying to Prometheus Fire:

Hi Prometheus Fire, in fact i've described what the patch i suggested was doing. I'm sure it can be improved. I'll rethink about a way to track CPT that has been set in a child blog but not on the root one.
I wonder if we should add a BuddyPress settings section in child blogs to ask for the blog's administrator if he wants to track the CPT. This way it will be easier to get all post types and there may be cases when a child blog administrator doesn't want a specific post type to be tracked even if he has set his search engine settings to true.

Thank you for your suggestion.

comment:54 ircbot3 months ago

This ticket was mentioned in IRC in #buddypress-dev by boonebgorges. View the logs.

comment:55 ircbot8 weeks ago

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.

imath7 weeks ago

comment:56 imath7 weeks ago

3460.04.diff is refreshed to latest trunk.

I improved a few things comparing to 3460.03.diff, taking care of backcompat in case plugins / themes were using the filters :

  • bp_blogs_record_post_post_types
  • bp_blogs_record_comment_post_types

The way it works hasn't changed from 3460.03.diff, tracked post types can be by default :

  • the post types created by plugins that are network activated
  • the post types that are set on root blog
  • the post types that are in common between root blog and child blogs

About the post types that are only "active" on child blogs, it's possible to make them trackable by using some filters in bp-custom.php, for instance :

// to save an activity when a 'my_custom_post_type' is posted
function child_blog_only_post_type( $tracked_post_type = array() ) {
	if ( ! in_array( 'my_custom_post_type', $tracked_post_type ) )
		$tracked_post_type['my_custom_post_type'] = 1;

	return $tracked_post_type;
}
add_filter( 'bp_blogs_is_blog_post_type_trackable', 'child_blog_only_post_type', 10, 1 );

// to add a new action in filter selectboxes of the activity directory and of the member profile page
function test_de_filtre() {
	bp_activity_set_action( buddypress()->blogs->id, 'new_blog_my_custom_post_type', sprintf( __( 'New %s posted', 'buddypress' ), 'my Custom post type' ) );
}

add_action( 'bp_blogs_register_activity_actions', 'child_blog_activity_action' );

There's a way to make things simplier than using these filters for the child blog only post types. We can create a new Administration UI that would only load on child blogs to create checkboxes for each post type the same way bp_admin_setting_callback_post_types_tracked() is building the checkbox list for the "network active" post types. Then we could "merge" the child blog settings with network one into the bp_blogs_is_blog_post_type_trackable() function as one of its argument is the blog id.

Building a UI into child blogs just for this purpose might not worth it... On the other hand, it could be a way to have an allow/disallow feature per post type / per blog. For instance if the page post type is active on the root blog, a child blog could deactivate the page of his blog... Then, we would have to think about who can access to this UI : regular admin or superadmin only ?

I think the patch is ready to be tested.

imath7 weeks ago

comment:57 imath7 weeks ago

3460.05.patch is adding a new admin UI in each subsite in case of a network. This is a screenshot of this UI :

http://farm8.staticflickr.com/7432/12836714093_9709eec427_z.jpg

Thanks to it :
The subsite administrator can eventually deactivate a post type that is active on the network or activate one that is not available on the network.

The really tricky part is for populating the action filters in activity directory and member activity page. I've tried to think about all possibilities but one will probably need a repare tool (i need to test #5296) :)

If a post type is subsite only, and activated in the subsite: it's tracked and the activity actions is added to the selectboxes. If the site administrator change his mind and decide to untrack the post type from his subsite, the post type is no more tracked and the activity actions is no more in the selectbox (except if the same post type is still tracked by another subsite).

But if the administrator uses the "discourage search engine" option before untracking his post type, then there's a chance the selectbox keeps on displaying the action for the untracked post type.

So if we are to do this, from my point of view it can be interesting to have a repare tool to check if the post type is no more in use in any subsite to eventually remove it from the activity selectboxes.

Another approach would be to consider in the case of "specific to subsite" post type, we're not taking care of creating an activity filter..

Prometheus Fire7 weeks ago

UI for CPT on multisite subsites

comment:58 Prometheus Fire7 weeks ago

I was thinking that adding any UI to the subsites doesn't mesh well with the philosophy of keeping the UI to a minimum. I do think your idea of adding a snippet to bp-custom.php is sufficient, provided the documentation for this is well-written and easily available in the codex. This is a great way to handle it, it just needs good documentation on the hows and whys of it.

However, if a UI is added, why not add it to the BP admin area in the network admin. Shouldn't this be controlled by the super admin anyhow? I do think it should respect the search engines discouraged setting though.

I do see the merit in having the control being done by the subsite admin as opposed to the super admin in the network settings. I am beginning to feel that this is a decisions vs options issue.

Last edited 7 weeks ago by Prometheus Fire (previous) (diff)

comment:59 follow-up: boonebgorges7 weeks ago

See also #3856, which is (I think) a blocker for this one.

comment:60 in reply to: ↑ 59 imath7 weeks ago

Replying to boonebgorges:

See also #3856, which is (I think) a blocker for this one.

I'm afraid, i must agree :(

So if callback functions are to be used, i agree it's no more possible to "catch" post types as it would be almost impossible (very very hard!) to get the domain of plugin/theme that creates the post type. (Too bad i've just thought of using transient in conjonction with a hook on registered_post_type)

Although if the post type arguments were including a bp_tracking one (like i'm introducing in the patch) with the callback functions, then it would be possible. But most post types won't.. ahrf too bad.

comment:61 follow-up: boonebgorges7 weeks ago

So if callback functions are to be used, i agree it's no more possible to "catch" post types as it would be almost impossible (very very hard!) to get the domain of plugin/theme that creates the post type. (Too bad i've just thought of using transient in conjonction with a hook on registered_post_type)

I should be clear that I don't think #3856 will make this ticket impossible. Only that I think that #3856 should be done before we come up with final solutions here. I'm thinking that, for custom post types, we can provide a generic fallback callback function. Something like:

function bp_activity_format_activity_action_generic_new_post( $activity ) {
    $post = get_post( $activity->secondary_item_id );
    $label => $wp_post_types[ $post->post_type ]->labels->singular_name; 
    //...
    return sprintf( __( '%s posted a new %s', 'buddypress' ), $user_link, $label );
}

This is similar to what you've already set up in your patch. If by "domain" you mean "textdomain", I don't think we need to worry about getting it out of the plugin. We will use our own textdomain for the default string. Plugin authors will be able to override it with their own string by registering their own callback (ie by not using ours).

So, I think the direction that we need to move in is providing an API for plugin authors to opt into tracking by BuddyPress. They'll be able to opt in minimally, and we can provide our fallback activity action strings. Or they can opt in and override some of our fallbacks for a more customized experience.

imath, I'm planning to dig deeper into your patch very soon. I think that much of it is on the right track and can be used. But I think that the critical bit will be to establish easier ways for plugin authors to announce to BP that they want to be included in the activity stream, and this is something that should probably wait until #3856 has been decided on.

comment:62 in reply to: ↑ 61 imath7 weeks ago

Replying to boonebgorges:

I should be clear that I don't think #3856 will make this ticket impossible. Only that I think that #3856 should be done before we come up with final solutions here.

oops, my fault, sorry lost in translation : i really should attend some english classes ;) I've seen that you posted a patch on #3856, so i renew my thanks. I've tested it, it's really an interesting improvement for translations.

imath, I'm planning to dig deeper into your patch very soon. I think that much of it is on the right track and can be used. But I think that the critical bit will be to establish easier ways for plugin authors to announce to BP that they want to be included in the activity stream, and this is something that should probably wait until #3856 has been decided on.

I agree, i think testing the #3856 patch:
1) BuddyPress plugin developer using custom post types will probably use bp_activity_set_action() to set their callbacks
2) "random" custom post types will not.
So my idea of using a post type argument to set the callbacks function might not be so great or incomplete. Because post types created in 1) will probably have their own ways to generate activities, so in this case we should give them a way to avoid having 2 activities posted : 1 by their plugin, the other by the blogs component. We could rely on the 'show_in_nav_menus' argument but i'm not sure plugins are "hiding" their content from being added to nav menus.

Then taking in consideration Promotheus Fire comments:
Custom post types created on child blogs are not visible when in root_blog_id, so i thought in 3460.05.patch that it could be interesting to let the choice to subsites admin to set their preferences and progressively create a "bp option" that would include all the post types activated by blogs. I realize:

  • it can be annoying for super administrator to give that "power" to subsite administrator
  • it creates an UI only for that into the subsite administrations.

On the other hand:

  • this UI made it possible to create a specific filter in the template selectbox for the custom post type, and when a blog was deactivating the custom post type, then the "bp option" was updated or deleted and filters were no more showing...
  • Super admin could review the subsite setting...
  • in network admin, the UI didn't need to be completely reviewed to list all post types by blogs.
  • if we would have chosen not to display a specific filter action in the template selectbox, the "bp option" could have been avoided...

Then my other idea, if 3460.05.patch is a "wont-fix", as said in previous comment is to hook the registered_post_type to progressively create this list of post types activated by blogs. I thought about transient but there's no get_blog_transient() function in WordPress and it's probably a bad idea as the key would need to be different by blog, on the other hand using transient can help us not updating the list of post types at each blog 'init'...

Last edited 7 weeks ago by imath (previous) (diff)

comment:63 landwire7 weeks ago

  • Cc sascha@… added

comment:64 follow-up: trishasalas7 weeks ago

For what it's worth, I've tested this on WordPress Trunk in TwentyThirteen with BuddyPress 2.0. I created 2 custom post types using the SuperCPT plugin and both were visible in settings. I checked both to be visible in the Activity Stream and they worked as expected.

Also, I love the that you have included 'pages' as optional in the settings. I can see that as being potentially very beneficial for certain sites.

I have not tested on multisite but plan to do so if I have the time this week (hopefully before Wednesday.)

comment:65 in reply to: ↑ 64 imath7 weeks ago

Replying to trishasalas:

I have not tested on multisite but plan to do so if I have the time this week (hopefully before Wednesday.)

Thanks a lot trishasalas for your tests and feedback :) I'm very interested by the tests you plan to do on a Multisite config.

comment:66 trishasalas7 weeks ago

@imath, I have multisite set up on WPE and will begin testing in a few hours (after I finish with work for the day.)

I will have a write up for you before Wednesday :)

comment:67 trishasalas7 weeks ago

I haven't gotten to test this as thoroughly as I would like but everything looks really good at first glance. The checkbox options for post type are in settings on the Network Admin and an Event created with the Events Calendar on the main site clearly visible in the Activity stream.

I don't have any other post types activated yet and I've not tried anything on the secondary site yet but I have it on my schedule for the weekend.

As an aside, I have the BuddyDev sitewide activity widget installed and it's not working properly. It knows something has happened because the Avatar is visible but everything else for that activity item is blank. If we go this route, it might be nice to let plugin authors know ahead of time (which I know we do with Beta, but perhaps a special announcement of this change would be good?)

I really like this patch as it stands now, I'm not as familiar with the complexities of multisite but the behavior seems intuitive.

I will keep testing and report back.

Last edited 7 weeks ago by trishasalas (previous) (diff)

comment:68 imath5 weeks ago

  • Keywords needs-testing added

I'm back on this ticket !

I've built a new patch to take in account improvements boonebgorges introduced in r8125 & r8126 to dynamically generate activity actions.

I think i've never been so close to "touchdown" with this ticket :)

So 3460.06.diff will require trunk > 8126 (of course see first paragraph). It's a bit less complex than its predecessors actually.

I've created a class BP_Blogs_Post_Type_Tracker that hooks 'bp_blogs_register_activity_actions' to set the activity actions ( after hooking init in the class at a 1000 priority, to be sure i catch all post types !).

In its setup_post_types() method, it uses by default 'bp_blogs_format_activity_action_new_blog_post' && 'bp_blogs_format_activity_action_new_blog_comment' for each post type.
But if a post type has a BuddyPress specific argument (bp_tracking), he can override these default values in favor of it's own function.
For instance

$args_post_type = array(
	'label'	            => __( 'Photo Sets', 'buddypress' ),
	'labels'            => $labels_photo_sets,
	'public'            => true,
	'rewrite'           => false,
	'show_ui'           => true,
	'show_in_admin_bar' => false,
	'supports'          => array( 'title', 'editor', 'author' ),
        'bp_tracking'       => array( 
                'post_callback'    => 'the_format_function_for_posts',
                'comment_callback' => 'the_format_function_for_comments',
       )
);

I've needed to edit a bit bp_blogs_format_activity_action_new_blog_post() && bp_blogs_format_activity_action_new_blog_comment() formatting functions so that they could work with any post types. I had to create a new activity meta to store the post_type value for each post type.
Boone : as you will see, i treat post like a post type. So if the patch does not make it for 2.0, later we will need to add an extra check for this "legacy" post_type ;)

I've maintained the "subsite" Network Tracking settings submenu :

  • i think it's easier this way to catch post types that are specific to child blogs
  • it gives a way to sub blog Administrator to customize the post type that are tracked, by adding or removing them. For instance if page post type is active on root blog, the child blog can decide not to track his page post type.
  • i've added a setting in BuddyPress Network settings to let Super Administrators eventually restrict this access.

Tested it in Multisite and regular configs. Seems to work fine :)

Thanks in advance for your eyes on this one.

imath5 weeks ago

comment:69 ircbot5 weeks ago

This ticket was mentioned in IRC in #buddypress-dev by trishasalas. View the logs.

comment:70 follow-up: boonebgorges4 weeks ago

Thanks for the work here, imath. I've finally started to take a close look at it.

The underlying technical architecture (where you register the activity_actions dynamically, based on labels->singular_name) looks good to me.

I am concerned about the workflow. I'm finding it to be a bit complex in some ways, and a bit incomplete in others, especially surrounding subsites. Some thoughts:

  • If I understand correctly, settings for a given post type are shared across sites. Say I'm running a plugin that registers post type 'foo' on a couple of subsites. If I want to record 'foo' on one subsite but not on another, then I'm out of luck, right? (aside from changing privacy settings)
  • In the same vein, it appears that a post type (say, Pages) that is marked as "tracked" in the global BP settings cannot be disabled on subsites. If this is going to be the case, then it should be reflected in the interface. However, I think it probably should *not* necessarily be the case - it seems like a very common use case to want to track a post type on the root blog but not the same post type on a secondary blogs.
  • The interaction of privacy settings and trackability on subsites is a bit confusing. As it stands, it looks like setting blog_public = 0 is enough to disable all tracking for the site. But if this is the case, then the interface should enforce it and/or provide documentation. So, eg, the whole checkbox section should be disabled or hidden when the blog is not public.
  • At the same time, if we're going to start allowing fine-grained, per-post-type user settings for blog tracking, maybe we should be doing away with the blog_public bit - or at least, we would only use blog_public = 0 to provide default tracking settings (unchecked)

Again, I'm not 100% sure which way to go with any of these questions, but all of them need some work and some discussion.

As for the milestone, I think that we are too late to complete this ticket, as currently conceived, for BP 2.0. I do think that the root-blog stuff is good, however, so there may still be time to do a partial fix. That would mean removing all parts of this patch that are specific to subsites, and introducing support only for CPTs on the root blog for 2.0. This would be a more modest new feature, but it is much simpler from a UX point of view, and it might give us a chance to put the meat of the patch through its paces while we work on the subsite stuff for 2.1. Does this seem worth it?

comment:71 in reply to: ↑ 70 imath4 weeks ago

Replying to boonebgorges:

Thanks for the work here, imath. I've finally started to take a close look at it.

You're welcome and thanks a lot for looking at it :)

  • If I understand correctly, settings for a given post type are shared across sites. Say I'm running a plugin that registers post type 'foo' on a couple of subsites. If I want to record 'foo' on one subsite but not on another, then I'm out of luck, right? (aside from changing privacy settings)
  • In the same vein, it appears that a post type (say, Pages) that is marked as "tracked" in the global BP settings cannot be disabled on subsites. If this is going to be the case, then it should be reflected in the interface. However, I think it probably should *not* necessarily be the case - it seems like a very common use case to want to track a post type on the root blog but not the same post type on a secondary blogs.

If a plugin doesn't want to be tracked, he can choose to set his show_in_nav_menu args to false.
Actually, each site Admin can choose what post type will be tracked. In the example above. If site A want to track 'foo' and site B doesn't want, he can choose to deactivate the tracking. Each site has the choice. If the SuperAdmin wants to be the only one to control, he can disable the sub site Site Tracking settings sub menu.

  • The interaction of privacy settings and trackability on subsites is a bit confusing. As it stands, it looks like setting blog_public = 0 is enough to disable all tracking for the site. But if this is the case, then the interface should enforce it and/or provide documentation. So, eg, the whole checkbox section should be disabled or hidden when the blog is not public.

Well it's the case, if the blog is defining itself as not public, then, the Site Tracking settings menu is no more available as anyway when the blog record post function will run no post type from his blog will be tracked. As soon as the admin will define his blog as public the Site Tracking submenu will show again.
I haven't check for the root blog, but i have some doubt he would do that, because it would be weird to activate the site tracking component in this case :)

  • At the same time, if we're going to start allowing fine-grained, per-post-type user settings for blog tracking, maybe we should be doing away with the blog_public bit - or at least, we would only use blog_public = 0 to provide default tracking settings (unchecked)

Well it's still a good way to completely disable the blog tracking, but it can be "done away" because as you said, each blog has the power to choose what post type will be tracked.

Again, I'm not 100% sure which way to go with any of these questions, but all of them need some work and some discussion.
As for the milestone, I think that we are too late to complete this ticket, as currently conceived, for BP 2.0.

I agree, this still needs a lot of testing. I'd say let's do it all for 2.1 ;)

My concern is more about the drop down list to filter activities.. If the network is tracking a lot of post types, this can be huge !!

comment:72 follow-up: boonebgorges4 weeks ago

  • Milestone changed from Future Release to 2.1

If a plugin doesn't want to be tracked, he can choose to set his show_in_nav_menu args to false.

Yes, but I'm talking about site admins.

In the example above. If site A want to track 'foo' and site B doesn't want, he can choose to deactivate the tracking.

You mean A can opt in while B can opt out, without disabling tracking altogether? I know that B can disable tracking by making his site private. But if A checks the Foo box, while B unchecks it, won't it be the case that all Foo posts across the network are still recorded? I don't see where in the patch it's checking the settings that are specific to the site.

if the blog is defining itself as not public, then, the Site Tracking settings menu is no more available

Ah, you're right. I missed that (the is_public check). But even this is a bit confusing, because the menu simply disappears when changing what appears to be an unrelated setting ('Discourage search engines').

My concern is more about the drop down list to filter activities.. If the network is tracking a lot of post types, this can be huge !!

Could be, but I have a hard time imagining a network that has more than a few dozen post types. (Think about how many plugins you'd be running if you had more than this!)

==

I had another thought as I was writing this. What, if anything, should we do about plugins that handle their own activity items? Say, bbPress. Ideally, we would keep them out of the list of checkboxes, or else they'd be duplicated in the activity stream. What do you think?

comment:73 in reply to: ↑ 72 imath4 weeks ago

Replying to boonebgorges:

You mean A can opt in while B can opt out, without disabling tracking altogether? I know that B can disable tracking by making his site private. But if A checks the Foo box, while B unchecks it, won't it be the case that all Foo posts across the network are still recorded? I don't see where in the patch it's checking the settings that are specific to the site.

Yes exactly each blog can define what post type can be tracked, and untrack a post type that is track by root blog.

if ( $blog_id != bp_get_root_blog_id() ) { 
   $trackable = (array) apply_filters( 'bp_blogs_is_blog_post_type_trackable', bp_blogs_blog_trackable_post_type( array(), $blog_id ), $blog_id ); 
}

the function bp_blogs_blog_trackable_post_type() gets the option of the child blog

Ah, you're right. I missed that (the is_public check). But even this is a bit confusing, because the menu simply disappears when changing what appears to be an unrelated setting ('Discourage search engines').

Well, we could create a specific option that would not make the tracking rely on this setting. And during the update fill that option with the Discourage one. Or we could rely on the fact that unchecking each post type will make my site "untrackable" which would be the same result than checking Discourage search engines

Could be, but I have a hard time imagining a network that has more than a few dozen post types. (Think about how many plugins you'd be running if you had more than this!)

Yes i guess you're right and even if themes in the network might create "exotic" post types, as they won't be in the root one and not network available they will fall in my fallback 'posted a new item'

==

I had another thought as I was writing this. What, if anything, should we do about plugins that handle their own activity items? Say, bbPress. Ideally, we would keep them out of the list of checkboxes, or else they'd be duplicated in the activity stream. What do you think?

I think, you're right!! i forgot this case, well i used to handle it in previous patch somehow by checking for a post type argument (bp_tracking set to false). That's a problem, because this means we'll need to handle it without relying on a post type argument because i doubt plugins would be all ready to add this custom argument.

Note: See TracTickets for help on using tickets.