Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

Last modified 4 years ago

#7290 closed defect (bug) (no action required)

2.7.0-rc2 breaks GeomyWP + Groups Locator groups list

Reported by: dreadedhamish's profile dreadedhamish Owned by:
Milestone: Priority: highest
Severity: normal Version:
Component: Groups Keywords: reporter-feedback
Cc:

Description

Using BP 2.6.2 with most recent GeomyWP and GMW Add-on-Groups Locator, I can insert a map of all groups and a list of recent groups.

Upgrading to 2.7.0-rc2 the neither the map nor list of groups appears.
See here: http://wordpress-19919-55865-165695.cloudwaysapps.com/group-map/

I can give access to someone to further debug if necessary. I've not no idea what the conflict is.

Attachments (2)

Screen Shot 2016-10-15 at 4.44.18 pm.jpg (229.5 KB) - added by dreadedhamish 8 years ago.
2.6.2 Expected Behaviour
Screen Shot 2016-10-15 at 1.48.14 AM.png (33.4 KB) - added by tw2113 8 years ago.

Download all attachments as: .zip

Change History (36)

@dreadedhamish
8 years ago

2.6.2 Expected Behaviour

#1 follow-up: @tw2113
8 years ago

I'm seeing the errors above in my browser javascript console which could very likely be contributing to the issues. I know Google has been cracking down on map-based items that aren't using API keys lately, as they have become required items for the services.

#2 in reply to: ↑ 1 ; follow-up: @dreadedhamish
8 years ago

Replying to tw2113:

I'm seeing the errors above in my browser javascript console which could very likely be contributing to the issues. I know Google has been cracking down on map-based items that aren't using API keys lately, as they have become required items for the services.

These same warnings are present using 2.6.2, but don't affect the presentation of the map or list of groups.
Here is the live version: http://neighbourhoodcollective.org.au/group-map/ using 2.6.2

#3 @tw2113
8 years ago

I would still say it's something to look into taking care of, in case it's a contributing factor. Other details would be the different domains, Google probably sees the situation as two separate sites. Just saying, regarding these parts.

I am not going to close the ticket by any means, as I'd like other core devs to chime in, in case they have ideas/leads.

From my brief poking around, the div#gmw-map-3 isn't getting populated for some reason. I'd bring up the issue with the developers of this map plugin as well, as they maybe had a lead already and a fix is coming soon. It could happen.

#5 in reply to: ↑ 2 @dreadedhamish
8 years ago

Replying to dreadedhamish:

Replying to tw2113:

I'm seeing the errors above in my browser javascript console which could very likely be contributing to the issues. I know Google has been cracking down on map-based items that aren't using API keys lately, as they have become required items for the services.

These same warnings are present using 2.6.2, but don't affect the presentation of the map or list of groups.
Here is the live version: http://neighbourhoodcollective.org.au/group-map/ using 2.6.2

I've added an API key, which has resolved the error, and the problem persists.

#6 follow-up: @r-a-y
8 years ago

  • Keywords reporter-feedback added

The first thing to solve is why your Groups Directory isn't outputting anything.

  • Under "Settings > BuddyPress > Components", make sure Groups is enabled.
  • Under "Settings > BuddyPress > Pages", make sure a page is assigned to the Groups component.

If that fails, what happens when you disable GeomyWP? Does the Groups Directory work again?

#7 in reply to: ↑ 6 @dreadedhamish
8 years ago

Replying to r-a-y:

The first thing to solve is why your Groups Directory isn't outputting anything.

  • Under "Settings > BuddyPress > Components", make sure Groups is enabled.
  • Under "Settings > BuddyPress > Pages", make sure a page is assigned to the Groups component.

If that fails, what happens when you disable GeomyWP? Does the Groups Directory work again?

Groups is enabled, and page assigned.
The groups directory does function correctly: http://wordpress-19919-55865-165695.cloudwaysapps.com/groups/

The groups listing on the maps page is generated by geomywp, not buddypress, and it uses this list of groups to generate the map.

#8 @dreadedhamish
8 years ago

  • Keywords reporter-feedback removed

#9 @r-a-y
8 years ago

  • Keywords reporter-feedback added
  • Milestone changed from 2.7 to Awaiting Review
  • Priority changed from high to normal
  • Severity changed from major to normal

I think we'll have to wait for a GeomyWP dev to confirm the issue and to see what the problem is.

