Opened 10 years ago
Closed 7 years ago
#5921 closed enhancement (maybelater)
Add post to blog button: Enhancement suggestion for bp-blogs/bp-blogs-template.php
Reported by: | lenasterg | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Blogs | Keywords: | needs-patch, trac-tidy-2018 |
Cc: |
Description
Hi.
I think in would be usefull if we had a "Add post to site" button similar to "Visit site" button for blogs.
What do you think about it?
In the patch you can find the code suggestion for it.
Can it be included in the Buddypress or should I "transform" it to a plugin?
Bests
Lena
Attachments (2)
Change History (14)
#1
@
10 years ago
- Keywords reporter-feedback needs-testing added
- Milestone set to Awaiting Review
I kinda like this. A few concerns for investigation:
- I'm a bit concerned about the overhead of checking
bp_current_user_can( 'edit_posts', $blog_id );
in the loop. If you wouldn't mind, would you do a bit of testing to see how that affects page load time, and in particular MySQL queries and memory usage? - My preference for the button text would be "New Post" :)
#2
@
10 years ago
Hi boonebgorges.
I'm glad you liked the idea.
I tried to minor down the overhead in the loop using caching (Actually I'm not quite sure if I put it in the right place, maybe it should goes into the BP_Blogs_Blog::get_blog_ids_for_user() function ).
Unfontunately, I'm not familiar with correct testing :-) Could you please, provide some info, guidelines etc you use in similar occasions?
#3
@
10 years ago
Thanks, lenasterg! I hadn't thought of this specific way of caching the values, but at a glance it seems like it should work fairly well. It'll still require switch_to_blog()
for the cap checks, but at least the cap checks will only happen when we know that the current user is a member of the blog.
If it turns out that your technique doesn't perform well, we might try messing with WP's get_blogs_of_user()
, which I believe gets role information in a single query.
Here's a simple technique to test the query overhead:
- Add
define( 'SAVEQUERIES', true );
to wp-config.php - Put the following function somewhere (wp-config.php is fine):
function debug_database_queries() { global $wpdb; echo '<pre>'; print_r( $wpdb->queries ); echo '</pre>'; } register_shutdown_function( 'debug_database_queries' );
Run that before and after the patch. That'll give you a sense of how many queries are being introduced, and how that affects load time.
#4
@
10 years ago
- Milestone changed from Awaiting Review to Future Release
I think we're overdue talking about where we want to take the Blogs component in the future, but this kind of idea is one direction that we might want to take it. If everyone contributing to this change can carry on developing the patch and check on the performance concerns, I've no problem with it going in.
#5
@
10 years ago
Hi. Sorry for late response.
I run the @boonebgorges debug function, with and without the suggested patch on an installation with 25000 blogs / 20 per page.
By hitting the /blogs page:
For notlogged user: no queries added.
For logged in user with 5 blogs: 1 query added.
For super admin: 0 query added.
#6
@
10 years ago
- Keywords reporter-feedback needs-testing removed
- Milestone changed from Future Release to 2.2
Excellent - thanks, lenasterg! I think this is looking pretty good.
I'd like to suggest a change to the cache scheme. Instead of
wp_cache_set( 'bp_blog_ids_of_user_' . bp_loggedin_user_id() . '_inc_hidden', $blogs, 'bp' );
let's do this:
wp_cache_set( bp_loggedin_user_id(), $blogs, 'bp_blog_ids_of_user_inc_hidden' );
It makes invalidation a wee bit easier, is in keeping with how we do most of our cache groups, and makes cache grouping more fine-grained and customizable. See #5733.
I also think we should move the caching right into BP_Blogs_Blog::get_blog_ids_for_user()
.
#7
@
10 years ago
@boonebgorges, hi.
Sorry, for asking :-)
Should I make the changes you suggest and attack a patch for you to check or will you apply them directly to trac?
#8
@
10 years ago
- Keywords needs-patch added
lenasterg - Sorry for not being clearer :) Please go ahead and write a new patch if you have a chance. And if you are able to write unit tests for the invalidation of the new cache, that would be really great. Here's an example of how we do a similar test in the activity component: https://buddypress.trac.wordpress.org/browser/tags/2.1.1/tests/phpunit/testcases/activity/functions.php#L299
#11
@
7 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
suggested Add new post button to blog button