cuny-academic-commons / bp-event-organiser

Allows Event Organiser plugin events to be assigned to BuddyPress groups and generates a group calendar page for each group
GNU General Public License v2.0
3 stars 1 forks source link

EO errors global can be uninitialized in some cases #52

Closed boonebgorges closed 7 years ago

boonebgorges commented 8 years ago

In some circumstances, the $EO_Errors global may not be properly initalized by EO, so that invoking methods like $EO_Errors->add() will result in fatal errors. This was discovered in the context where the end date was before the start date in eventorganiser_details_save() (as called from bp-event-organiser) but could probably arise in other circumstances.

I'm going to introduce a fix for bp-event-organiser, but I wanted to flag this for @stephenharris in case he wanted to come up with a more general way of initializing this variable. (I'd suggest something like a wrapper: maybe eo_errors(), which would return the global if it exists, and otherwise would create the global.)

stephenharris commented 8 years ago

Are you using that global, or just need to be initialised so that eventorganiser_details_save() doesn't error?

In either case, I'd be tempted to suggest the following:

global $EO_Errors;
if ( ! is_wp_error( $EO_Errors ) ) {
    $EO_Errors = new WP_Error();
}

eventorganiser_admin_init() is a private function and is liable to renamed/removed.

But I agree - that global was a rather naive attempt at being able to communicate errors from earlier on in the script execution to the point where it's needed to be displayed - it needs a reliable to initialise it.

boonebgorges commented 8 years ago

The goal was just to get eventorganiser_details_save() to work. The errors are not actually used in this context, at least as far as I can see in our implementation.

I'll make the change to initialize the global myself. Thanks for the response!

On 07/08/2016 10:21 AM, Stephen Harris wrote:

Are you using that global, or just need to be initialised so that |eventorganiser_details_save()| doesn't error?

In either case, I'd be tempted to suggest the following:

|global $EO_Errors; if ( ! is_wp_error( $EO_Errors ) ) { $EO_Errors = new WP_Error(); } |

|eventorganiser_admin_init()| is a private function and is liable to renamed/removed.

But I agree - that global was a rather naive attempt at being able to communicate errors from earlier on in the script execution to the point where it's needed to be displayed - it needs a reliable to initialise it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cuny-academic-commons/bp-event-organiser/issues/52#issuecomment-231388793, or mute the thread https://github.com/notifications/unsubscribe/AAPDY-Ih6isitObshqLL_5x1inCCW90Yks5qTmsTgaJpZM4JHdOP.