eventespresso / event-espresso-core

Event Espresso 4 Core for WordPress: Build an Event Ticketing Website Today!
https://eventespresso.com
GNU General Public License v3.0
122 stars 87 forks source link

Allow page builder requests to have template_tags.php loaded #360

Closed joshfeck closed 6 years ago

joshfeck commented 6 years ago

@sethshoultes mentioned this in Slack:

i have both EE4 and Beaver Builder activated locally but i am getting a fatal error when trying to use the page builder on an event page:

Fatal error: Call to undefined function espresso_venue_id() in /x/wp-content/plugins/event-espresso-core-github/core/helpers/EEH_Schema.helper.php on line 70

I've seen something similar while working with the Elementor page builder plugin:

Fatal error: Uncaught Error: Call to undefined function espresso_list_of_event_dates() in /x/wp-content/plugins/event-espresso-core/widgets/upcoming_events/EEW_Upcoming_Events.widget.php:394 
Stack trace: 
#0 /x/wp-content/plugins/elementor/includes/widgets/wordpress.php(244): EEW_Upcoming_Events->widget(Array, Array) 
#1 /x/wp-content/plugins/elementor/includes/base/widget-base.php(462): Elementor\Widget_WordPress->render() 
#2 /x/wp-content/plugins/elementor/includes/base/widget-base.php(591): Elementor\Widget_Base->render_content() 
#3 /x/wp-content/plugins/elementor/includes/base/element-base.php(662): Elementor\Widget_Base->get_raw_data(true) 
#4 /x/wp-content/plugins/elementor/includes/base/element-base.php(662): Elementor\Element_Base->get_raw_data(true) 
#5 /x/wp-content/plugins/elementor/core/base/document.php(521): Elementor\Element_Base->get_raw_data(t in /x/wp-content/plugins/event-espresso-core/widgets/upcoming_events/EEW_Upcoming_Events.widget.php on line 394

One possible solution would be to tweak the conditionals that determine whether to include the template tags file.

The underlying issue is the page builders are trying to load content as if it was the front end of the site, but it’s not the front end of the site. Here’s the conditional from EE_System.core.php:

if (
    is_readable(EE_PUBLIC . 'template_tags.php')
    && ($this->request->isFrontend() || $this->request->isIframe() || $this->request->isFeed())
) {
    require_once EE_PUBLIC . 'template_tags.php';
}

so one idea could be to look at tweaking what’s defined as isFrontend()

and/or add a filter switch to that conditional to be used by a compatibility shim.

Pebblo commented 6 years ago

Another report here:

https://eventespresso.com/topic/editing-in-elementor-gets-un-error-uncaught-error-call-to-undefined-function/

I created a snippet that will include that file for specific actions, right now it checks for elementor:

https://gist.github.com/Pebblo/eb77e0c3ce13e52afb4482f52aa642cf

joshfeck commented 6 years ago

Another report:

https://eventespresso.com/topic/fatal-error-after-using-upcoming-event-widget/

sethshoultes commented 6 years ago

@mnelson4 is this something you feel comfortable looking into? Can we add @Pebblo's code to core or look into @joshfeck's suggestions?

mnelson4 commented 6 years ago

This is more Brent's domain, but if you really want to offload stuff from him, I can attempt it.

I'm torn between just always including template_tags.php and using a filter only load the file when a known page builder needs it. Reasons to always include template_tags.php:

Reasons to have a filter to only load template_tags.php when a known page builder needs it:

I'm inclined to just always include template tags, even on the back-end.

I think we want to not distract Brent with this, so Darren do you have any thoughts on this?

nerrad commented 6 years ago

So template_tags.php is just a bunch of function definitions. I'm assuming that that means there shouldn't be any issue just loading it everywhere because any usage of those functions in the backend or other contexts would cause problems anyways (such as what triggered this issue)! So, I think your assessment seems sound and we probably could get away with just loading it everywhere.

The only concern I have with loading it everywhere is with functions like espresso_get_events which call:

wp_reset_query();
wp_reset_postdata();

I wonder if the function is called somewhere in a cpt editor or wp-list-table context if that could cause things to blow up.

mnelson4 commented 6 years ago

Yeah using functions meant for the front-end from the back-end does seem problematic... yes, things might blow up further down the road. In that particular example, wp_reset_query() and wp_reset_postdata first check if global $wp_query is defined. I'd think the page builders would define it if it's needed, and even if they don't, those particular functions shouldn't blow up.

So I think this is worth trying, but ya, it's possible this will be a rabbit hole. I'm just going to put one foot in...

mnelson4 commented 6 years ago

PR merged to master that addresses this