WordPress.org

Ready to get started?Download WordPress

Forums

BuddyPress Live Notification
[resolved] Some fixes needed to work with actual WP version (11 posts)

  1. Alex Popov
    Member
    Posted 3 months ago #

    Hi there,

    I fixed this live notifications with two simple steps, and now it works.

    1. Replace enclosed quotes in server response (see file bpln.js from line 20):


    var d2 = data.replace(/^\"(.*?)\"$/, "'$1'");
    data=JSON.parse(d2);

    2. Add NULL argument to function call in file bp-live-notification.php at line 152:


    $notifications =$wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", NULL), 1 );

    Optionally you can add "alert" class to notification (bp-live-notification.php, line 121:


    $menu_title = sprintf( __( 'Notifications <span id="ab-pending-notifications" class="pending-count alert">%s</span>', 'buddypress' ), count( $notifications ) );

    https://wordpress.org/plugins/bp-live-notification/

  2. Brajesh Singh
    Member
    Plugin Author

    Posted 3 months ago #

    Hi Alex,
    I am sorry for the inconvenience. but your proposed answers are not the solution.
    1. Can you please tell me why do we need to do this

    var d2 = data.replace(/^\"(.*?)\"$/, "'$1'");
    data=JSON.parse(d2);

    when the data sent from server is already in JSON format?
    2. What is the purpose of passing null when we are checking for new notification?

    Thank You
    Brajesh

  3. Alex Popov
    Member
    Posted 3 months ago #

    Hi, Brajesh.

    1. Server response is a string and therefore it is parsed as "somedata", e.g. "{"a":"1"}". So we have JSON syntax error there.

    2. Second argument in function $wpdb->get_results is NOT optional.

    Thanks for plugin.

  4. Alex Popov
    Member
    Posted 3 months ago #

    I'm sorry, I mean function $wpdb->prepare (file wp-includes/wp-db.php at line 1147):

    function prepare( $query, $args )

  5. Brajesh Singh
    Member
    Plugin Author

    Posted 3 months ago #

    Hi Alex,
    instead of using the Js mod, I added the 4th parameter as 'json'. So, we don't need to even parse that on client side anymore.

    Can you please check if the new upgrade works for you.

  6. Alex Popov
    Member
    Posted 3 months ago #

    There is a description of problem with prepare function: PHP Warning: Missing argument 2 for wpdb::prepare() and there is subject: Protect Queries Against SQL Injection Attacks.

    So actually fix for problem is (if you don't suppress error_reporting):

    $notifications =$wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", $notification_ids), 1 );

    Sorry again for NULL (this is a dirty hack), but I'm not a WP developer.

  7. Brajesh Singh
    Member
    Plugin Author

    Posted 3 months ago #

    Hi Alex,
    I understand that. That's why I said about your last update. We don't need to modify the query. It was related to JSON proper parsing. Please upgrade to the 1.0.5 and let me know if that works for you or not.

  8. Alex Popov
    Member
    Posted 2 months ago #

    Hi Brajesh,

    it works, thanks (I forgot about response format in jQuery)!

    You said: "We don't need to modify the query", but added parameter does not modify query, in your actual version it looks like:

    $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", 1)

    I don't know how WP handles queries, I'm completely WP newbie, but why $wpdb->prepare works with any argument in this case, even NULL (and with $notification_ids -- it's my mistake, sorry, actually $list_ids is needed)? As a result we have insecure query, doesn't?

  9. Brajesh Singh
    Member
    Plugin Author

    Posted 2 months ago #

    Hi,
    Thank you for confirming.
    List ids do not make the query insecure. It is not user generated, It is coming from database and that too is independent of user generated content. So, No that does not make the query insecure.

  10. Alex Popov
    Member
    Posted 2 months ago #

    Brajesh,

    thanks for support! I proposed also add "alert" class (defined in buddypress/bp-core/css/admin-bar.css) to notifications counter, but it's just a little cosmetic improvement:

    http://i.imgur.com/ZIF4N0X.png

  11. Brajesh Singh
    Member
    Plugin Author

    Posted 2 months ago #

    Hi Alex,
    Thank you.
    I am sorry I did not add the class alert as it is too generic and may cause conflict on some theme. I am sure you can style it using available selectors.

Reply

You must log in to post.

About this Plugin

About this Topic

Tags