It's a premium plugin and we do not have access to it.

#10 follow-up: @ninjew
8 years ago

Hi,
@dreadedhamish on the site with the issue, can you please go to the page editor of the groups page and tell me which GEO my WP's shortcodes you entered in the content area?

And just for general information. Google now requires Maps API key in order to use its map services. That includes all domains created after the change took place, which is sometime in April of this year. That means, that the maps might work without an API key on your original site, which was created previously the new requirement. However, it will not work on the new site ( which has a different domain name ) if it was created after that date. So for the new site you would need a maps API key.

#11 in reply to: ↑ 10 @dreadedhamish
8 years ago

Under 2.7
http://wordpress-19919-55865-165695.cloudwaysapps.com/group-map/
[gmw map="3"]
[gmw form="3"]
Behaviour - no map - no group listing

Under 2.6.2
http://neighbourhoodcollective.org.au/group-map/
[gmw map="3"]
[gmw form="3"]
Behaviour - map + listing

Also - I added the google maps API key - no more warning.

Replying to ninjew:

Hi,
@dreadedhamish on the site with the issue, can you please go to the page editor of the groups page and tell me which GEO my WP's shortcodes you entered in the content area?

And just for general information. Google now requires Maps API key in order to use its map services. That includes all domains created after the change took place, which is sometime in April of this year. That means, that the maps might work without an API key on your original site, which was created previously the new requirement. However, it will not work on the new site ( which has a different domain name ) if it was created after that date. So for the new site you would need a maps API key.

Version 0, edited 8 years ago by dreadedhamish (next)

#12 @ninjew
8 years ago

Thank you.
I installed BP 2.7 on my test site and I see the issue as well. Looks like the groups query was modified in version 2.7. I am going to look into this change and I will update the groups extensions this week.

#13 @DJPaul
8 years ago

@ninjew When you look at it, can you let us know what has broken it for you? We want to ship BuddyPress 2.7 this week.

#14 @ninjew
8 years ago

@DJPaul for sure.

I took a quick look now and I found a couple of things that were modified and possibly caused my plugin to "break".

One of the filters I used in order to modify the groups query is

bp_groups_get_total_groups_sql

. In the previous version of BuddyPress the FROM and WHERE clauses were added to the $sql array which passes to that filter.

in line 778:

<?php
$sql['from']   = " FROM {$bp->groups->table_name_groupmeta} gm1, {$bp->groups->table_name_groupmeta} gm2,";

and line 784:

<?php
$sql['group_from'] = " {$bp->groups->table_name} g WHERE";

( BTW, the $sql[group_from] array value, which I also use for modifying the query, is also gone in 2.7. )

And in line 908, the SQL query which passes to that filter is created using a join:

<?php
join( ' ', (array) $sql )

However, in version 2.7 the FROM and WHERE clauses are missing from the $sql array and the SQL query is created in line 1093:

<?php
$paged_groups_sql = "{$sql['select']} FROM {$sql['from']} {$where} {$sql['orderby']} {$sql['pagination']}";

Now, to modify the groups query in my extension I have a custom function attached to the filter above. In the custom function I did something like:

<?php
$sql['from']  .= "my custom query.................";

and in the end of the function I return the value using join ( same way as BuddyPress plugin does in the previous version ):

<?php
return join( ' ', ( array )$sql );

However, with version 2.7 the returned SQL is missing the FROM and WHERE clauses which results in "no results".

I hope that makes sense. It is 1:00 am here and my brain is half asleep :). So forgive me if the above is confusing or doesn't make much sense. I will look into everything again tomorrow morning after I have my coffee.

Kind regards,

#15 @r-a-y
8 years ago

@ninjew - Check out this development post outlining the group query changes:
https://bpdevel.wordpress.com/2016/09/19/group-queries-have-been-rewritten-for-bp-2-7/

Going to ping @boonebgorges, just so he is aware as well.

If there is a change that would make your plugin compatible with these changes, please let us know or by posting a patch.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


8 years ago

#17 @boonebgorges
8 years ago

@ninjew Thanks for posting the detailed thoughts here.

As described in the bpdevel post linked by @r-a-y above, I did pretty extensive searches of the wordpress.org repo and GitHub at the time that we made the group query changes, but obviously this search only returned items that are listed in one of those two places :)

