Builder Won't Load with Gravity Form enabled

nrherron92 commented 3 years ago

Reproduction Steps

HS-169138

I cannot reproduce this on trial or local
With Gravity Forms and LifterLMS enabled attempt to open the Course Builder on a course with a lesson on it (it's on 5.0 but this was happening in 4.X)
I can get it to open without the gravity forms shortcode on the lesson (you'll see this in the staging site as the one titled clone)
I can also get it to open with out: title, description and ajax [gravityform id="1" title="false" description="false" ajax="true"]
However, having these on my trial site doesn't cause any issue

Expected Behavior

The course builder will open for edits

Actual Behavior

on BuddyBoss it opens to a 404 page on the front end
in twenty-twentyone it just loads and loads

Error Messages / Logs

Include any relevant error messages or log files


### System and Environment Information

<details>
<summary>System Report</summary>

<!-- Paste your System Report between the three backticks below this line -->

Wordpress

Home Url: [removed] Site Url: [removed] Login Url: [removed]/wp-login.php Version: 5.7.2 Debug Mode: Yes Debug Log: Yes Debug Display: No Locale: en_US Multisite: No Page For Posts: Blog (#1892) [[removed]/blog/] Page On Front: Home (#1772) [[removed]/] Permalink Structure: /%postname%/ Show On Front: page Wp Cron: Yes

Settings

Version: 5.0.0 Db Version: 5.0.0 Course Catalog: Course Library (#5464) [[removed]/course-library/] Membership Catalog: Membership Catalog (#2158) [[removed]/memberships/] Student Dashboard: Dashboard (#4871) [[removed]/dashboard/] Checkout Page: Registration Welcome and Sign In (#2159) [[removed]/home/choice/purchase/] Course Catalog Per Page: 10 Course Catalog Sorting: title,ASC Membership Catalog Per Page: 10 Membership Catalog Sorting: menu_order,ASC Site Membership: Not Set Courses Endpoint: my-courses Edit Endpoint: edit-account Lost Password Endpoint: lost-password Vouchers Endpoint: redeem-vouchers Autogenerate Username: yes Password Strength Meter: no Minimum Password Strength: weak Terms Required: yes Terms Page: Terms and Conditions (#2304) [[removed]/terms/] Checkout Names: required Checkout Address: hidden Checkout Phone: hidden Checkout Email Confirmation: no Open Registration: yes Registration Names: required Registration Address: hidden Registration Phone: hidden Registration Voucher: optional Registration Email Confirmation: no Account Names: required Account Address: optional Account Phone: optional Account Email Confirmation: no Confirmation Endpoint: confirm-payment Force Ssl Checkout: no Country: US Currency: USD Currency Position: left Thousand Separator: , Decimal Separator: . Decimals: 2 Trim Zero Decimals: no Recurring Payments: no Email From Address: [removed] Email From Name: [removed] Email Footer Text: Your success is our mission. Thanks for joining inBizWorks Email Header Image: 5337 Cert Bg Width: 800 Cert Bg Height: 600 Cert Legacy Compat: no

Constants

LLMS_REMOVE_ALL_DATA: undefined LLMS_REST_DISABLE: undefined LLMS_SITE_FEATURE_RECURRING_PAYMENTS: undefined LLMS_SITE_IS_CLONE: undefined

Gateways

Manual: Disabled Manual Logging: no Manual Order: 1

Server

Mysql Version: 8.0.18 Php Curl: Yes Php Default Timezone: UTC Php Fsockopen: Yes Php Max Input Vars: 1000 Php Max Upload Size: 1,000 MB Php Memory Limit: 512M Php Post Max Size: 1000M Php Soap: Yes Php Suhosin: No Php Time Limt: 240 Php Version: 7.3.14 Software: Apache Wp Memory Limit: 40M

Browser

HTTP USER AGENT: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.114 Safari/537.36

Theme

Name: Twenty Twenty-One Version: 1.3 Themeuri: https://wordpress.org/themes/twentytwentyone/ Authoruri: https://wordpress.org/ Template: Child Theme: No Llms Support: No

Plugins

Gravity Forms: 2.5.6 LifterLMS: 5.0.0

Integrations

BbPress: No BuddyPress: No

Template Overrides



</details>

This issue has be recreated:
+ [x] Locally
+ [X] On a staging site
+ [ ] On a production website
+ [ ] With only LifterLMS and a default theme

### Browser, Device, and Operating System Information

+ Browser name and version
+ Operating System name and version
+ Device name and version (if applicable)

thomasplevy commented 3 years ago

@eri-trabiccolo can you look into this please

eri-trabiccolo commented 3 years ago

@thomasplevy @nrherron92 Alright, after diggin' a little bit too much into this (I exported the infamous course from the user's website - of course disabling gf) I've come up with a partial explanation. The problem is that each of those course's lessons have a lot of post metas, specially because they're built with the beaver builder, and the bb post metas are pretty big. This leads to an apparently infinite load culminating in a timeout (or for some weird reason in a 404).

I said "partial" because I've not been able to find a direct connection between the post metas size and gravityforms, it might just be that it adds overhead?!

I've verified this theory both on the user's staging website and locally by adding this snippet (now removed from the user's staging):

        add_filter(
            'llms_lesson_skip_custom_field',
            function( $bool, $key ) {
                if ( $bool ) {
                    return $bool;
                }
                return ( 0 === strpos( $key, '_fl_builder_' ) );
            },
            10,
            2
        );

which leverages this filter: https://github.com/gocodebox/lifterlms/blob/5.0.0/includes/abstracts/abstract.llms.post.model.php#L1644

Ideally we could totally skip the custom fields in the builder (have to verify that our add-ons do not leverage this), although the problem would remain when exporting...

thomasplevy commented 3 years ago

@eri-trabiccolo and I discussed the in's and out's of this complicated case and arrived at the following (none of this set in stone as a proper direction forward)

Possibly lazy loading lesson data on the builder (like we do with courses)
Possibly implementing a safelist of custom field data that is loaded within the builder
- We'd need to coordinate with integrations (themes/plugins) that add data to the builder to ensure the data they rely on gets safelisted. Possibly we can do this automatically using our registration API but we'll need to look into the code to determine if this is true.
Possibly incrementally loading lessons in the builder (load 1 section and up to 10 lessons at time via AJAX) in favor of loading the entire course synchronously

None of the above resolves issues that will also be encountered during course exports, to resolve export issues we're going to have to adopt a strategy of incrementally exporting courses:

Trigger an async request that builds the course in the background (similar to CSV exports of student reporting data)
Build a course skeleton object (with no children) and store it in a temporary json file
Push sections into the course one at a time using the async processor functionality to ensure we don't timeout
Loop through sections, pushing lessons into each section one at a time and then lesson children into the lesson
When the export is completed prompt the user to download and then delete the temporary file

Seems time consuming but for courses that don't time out today there would be more or less no difference in time (more than likely) and larger courses that do time out won't have that problem

To resolve the immediate customer issue a patch can be provided to exclude the offending data (right now _fl_builder_* data (from beaver builder) which is absolutely unneeded. This won't resolve export issues but at the moment I don't think the affected user is worried about exports.

nrherron92 commented 3 years ago

@eri-trabiccolo @thomasplevy thanks! I followed up with the user!

gocodebox / lifterlms