Skip to:
Content

BuddyPress.org

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's profile 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)

blog_add_post_button.patch (2.7 KB) - added by lenasterg 10 years ago.
suggested Add new post button to blog button
bp-blogs.patch (3.4 KB) - added by lenasterg 10 years ago.
With caching of bp_blog_ids_of_user_XXX

Download all attachments as: .zip

Change History (14)

@lenasterg
10 years ago

suggested Add new post button to blog button

#1 @boonebgorges
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 @lenasterg
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?

@lenasterg
10 years ago

With caching of bp_blog_ids_of_user_XXX

#3 @boonebgorges
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 @DJPaul
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 @lenasterg
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 @boonebgorges
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().

Last edited 10 years ago by boonebgorges (previous) (diff)

#7 @lenasterg
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 @boonebgorges
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

#9 @lenasterg
10 years ago

Sorry, for not responding. I will make it as soon as I can.

#10 @DJPaul
10 years ago

  • Milestone changed from 2.2 to Future Release

#11 @DJPaul
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/

#12 @DJPaul
7 years ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.