The backward compatibility break is unfortunate, but unavoidable, as we needed to make fairly extensive changes to the query format to make things work. See #5451. I believe that the filter in 2.7 has enough information to do what you're trying to do, but please let us know if it's unclear how that should work.

#18 @ninjew
8 years ago

@boonebgorges I understand, and that shouldn't be a problem. This extension needs my attention and a good update of its core regardless.

Thanks to @dreadedhamish, That's good that we noticed it before the official release of 2.7.

I will go over the post that @r-a-y posted above ( thank you ), and I will work on the update of this extension this week. I will do my best to release it ASAP.

#19 @boonebgorges
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Thank you very much, @ninjew! Don't hesitate to reach out, here or on the bpdevel.wordpress.com post, if you need any assistance or a second pair of eyes on your fixes.

#20 @ninjew
8 years ago

Will do, Thank you.

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


8 years ago

#22 @ninjew
8 years ago

@boonebgorges,
I have been trying integrate my Groups Locator extension with BuddyPress 2.7 but with no success so far.

I will explain how my extension work and how I integrated it in the previous version of BuddyPress, and hopefully you will be able to help me out integrating it with BP 2.7.

The purpose of my extension is to geotag BP groups and to be able to search and find groups based on address, distance and more. The extension creates a custom table in database gmw_groups_locator where it holds the location information of each groups together with the group ID to be able to map it when doing the search query. So the table has columns like id, address, city, state, lat, lng and a few more.

The groups proximity search is being done using a custom search form created using GEO my WP plugin ( the core plugin ). On form submission the form values pass via GET into the search query. In order to query the search results based on location and distance, the Groups Locator extension uses the bp_has_groups() function; by passing some of the form values via $args into the function but also by modifying the main SQL query. To modify the SQL query I used both bp_groups_get_paged_groups_sql and bp_groups_get_total_groups_sql filters.

In order to modify the main query to only pull groups within a specific radius from a specific address, I extended the query by modifying its clauses.

The SELECT clause i extended with something like:

<?php
$wpdb->prepare( ", gg.id, gg.lat, gg.lng, gg.address, gg.formatted_address, gg.map_icon, ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) AS distance",
                                        $radius, $lat, $lng, $lat );

and the groups_from clause I extended like:

<?php
 `{$wpdb->prefix}gmw_groups_locator` gg WHERE g.id = gg.id AND ";

The above added a few important fields to the groups object like address, coordinates, map icon and the distance from the original address that was entered in the form.

I also added a custom HAVING clause like:

<?php
$wpdb->prepare(" HAVING distance <= %d OR distance IS NULL ", $radius );

Which would filter the groups based on if they are within the range of radius entered in the form.

Now, with BP 2.7 and the split query, I am unable to figure this out yet.

In BP 2.7 the main query ( which used to query and pull all groups data ) now only pulls the groups ID which makes it impossible for me to add all the extra group data ( address, lat, lng and so on ) into the groups object. It seems that now the groups object is being pulled from database using BP_Groups_Group->populate(); ( This is happening within a foreach loop of the groups ID returning from the main query ) but I don't see any filters or a way to modify the populate() function in order to extend the groups object with the custom data from my extension.

This is where I am stuck and hopefully someone can help out.

FYI, I have been away from the project and development for about 6 months and I just got back to it recently. So please forgive me if that is something simple that I am missing :).

Thanks,
Eyal

#23 @ninjew
8 years ago

  • Priority changed from normal to highest
  • Resolution invalid deleted
  • Status changed from closed to reopened

Any thought on the above?

I am getting more users contacting me regarding this issue and the only solution I can provide them with at the moment is to downgrade BuddyPress plugin.

#24 @r-a-y
8 years ago

Would being able to filter the $paged_group_ids variable help for the split query?

Or do you need to modify the returned group object data ($paged_groups)? Or both?

#25 @boonebgorges
8 years ago

Hi @ninjew - Thanks for your patience. I needed to find a couple minutes to sit down and think about this.

First, you're not missing anything obvious. We don't make it very easy to add data to group objects. We should do this - perhaps by providing a filter in the __get() method, or an action in populate(), or something like that.

Second, it's true that what used to take a single SQL query now requires two separate steps. This is a bit annoying from the plugin developer's point of view, but it's actually good in the long run. Fetching your lat/lng data separately, as I'll demonstrate below, gives you a better opportunity to add fine-grained caching, to avoid needless queries, etc.

Here's a simplified example of how to adapt your plugin for BP 2.7. I haven't tested with your plugin - I'm not sure if it's publicly available - but I made some guesses based on the code that you pasted above. It should be enough to show you the general technique.

Two parts. The first is to filter the ID queries. The fact that you are using MySQL to do the math and using HAVING is what makes this hard, because we use $wpdb->get_col() to get matching IDs (so the distance alias breaks the syntax). One thing we might look into in BP is switching to $wpdb->get_results() to fetch a single column here - it would require a bit more logic in BP, but it would make this kind of syntax easier for plugins like yours. In any case, I think the only way around for the time being is to convert to a subquery (which might be faster anyway). Something like this:

<?php
function bp7290_filter_group_query( $query, $sql, $r ) {
        global $wpdb;

        $parts = explode( 'WHERE', $query );

        $table_name = "{$wpdb->prefix}gmw_groups_locator";
        $subquery = $wpdb->prepare( "SELECT gg.id FROM `$table_name` WHERE gg.id, gg.lat, gg.lng, gg.address, gg.formatted_address, gg.map_icon, ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) AS distance HAVING distance <= %d OR distance IS NULL",
                        $radius, $lat, $lng, $lat, $radius );
        $parts[1] = "g.id IN ($subquery) AND " . $parts[1];

        return implode( ' WHERE ', $parts );
}
add_filter( 'bp_groups_get_paged_groups_sql', 'bp7290_filter_group_query', 10, 3 );
add_filter( 'bp_groups_get_total_groups_sql', 'bp7290_filter_group_query', 10, 3 );

To add data to the group objects, you'll have to use the 'groups_get_groups' filter. It's the closest filter we've got to the SQL results themselves, so it should cover all regular uses. You'll also have to perform a similar query again, but this time only for the matched group IDs. (This one can be aggressively cached.) Something like this:

<?php
function bp7290_add_group_data( $groups ) {
        global $wpdb;

        $group_ids = wp_list_pluck( $groups['groups'], 'id' );
        $group_ids_sql = implode( ',', array_map( 'intval', $group_ids ) );

        $map_data = $wpdb->prepare( "SELECT gg.id, gg.lat, gg.lng, gg.address, gg.formatted_address, gg.map_icon, ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) AS distance FROM {$wpdb->prefix}gmw_groups_locator WHERE gg.id IN {$group_ids_sql}",
                                        $radius, $lat, $lng, $lat );

        // Add located data to BP's group objects
        foreach ( $map_data as $group_data ) {
                // you might be able to optimize this so you don't need nested loops
                foreach ( $groups['groups'] as &$group ) {
                        if ( $group_data->id == $group->id ) {
                                $group->lat = $group_data->lat;
                                $group->lng = $group_data->lng;
                                // etc
                                break;
                        }
                }
        }

        return $groups;
}
add_filter( 'groups_get_groups', 'bp7290_add_group_data' );

Hopefully this is enough to get you moving. Please let us know if you're successful - your feedback will help us to make this kind of thing easier to do in future versions of BuddyPress.

#26 @ninjew
8 years ago

Thank you @r-a-y and @boonebgorges for the replies.

@boonebgorges thank you for the solution above.

I have done some testing using the logic and scripts you provided above. I haven't got it to completely work yet but It seems that I am getting there. I am still having some issues with main query. I will continue working on it tomorrow morning and hopefully will find a solution.

What I was thinking at first would be better is a to have a filter in the populate(), as you suggested above. But I am not yet sure.

I will dedicate the coming weekend to work on this and hopefully will have a solution.

I will post back soon.

Thank you,

#27 @ninjew
8 years ago

OK, looks like I got things working again. I am still testing it but so far it looks good.

At first I tweaked a bit the script above provided by @boonebgorges ( Thank you ). The end results were 2 parts.

Part 1:

<?php
function filter_group_query( $query, $sql, $r ) {  
        
        // abort if no address entered. There is no need for proximity search.
        if ( empty( $this->form['org_address'] ) ) {
                return $query;
        }

        global $wpdb;

        $table_name = "{$wpdb->prefix}gmw_groups_locator";
        $subquery       = $wpdb->prepare( "
                SELECT gg.id 
                FROM `$table_name` gg 
                WHERE ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) <= %d",
            $this->form['units_array']['radius'], 
            $this->form['your_lat'], 
            $this->form['your_lng'], 
            $this->form['your_lat'], 
            $this->form['radius'] 
        );

        $where = "WHERE g.id IN ($subquery)";

        // if where clause already exist.
        if ( ! empty( $sql['where'] ) ) {
                $where .= " AND " . $sql['where'];
        }

        // return groups query
        if ( current_filter() == 'bp_groups_get_paged_groups_sql' ) {
                return "{$sql['select']} FROM {$sql['from']} {$where} {$sql['orderby']} {$sql['pagination']}";
        
        // return groups total query
        } else {
                return "SELECT COUNT(DISTINCT g.id) FROM {$sql['from']} $where";
        }
        }

This part of the code filters the groups by location and distance when address entered in the search form.

Note that I didn't use the "parts" method you suggested by using

<?php
 $parts = explode( 'WHERE', $query );

as it did not work for all scenarios. In the search form created by GEO my WP there is a "order by" dropdown which allows the user to change the order of the results. When the results are ordered by either "Alphabetically" or "Newly created", there is no "WHERE" clause added the the SQL query which "breaks" the function above ( when using the "parts" method ). Perhaps having WHERE 1 = 1 instead of omitting the where clause would be helpful in such scenarios.

The second part of the code is:

<?php
function add_group_data( $groups ) {
        global $wpdb;

        $group_ids         = wp_list_pluck( $groups['groups'], 'id' );
        $group_ids_sql = implode( ',', array_map( 'intval', $group_ids ) );

        if ( empty( $group_ids_sql ) ) {
                return $groups;
        }

        // if address entered and doing proximity search
        if ( ! empty( $this->form['org_address'] ) ) {

                $map_data = $wpdb->get_results( 
                        $wpdb->prepare( "
                                SELECT gg.id, gg.lat, gg.lng as `long`, gg.address, gg.formatted_address, gg.map_icon, ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) AS distance 
                                FROM {$wpdb->prefix}gmw_groups_locator gg
                                WHERE gg.id IN ( {$group_ids_sql} )",
                        $this->form['units_array']['radius'], 
                        $this->form['your_lat'], 
                        $this->form['your_lng'], 
                        $this->form['your_lat'], 
                        $this->form['radius'] 
                    )
                );
            
            //otherwise, just collect data from location table.
            } else {

                $map_data = $wpdb->get_results( "
                        SELECT gg.id, gg.lat, gg.lng as `long`, gg.address, gg.formatted_address, gg.map_icon
                        FROM {$wpdb->prefix}gmw_groups_locator gg
                        WHERE gg.id IN ( {$group_ids_sql} )"
                );
            }

        // Add located data to BP's group objects
        foreach ( $map_data as $group_data ) {
            
            // you might be able to optimize this so you don't need nested loops
            foreach ( $groups['groups'] as &$group ) {
                
                if ( $group_data->id == $group->id ) {
                    
                    $group->lat = $group_data->lat;
                    $group->lng = $group_data->long;
                    $group->address = $group_data->address;
                    $group->formatted_address = $group_data->formatted_address;
                    $group->map_icon = $group_data->map_icon;

                    if ( ! empty( $group_data->distance ) ) {
                        $group->distance = $group_data->distance;
                        }

                    // etc
                    break;
                }
            }
        }

        $orderby = ! empty( $_GET['gmw_orderby'] ) ? $_GET['gmw_orderby'] : 'distance';
                        
        if ( $orderby == 'distance' ) {
            usort( $groups['groups'], array( $this, 'order_results_by_distance' ) );
        }
                                
        return $groups;
}

This function collects the geolocation data from the locations table and add it to each groups object. Note that at the end of the function I used this:

<?php
 if ( $orderby == 'distance' ) {
     usort( $groups['groups'], array( $this, 'order_results_by_distance' ) );
}

to order the results by the distance ( when choosing "distance" from the order by dropdown ). The order by also used to happen in the main query because I was able to SELECT the distance using the ROUND function and then do ORDER BY distance. But that doesn't seem to be possible now.

I tested the above and it was working well.

However, I then came up with a different method which to me seems more efficient. I might be wrong and any input would be greatly appreciated.

I first added a new public function:

