Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comment on Activity Update hook event #16

Closed
JDGrimes opened this issue Dec 15, 2016 · 7 comments
Closed

Comment on Activity Update hook event #16

JDGrimes opened this issue Dec 15, 2016 · 7 comments
Assignees
Milestone

Comments

@JDGrimes
Copy link
Member

See #4 (comment).

@JDGrimes JDGrimes added this to the 1.0.0 milestone Dec 15, 2016
@JDGrimes JDGrimes self-assigned this Dec 19, 2016
@JDGrimes
Copy link
Member Author

The issue here is that comments can actually be left on any kind of activity, not just status updates. So we have to decide whether we want to only award comments on status updates, or for all activity. And if we award for all activity, then do we relate that to the activity entities or not?

@JDGrimes
Copy link
Member Author

If we don't relate the comments to the activity that it relates to, then we don't provide the user with the option to award the user the activity relates to instead of the commenter. It also doesn't allow them to conditionally award for comments on particular kinds of activity.

As I think about this though, I'm wondering if maybe rather than creating a dynamic entity that would affect the activity updates as well, maybe we should just create a generic activity entity for this purpose, to relate the comments to, but not used for that other event. So we'd just have one entity for comments on every type of activity, and just one event then too.

But the problem with that comes when we want to award more users, like when the activity relates to a group or blog or post or something like that, we don't have a way of providing those relationships dynamically if we just have a generic entity.

@JDGrimes
Copy link
Member Author

Here are the activity types registered in BuddyPress core:

// Activity component

	bp_activity_set_action(
		$bp->activity->id,
		'activity_update',
		__( 'Posted a status update', 'buddypress' ),
		'bp_activity_format_activity_action_activity_update',
		__( 'Updates', 'buddypress' ),
		array( 'activity', 'group', 'member', 'member_groups' )
	);

	bp_activity_set_action(
		$bp->activity->id,
		'activity_comment',
		__( 'Replied to a status update', 'buddypress' ),
		'bp_activity_format_activity_action_activity_comment',
		__( 'Activity Comments', 'buddypress' )
	);

// Blogs component

	if ( is_multisite() ) {
		bp_activity_set_action(
			buddypress()->blogs->id,
			'new_blog',
			__( 'New site created', 'buddypress' ),
			'bp_blogs_format_activity_action_new_blog',
			__( 'New Sites', 'buddypress' ),
			array( 'activity', 'member' ),
			0
		);
	}

// Friends component

	bp_activity_set_action(
		$bp->friends->id,
		'friendship_accepted',
		__( 'Friendships accepted', 'buddypress' ),
		'bp_friends_format_activity_action_friendship_accepted',
		__( 'Friendships', 'buddypress' ),
		array( 'activity', 'member' )
	);

	bp_activity_set_action(
		$bp->friends->id,
		'friendship_created',
		__( 'New friendships', 'buddypress' ),
		'bp_friends_format_activity_action_friendship_created',
		__( 'Friendships', 'buddypress' ),
		array( 'activity', 'member' )
	);

// Groups component


	bp_activity_set_action(
		$bp->groups->id,
		'created_group',
		__( 'Created a group', 'buddypress' ),
		'bp_groups_format_activity_action_created_group',
		__( 'New Groups', 'buddypress' ),
		array( 'activity', 'member', 'member_groups' )
	);

	bp_activity_set_action(
		$bp->groups->id,
		'joined_group',
		__( 'Joined a group', 'buddypress' ),
		'bp_groups_format_activity_action_joined_group',
		__( 'Group Memberships', 'buddypress' ),
		array( 'activity', 'group', 'member', 'member_groups' )
	);

	bp_activity_set_action(
		$bp->groups->id,
		'group_details_updated',
		__( 'Group details edited', 'buddypress' ),
		'bp_groups_format_activity_action_group_details_updated',
		__( 'Group Updates', 'buddypress' ),
		array( 'activity', 'group', 'member', 'member_groups' )
	);

// Members component

	bp_activity_set_action(
		buddypress()->members->id,
		'new_member',
		__( 'New member registered', 'buddypress' ),
		'bp_members_format_activity_action_new_member',
		__( 'New Members', 'buddypress' ),
		array( 'activity' )
	);

// xProfile component

	bp_activity_set_action(
		// Older avatar activity items use 'profile' for component. See r4273.
		'profile',
		'new_avatar',
		__( 'Member changed profile picture', 'buddypress' ),
		'bp_xprofile_format_activity_action_new_avatar',
		__( 'Updated Profile Photos', 'buddypress' )
	);

	bp_activity_set_action(
		buddypress()->profile->id,
		'updated_profile',
		__( 'Updated Profile', 'buddypress' ),
		'bp_xprofile_format_activity_action_updated_profile',
		__( 'Profile Updates', 'buddypress' ),
		array( 'activity' )
	);

@JDGrimes
Copy link
Member Author

I honestly don't think that comments on any of these really need to be awarded points in the initial version, except for the status updates. If we get requests for the others, then we can figure out what do do then.

@JDGrimes
Copy link
Member Author

Interestingly, comments themselves are also stored as activity items.

@JDGrimes
Copy link
Member Author

The ID of the activity the comment is on is the item_id, and the ID of the activity item that it is in direct reply to is the secondary_item_id. For top-level comments, these are the same, but for replies to those comments they are different. Because of this, I'm unsure how to handle this, for users who may want to award a comment author for replies to their comment, for example.

I guess really this is an optional relationship (WordPoints/wordpoints#590). We could go ahead and register it anyway, but if a user tired to use it to award points, the activity author would also be awarded points by it. That is, unless we make sure that we check that the activity type matches that expected for the entity. Which would make this a truly optional, and not just variable, relationship.

@JDGrimes
Copy link
Member Author

I didn't test this with the groups entity, where a group parent is a group resulting in loops, but we have something similar with the comment parent, and I happened to think I should check if this breaks the UI. It does, because it results in an infinite loop when trying to build the list of potential targets. We might be able to work around this in some way (or just not register those entity children yet), but ultimately this is a bug that should be fixed in WordPoints.

JDGrimes added a commit that referenced this issue Dec 23, 2016
And update the activity content entity to relate to it instead of only activity updates.

See #19, #16
JDGrimes added a commit that referenced this issue Dec 26, 2016
While working on #17, several issues were discovered in the Activity Update Post and Activity Comment Post hook events.

- The ham action was specifying the arg index incorrectly, the activity is the first parameter, not the third. Because of the way that the unit tests were constrcted, they did not detect this. They have now been updated.
- The ham and spam were firing for any activity type, not just the one in question (update or comment, respectively). This has been fixed by introducing an action object that checks the activity type, and the tests have been updated to check for this as well. This isn't such a big deal for spam, because of the way that the Points reactor handles reversals. However, for ham it is a big issue.

Issues were also discovered with the autoloader classmap, that we weren't aware of until now. Getting everything working correctly in regard to that also required a few changes to the dev-lib.

See #17, #16, #4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant