Opened 10 years ago
Closed 10 years ago
#6119 closed defect (bug) (fixed)
Imports cause PHP Warnings in bp_blogs_format_activity_action_new_blog_post
Reported by: | DJPaul | Owned by: | imath |
---|---|---|---|
Milestone: | 2.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Blogs | Keywords: | has-patch commit |
Cc: | imath |
Description
When running a WordPress Import, the screen after you confirm the usernames generates a couple of PHP Warnings:
`
Notice: Undefined property: stdClass::$id in /srv/www/wordpress-develop/src/wp-content/plugins/buddypress/src/bp-blogs/bp-blogs-activity.php on line 127
Notice: Undefined property: stdClass::$id in /srv/www/wordpress-develop/src/wp-content/plugins/buddypress/src/bp-blogs/bp-blogs-activity.php on line 139
`
I used the WP theme unit test sample import for this. The line numbers may be different from trunk because I have am using a personal dev git branch. The backtrace shows they are coming from bp_blogs_format_activity_action_new_blog_post
.
Attachments (3)
Change History (14)
This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.
10 years ago
#3
@
10 years ago
Right, that's what I was trying to explain.
If bp_blogs_format_activity_action_new_blog_post
only happens when a new blog post is published, why would we ever already have the post title in the activity meta?
#4
@
10 years ago
bp_blogs_format_activity_action_new_blog_post
not only happens when a new blog post is published. It also happens to dynamically generate activity action strings. See https://buddypress.trac.wordpress.org/ticket/3856#comment:20 at the 5th paragraph of boonebgorges 's explanations.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
10 years ago
#7
@
10 years ago
- Keywords has-patch added; needs-patch removed
in 6119.04.patch changed empty checks to isset ones when checking the post_title and post_url properties as recommended by boonebgorges in our slack discussion.
I've also added some comments to try to explain some points.
- tested the "theme test" import file : no notice error
- tested adding a new post without a title : no notice error
- ran unit tests on non multi/multi modes : no failure
#8
@
10 years ago
Thanks for your work on this ticket, imath. I think the comments help a lot to make the code more understandable.
I am reluctant to make this comment again given your hard work, but... :) I thought we decided when we spoke with Boone the other day that we did not need the following clause added:
// If activity already exists try to get the post title from activity meta } else if ( ! empty( $activity->id ) ) { $post_title = bp_activity_get_meta( $activity->id, 'post_title' );
Chat link is https://wordpress.slack.com/archives/buddypress/p1421267309000163 Boone's reply is a few lines down:
If someone wants to update a post so that it has no title, we should respect that
Other than this last small point, the patch looks great.
#9
@
10 years ago
I haven't forgotten this part of our chat. And it's actually the case. If you test the patch you'll see that :
if i publish a post without a title, no notice
if i update a post having a title by removing the title, the activity action will be updated to inform the post has no title. This part is managed thanks to this function bp_blogs_update_post_activity_meta()
line 467 to 468
You should check https://wordpress.slack.com/archives/buddypress/p1421267970000197 to understand why i've kept
// If activity already exists try to get the post title from activity meta } else if ( ! empty( $activity->id ) ) { $post_title = bp_activity_get_meta( $activity->id, 'post_title' );
So in 6119.03.patch i've added a unit test to cover that possibility, but you'll see it will fail, that's because of #6127. You'll need to apply the patch i've added to 6127 to see test is not failing.
Hi DJPaul
Just checked with the WordPress importer plugin and did not manage to reproduce at first.
When importing the posts (exported from my personal blog), having the blogs component active, the activity are generated without any warnings due to BuddyPress. The only warnings are caused by the WordPress importer plugin (e.g.: Redefining already defined constructor for class WXR_Parser_Regex).
I'm sorry, i don't know about the WP theme unit test import tool you are talking about. On codex, i've found http://codex.wordpress.org/Theme_Unit_Test but when on import screen i don't get the 'Download and import file attachements' option mentioned at step 2 ??
So i've tested again with the WordPress importer plugin and the test file mentioned at step one and was able to reproduce.
The problem is due to the fact the imported post number 1169 has no title (edge case to test if the theme is placing the permalink on the post date)
Applying 6119.patch seems to fix the issue.