<?php
public function get_groups_based_location() {

                global $wpdb;

                $table_name = "{$wpdb->prefix}gmw_groups_locator";

        // if address entered, do a proximity search and get locations within the radius entered.
        if ( ! empty( $this->form['org_address'] ) ) {

                $groups = $wpdb->get_results( 
                        $wpdb->prepare( "
                                SELECT gg.id, gg.lat, gg.lng as `long`, gg.address, gg.formatted_address, gg.map_icon, 
                                ROUND( %d * acos( cos( radians( %s ) ) * cos( radians( gg.lat ) ) * cos( radians( gg.lng ) - radians( %s ) ) + sin( radians( %s ) ) * sin( radians( gg.lat) ) ),1 ) as distance
                                FROM `$table_name` gg
                                HAVING distance <= %d OR distance IS NULL",
                            $this->form['units_array']['radius'], 
                            $this->form['your_lat'], 
                            $this->form['your_lng'], 
                            $this->form['your_lat'], 
                            $this->form['radius'] 
                        )
                );
            
            //Otherwise, get all groups location.
            } else {

                $groups = $wpdb->get_results( "
                        SELECT gg.id, gg.lat, gg.lng as `long`, gg.address, gg.formatted_address, gg.map_icon
                        FROM `$table_name` gg"
                );
            }

        return $groups;
 }

The function will either get the location data of all groups or groups that are nearby the address entered.

I call the function before the bp_has_groups() function fires:

<?php
$this->gmw_groups = $this->get_groups_based_location();

Then the first part of the code above is now:

<?php
function filter_group_query( $query, $sql, $r ) {  
        
        // abort if no address entered
        if ( empty( $this->form['org_address'] ) ) {
                return $query;
        }

        global $wpdb;

        // get list of groups ID
        $group_ids         = wp_list_pluck( $this->gmw_groups, 'id' );
        $group_ids_sql = implode( ',', array_map( 'intval', $group_ids ) );

        // Filter the main query based on the location groups ID
        // if groups with location exist
        if ( ! empty( $group_ids_sql ) ) {
                $where = "WHERE g.id IN ( {$group_ids_sql} )";
        
        // otherwise show no results
        } else {
                $where = "WHERE 1 = 0";
        }

        if ( ! empty( $sql['where'] ) ) {
                $where .= " AND " . $sql['where'];
        }

        // return groups query
        if ( current_filter() == 'bp_groups_get_paged_groups_sql' ) {
                return "{$sql['select']} FROM {$sql['from']} {$where} {$sql['orderby']} {$sql['pagination']}";
        
        // return total groups query
        } else {
                return "SELECT COUNT(DISTINCT g.id) FROM {$sql['from']} $where";
        }
}

And the second part of the code is now:

<?php
function add_group_data( $groups ) {
        global $wpdb;

        // Add located data to BP's group objects
        foreach ( $this->gmw_groups as $group_data ) {
            
            // you might be able to optimize this so you don't need nested loops
            foreach ( $groups['groups'] as &$group ) {
                
                if ( $group_data->id == $group->id ) {
                    
                    $group->lat                           = $group_data->lat;
                    $group->lng                           = $group_data->long;
                    $group->address               = $group_data->address;
                    $group->formatted_address = $group_data->formatted_address;
                    $group->map_icon              = $group_data->map_icon;

                    if ( ! empty( $group_data->distance ) ) {
                        $group->distance = $group_data->distance;
                        }

                    // etc
                    break;
                }
            }
        }

        $orderby = ! empty( $_GET['gmw_orderby'] ) ? $_GET['gmw_orderby'] : 'distance';
                        
        if ( $orderby == 'distance' ) {
                usort( $groups['groups'], array( $this, 'order_results_by_distance' ) );
        }
                                
        return $groups;
}

The new method does the proximity SQL Query only once using the get_groups_based_location() function instead of having it 3 times for the filter_group_query, filter_group_query and add_group_data filters. Which I believe will improve performance.

I don't like the nested loops and the orderby distance function in the groups_data function. But it seems to be working and I will probably release it as an update for now.

I am going to look and see if it will be possible to have a better method if a hook will be added to the populate() functions.

Any feedback regarding the above is greatly appreciated.

Sorry for the long post.


#28 @ninjew
8 years ago

Well, I just realized that I can remove the first part of the code which uses the filters bp_groups_get_paged_groups_sql and bp_groups_get_total_groups_sql and simply pass the list of groups ID ( from $this->gmw_groups ) directly into the bp_has_groups() function using the include argument.

#29 follow-up: @boonebgorges
8 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

Thank you for the detailed feedback, @ninjew! It's extremely helpful.

