Skip to:
Content

Opened 4 years ago

Closed 3 years ago

#5807 closed defect (bug) (fixed)

'new_member' activity type should not be a xProfile activity type

Reported by: imath Owned by: imath
Milestone: 2.2 Priority: normal
Severity: normal Version: 1.1
Component: Activity Keywords: has-patch commit
Cc:

Description (last modified by imath)

I manage an Intranet powered by WordPress and BuddyPress for my company. On this intranet, i've deactivated the xProfile component.
Lately, i've opened registrations to the Intranet for new members, and i've noticed each time a user signs up, an activity with no action and no content (no content is normal) is created. This activity is a 'new_member' type. The action is empty because xProfile is not active.

I'm sorry to post this ticket, a bit "à la dernière minute", but it would be great to consider at least the 5807.disable_new_member.patch for 2.1 milestone.

I've also built, just in case, 5807.move_new_member.patch, because i really think the 'new_member' activity type should be a 'members' component activity type.

Do you think it's doable ?

Attachments (5)

5807.disable_new_member.patch (2.0 KB) - added by imath 4 years ago.
5807.move_new_member.patch (12.6 KB) - added by imath 4 years ago.
5807.at_the_very_least.patch (574 bytes) - added by imath 4 years ago.
5807.02.patch (12.6 KB) - added by imath 4 years ago.
5807.03.patch (12.7 KB) - added by imath 3 years ago.

Download all attachments as: .zip

Change History (23)

#1 @imath
4 years ago

  • Description modified (diff)

#2 follow-up: @boonebgorges
4 years ago

imath, can you please research whether 'new_member' items have always been associated with 'xprofile'? Was there ever a time when they were associated with 'profile' when xprofile was inactive? (We have at least one activity type that does this.)

I'm wary of changing this activity type without some research because it could wreak havoc with people who are expecting the activity to be part of certain filters.

#3 in reply to: ↑ 2 @imath
4 years ago

  • Version changed from 1.5 to 1.1

Replying to boonebgorges:

imath, can you please research whether 'new_member' items have always been associated with 'xprofile'? Was there ever a time when they were associated with 'profile' when xprofile was inactive? (We have at least one activity type that does this.)

Yes: the 'new_avatar' one is associated with 'profile'

Since r1851 (birth of this type) in version 1.1, the 'new_member' has been associated with the $bp->profile->id component, meaning it should have always been 'xprofile'.

I understand your concern, that's why i thought the 5807.disable_new_member.patch could be a "not too risky" step for 2.1, and then we could review with some more time the other patch for 2.2

#4 @boonebgorges
4 years ago

Thanks, imath. Yes, I think that your conservative patch is fine for 2.1. Then let's think about something more substantial for 2.2.

#5 follow-up: @DJPaul
4 years ago

Just so I can understand, other than changing the type of these profile field entries to tidy it up, and moving the code from one component to another, this doesn't do anything? (e.g. it doesn't fix any problem)?

#6 follow-up: @boonebgorges
4 years ago

other than changing the type of these profile field entries to tidy it up, and moving the code from one component to another, this doesn't do anything? (e.g. it doesn't fix any problem)?

Incorrect. The problem is this: bp_core_new_user_activity() (in the Members component) creates an xprofile activity item. If this happens when the xprofile component is disabled, the proper activity action callback won't exist (bp_xprofile_format_activity_action_new_member()). So an empty string gets saved in the database, and an empty activity item is displayed when the page is rendered.

The proper fix is to move the whole works into the Members component, but this is more than I think we should do for 2.1. imath's minimal fix will at least keep the blank items from appearing in these cases.

#7 in reply to: ↑ 5 @imath
4 years ago

Replying to DJPaul:

