Issue due to priority added to “pre_get_posts”

  • Resolved Sonali Agrawal

    (@sonali1215)


    5 years, 4 months ago

    Hello Team,

    I am the developer of the CartFlows plugin. I have updated LifterLMS plugin (version 3.36.3) along with CartFlows plugin at my end and have faced the below issue.

    Issue: On visiting CartFlows CPT (steps/flows), it gives a 404 error.

    Below are the findings :

    1. In the latest code update, you have added priority to pre_get_posts hook as 15.

    CartFlows plugin also has the same hook with priority 20 and hence, it is causing the issue.

    As you are modifying $query object, the code in the CartFlows returns instead of executing further.

    // Bail if this is not the main query.
if ( ! $query->is_main_query() ) {
	return;
}

    As $query->is_main_query() returns false and below code is not executed.

    // Add cartflows step post type to existing post type array.
		if ( isset( $query->query_vars['post_type'] ) && is_array( $query->query_vars['post_type'] ) ) {
....
}

    It works if I set the priority as 10 for “pre_get_posts” hook present in CartFlows or comment out the remove_action(“pre_get_posts”) in your code snippet. This will fix the issue for CartFlows but I am not aware of how it will affect LifterLMS functionality.

    So It will be helpful if we both work on some solutions to make both the plugins compatible with each other.

    Actually, there are many users who are using the LifterLMS plugin as well as the CartFlows. So, if we can figure it out with the permanent solution then this issue can be solved and both the plugins will work without any conflicts.

    Looking forward to solving this ASAP.

Viewing 8 replies - 1 through 8 (of 8 total)
  • Plugin Contributor Rocco Aliberti

    (@d4z_c0nf)

    5 years, 4 months ago

    Hi Sonali,
    thanks for reporting this!
    One of our users has recently reported this issue, too.

    I’ve logged it here:
    https://github.com/gocodebox/lifterlms/issues/976
    a fix proposal, made a couple of days ago, can be found here:
    https://github.com/gocodebox/lifterlms/pull/977

    Out of curiosity, what happens to CartFlows if in your code you change that callback priority from 20 to 15 instead?
    Or if, alternatively, you change:
    add_action( 'pre_get_posts', array( $this, 'pre_get_posts' ) );
    to:
    add_action( 'pre_get_posts', array( $this, 'pre_get_posts' ), 16 );

    in the Cartflows_Landing_Markup constructor.

    This will fix the conflict as well but I am not aware of how it will affect CartFlows functionality.

    Thread Starter Sonali Agrawal

    (@sonali1215)

    5 years, 4 months ago

    Hello Rocco Aliberti,

    We have added the priority as 20 to avoid conflicts with a few themes and plugins. So if we increase the priority to 1-15, it might conflict with other themes and plugins.

    I have gone through the fix added in this commit. I have also pulled this changed file and it worked with CartFlows plugin. It seems that you have removed the line remove_action(“pre_get_posts”) and handled it in another way.

    It will be helpful for us if you release this modified code if it doesn’t affect the LifterLMS plugin functionality as well.

    Thanks & Regards,
    Sonali

    Plugin Contributor Rocco Aliberti

    (@d4z_c0nf)

    5 years, 4 months ago

    Hi Sonali Agrawal,
    I see what you mean, we changed our pre_get_posts callback priority for the same reason: theme/plugin conflicts.

    What about this?

    Or if, alternatively, you change:
    add_action( ‘pre_get_posts’, array( $this, ‘pre_get_posts’ ) );
    to:
    add_action( ‘pre_get_posts’, array( $this, ‘pre_get_posts’ ), 16 );
    in the Cartflows_Landing_Markup constructor.

    Anyway, many thanks for your feedback on this PR.
    We’re testing it doesn’t affect any LifterLMS plugin functionality.

    Thanks.

    Thread Starter Sonali Agrawal

    (@sonali1215)

    5 years, 4 months ago

    Hello Rocco Aliberti,

    Thanks for your quick reply.

    Regarding,

    Or if, alternatively, you change:
    add_action( ‘pre_get_posts’, array( $this, ‘pre_get_posts’ ) );
    to:
    add_action( ‘pre_get_posts’, array( $this, ‘pre_get_posts’ ), 16 );
    in the Cartflows_Landing_Markup constructor.

    I tried it by changing the priority to 16 but setting the priorities (greater than) 15 is not working. As it returns from below code due to remove_action(“pre_get_posts”,””,15) present in LifterLMS plugin:

    // Bail if this is not the main query.
if ( ! $query->is_main_query() ) {
	return;
}

    Increasing the priority, 1-10 works for CartFlows but it conflicts with few themes and plugins and it may conflict with LifterLMS plugin. So we cannot modify the priority.

    The mentioned PR works with CartFlows. It will help both the plugins to work together properly.

    If you are planning to release the PR in the next update then let me know the approx time.

    Thanks in advance 🙂

    Plugin Contributor Rocco Aliberti

    (@d4z_c0nf)

    5 years, 4 months ago

    Hello Sonali Agrawal,

    I tried it by changing the priority to 16 but setting the priorities (greater than) 15 is not working. As it returns from below code due to remove_action(“pre_get_posts”,””,15) present in LifterLMS plugin:

    That’s weird, I’m not positive that change doesn’t work.
    From my tests, setting the priority to 16 to the pre_get_posts callback in the Cartflows_Landing_Markup constructor works as well.

    I’m talking about this line:
    https://plugins.trac.wordpress.org/browser/cartflows/tags/1.3.0/modules/landing/classes/class-cartflows-landing-markup.php#L38

    If you are planning to release the PR in the next update then let me know the approx time.

    We’re currently testing it, unfortunately cannot give you any ETA yet.
    You might want to follow the issue status:
    https://github.com/gocodebox/lifterlms/issues/976

    Thanks for your cooperation on this! 🙂

    R

    Thread Starter Sonali Agrawal

    (@sonali1215)

    5 years, 4 months ago

    We have used the “pre_get_posts” hook at 2 places in our plugin.

    This works for us and has no issues.

    I’m talking about this line:
    https://plugins.trac.wordpress.org/browser/cartflows/tags/1.3.0/modules/landing/classes/class-cartflows-landing-markup.php#L38

    You can test the below file consisting of the priority issue.
    https://plugins.trac.wordpress.org/browser/cartflows/tags/1.3.0/modules/flow/classes/class-cartflows-permalink.php

    Steps :
    Install & Activate CartFlows -> Cartflows -> Click Add New -> Import Free flow -> View landing OR checkout page (within the flow)
    You will find 404 error (when LifterLMS plugin is active along with CartFlows)

    You might want to follow the issue status:
    https://github.com/gocodebox/lifterlms/issues/976

    Thanks!

    Regards,
    Sonali Agrawal

    Plugin Contributor Rocco Aliberti

    (@d4z_c0nf)

    5 years, 4 months ago

    Further tests confirmed that,
    as said above,
    changing the priority to 15 on this line https://plugins.trac.wordpress.org/browser/cartflows/tags/1.3.0/modules/landing/classes/class-cartflows-landing-markup.php#L38
    fixes the issue as well.
    😀

    Plugin Contributor Rocco Aliberti

    (@d4z_c0nf)

    5 years, 4 months ago

    Hi there,
    LifterLMS 3.36.4 has just been released, it solves the conflict with the CartFlows plugin.

    I’m going to mark this topic as resolved.
    Feel free to unmark it if you think otherwise.

    Thanks again for the cooperation Sonali!

    Take care.

Viewing 8 replies - 1 through 8 (of 8 total)
  • The topic ‘Issue due to priority added to “pre_get_posts”’ is closed to new replies.