Opened 10 years ago
Closed 10 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 )
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)
Change History (23)
#3
in reply to:
↑ 2
@
10 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
@
10 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:
↓ 7
@
10 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:
↓ 8
@
10 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
@
10 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.
Back end (full size)
It's more "weird" on the front end
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 ofBP_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
@
10 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
@
10 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.
#12
follow-up:
↓ 13
@
10 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
@
10 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
@
10 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
#15
follow-up:
↓ 16
@
10 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.
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.