(e.g. it doesn't fix any problem)?

Thanks DJPaul for your feedback, it looks like my english is not very clear in the description of this ticket (really sorry for that).

The 2 patches are actually fixing the same issue in 2 different ways. I'll try to explain myself in a better way using screenshots to describe the issue. Let's consider this configuration :

  • Registration open to new members
  • BuddyPress xProfile component deactivated

A user activates his account, an empty activity is generated.

https://farm4.staticflickr.com/3923/14708933610_23eba62e1c_z.jpg
Back end (full size)

It's more "weird" on the front end

https://farm6.staticflickr.com/5596/14895596185_2155512c3f_z.jpg
Front end (full size)

About the patches :

  • 5807.disable_new_member.patch will fix the issue by checking id xProfile is active before adding a 'new_member' activity and will add the 'new_member' activities to the NOT IN sql part of BP_Activity_Activity::get(). No empty activity will show anymore.
  • 5807.move_new_member.patch will fix the issue by associating the 'new_member' activity type to the Members component. Conceptually, I personaly think it's the best component to associate this type with, as nothing in the name of the type or in the action performed by the user is related to his profile.
  • I'm going to add a 3rd patch: 5807.at_the_very_least.patch which basically does not create an activity if xProfile is inactive. Applying it will avoid new empty activities but old ones will show.

It may be an edge case, i guess, as it has never been reported. But i might not be the only one who doesn't activate xprofile and it disturbs me a bit to create some content relative to an inactive component..

Anyway, in the meantime I'm simply using this :
remove_action( 'bp_core_activated_user', 'bp_core_new_user_activity' ); from bp-custom.php

#8 in reply to: ↑ 6 @imath
4 years ago

Replying to boonebgorges:

The proper fix is to move the whole works into the Members component, but this is more than I think we should do for 2.1. imath's minimal fix will at least keep the blank items from appearing in these cases.

Thanks boonebgorges for your comment, i agree. I posted mine because it took me a while to write it ;)

#9 @DJPaul
4 years ago

  • Keywords commit added

Thanks for the explanation, both. imath, thanks for the pictures -- I can understand the problem this causes now. Let's get the minimal fix added in for 2.1.

#10 @imath
4 years ago

In 8818:

Do not add "new_member" activities if xProfile component is disabled

In Members component, bp_core_new_user_activity() automatically creates an "xProfile" activity item ("new_member" type) as soon as a user account is activated. If this happens when the xProfile component is disabled, the proper activity action callback is not available, as a result an empty string is saved in the action field of the activity table. On front-end and back-end, these mini activities are displayed without any actions.

This is a temporary fix for 2.1, we will think about something more substantial for 2.2.

See #5807

#11 @imath
4 years ago

  • Keywords commit removed
  • Milestone changed from 2.1 to 2.2

#12 follow-up: @DJPaul
4 years ago

  • Keywords needs-refresh added; has-patch 2nd-opinion removed

Where are we with this for 2.2, imath? Do we want to do more here, or close this ticket and revisit later?

#13 in reply to: ↑ 12 @imath
4 years ago

Replying to DJPaul:

Where are we with this for 2.2, imath? Do we want to do more here, or close this ticket and revisit later?

Thanks DJPaul. I still think we should move the activity into the members component. I'm going to update the patch asap.

#14 @imath
4 years ago

  • Keywords has-patch added; needs-refresh removed

5807.02.patch is updated to latest trunk. Its goal is to move the new_member activity action into the members component

@imath
4 years ago

#15 follow-up: @DJPaul
3 years ago

Looking at the patch, I think it looks OK except I think bp_members_migrate_new_member_activity_component should be inside the bp-core-update.php as it's part of the upgrade routine.

#16 in reply to: ↑ 15 @imath
3 years ago

Replying to DJPaul:

I think bp_members_migrate_new_member_activity_component should be inside the bp-core-update.php as it's part of the upgrade routine.

In 5807.03.patch, i'm moving the migrate function into the bp-core-update.php file.

@imath
3 years ago

#17 @DJPaul
3 years ago

  • Keywords commit added

Nice work

#18 @imath
3 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 9273:

Move the 'new_member' activity type into the Members component

Since BuddyPress 1.1, the 'new_member' type was attached to the xProfile component. In version 2.2, we are moving it into the Members one. Once the user will have upgraded to this version, every existing 'new_member' activity items will have their component updated to the 'members' one and each time a new member will register, the 'new_member' activity will be attached to the members component.

Props boonebgorges, DJPaul

Fixes #5807

Note: See TracTickets for help on using tickets.