Perhaps having WHERE 1 = 1 instead of omitting the where clause would be helpful in such scenarios.

This is a good idea. I've opened a ticket: #7332.

The new method does the proximity SQL Query only once using the get_groups_based_location() function instead of having it 3 times for the filter_group_query, filter_group_query and add_group_data filters. Which I believe will improve performance.

Yup, this seems like a good technique. If I were you, I'd also look into implementing some persistent caching, which, if done correctly, could eliminate at least some of the excess queries. More specifically, if you did the following after your initial query:

$groups = $wpdb->get_results( "
                        SELECT gg.id, gg.lat, gg.lng as `long`, gg.address, gg.formatted_address, gg.map_icon
                        FROM `$table_name` gg"
                );

foreach ( $groups as $group ) {
    wp_cache_set( $group->id, 'gmw_group_geo_data', $group );
}

then later on, when you need to append the data to the group objects, you could do something like:

$group_geo_data = wp_cache_get( $group_id, 'gmw_group_geo_data' );

This is similar to your $this->gmw_groups caching technique, with the additional advantage that sites running a persistent cache may be able to skip the query altogether on subsequent pageloads. (Of course, introducing this requires some other refactoring, and careful invalidation. So it may be an idea for future use.)

I've opened #7333 to talk more about populate() and extending core objects. For the record, I think we need to support use cases like yours for backward compatibility reasons. But I don't think we should necessarily be encouraging the modification of group objects. It may be preferable, in the future, to do something like this (I'm guessing that you need your geo data in the loop):

<?php
function bp7290_add_geo_data_to_group_display() {
    $group_id = bp_get_group_id();
    
    $geo_data = custom_function_to_get_geo_data_for_a_group( $geo_data );

    // do something with $geo_data
}
add_action( 'bp_directory_groups_item', 'bp7290_add_geo_data_to_group_display' );

custom_function_to_get_geo_data_for_a_group() would then fetch data from the WP cache (as described above), or from your own global/static object, or wherever you decide to store the data after querying it at the beginning of the loop, instead of relying on $groups_template->group. This way, you won't be dependent on any changes that BP might make to its own internal globals.

Thanks again for the extended discussion! It's been extremely helpful.

#30 in reply to: ↑ 29 @ninjew
8 years ago

You are welcome @boonebgorges and thanks for the reply.

Yup, this seems like a good technique. If I were you, I'd also look into implementing some persistent caching, which, if done correctly, could eliminate at least some of the excess queries.


Such caching system is actually already part of GEO my WP 3.0. So caching the the custom proximity search query will be possible in the future.

For the record, I think we need to support use cases like yours for backward compatibility reasons. But I don't think we should necessarily be encouraging the modification of group objects. It may be preferable, in the future, to do something like this (I'm guessing that you need your geo data in the loop):

<?php
function bp7290_add_geo_data_to_group_display() {
    $group_id = bp_get_group_id();
    
    $geo_data = custom_function_to_get_geo_data_for_a_group( $geo_data );

    // do something with $geo_data
}
add_action( 'bp_directory_groups_item', 'bp7290_add_geo_data_to_group_display' );

That actually might be a good solution. I am using something similar for a specific scenario for BuddyPress members geolocation. I need to look into this in more details and perhaps will change the way the plugin generate the geo data in general. As GEO my WP geolocate Post types, BP Members, BP Groups and WP User in a similar way by modifying the main queries.

Well, GEO my WP 3.0 has been under development for a long time, and after a break of a few months I am ready to get back to it. Perhaps with some new and fresh methods to query and generate the GEO data.

Thanks for all the feedback and suggestions above. And I am glad I could be a bit helpful as well.

Keep up the awesome work.

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


8 years ago

#32 @dreadedhamish
8 years ago

@ninjew Did this get resolved? Is there an update? Anything I need to know before testing?

#33 @ninjew
8 years ago

@dreadedhamish,
Yes, this was resolved a couple of weeks ago ( I believe that I updated you in GEO my WP forum ). The new version of the Groups Locator extension is available to download/update.

Please update your extension. If still there are any issues, please open a new support ticket ( https://geomywp.com/support-ticket/ ).

Thank you and to everyone above who helped out.

#34 @dreadedhamish
8 years ago

Thanks @ninjew
(The thread on the WMP forum is still unresolved)

Note: See TracTickets for help on using tickets.