2

I'm new to php and WP plugin development.

I've sent my first plugin to the WordPress Plugin Directory for review.

Among other things, they made this comment:

"You also must avoid checking for post submission outside of functions. Doing so means the check runs on every single load of the plugin which means every single person who views any page on a site using your plugin will check for a submission. Doing that makes your code slow and unwieldy for users on any high-traffic site, causing instability and crashes."

Could someone please explain what does it mean? Sorry i'm really a fresh starter...

1

1 Answer 1

1

Now I've seen your code, I think the reviewer is wrong:

  • they're talking about the form in agg-as-options.php, which is handled the way I describe below
  • except they're wrong:
    • the form is processed in the agg_options function, not outside of a function as they say
    • this is only shown and processed on the admin aggregate-options page, i.e. only for admin users on that page and not all visitors.
      (Your bracket indentation isn't completely clear throughout, but this should be obvious even at first glance.)
  • you are loading agg-as-options.php even if we're not in admin site, though; you could explicitly put that in an if ( is_admin() ) { test (which means admin site, not admin permissions).

I'd guess it's something like the code in this question (the first example I could find):

  • you generate a form on the page which posts back to the same page
  • you have some code similar to if ( isset( $_POST['miguels_form'] ) ) { in the plugin that looks for submissions from that form and processes them, where 'miguels_form' is a hidden field or submit button value that you're using to identify submissions from that particular form
  • this code is at the top level in your plugin, i.e. it will run on all pages, not just pages that display your form, at the point that the plugin is loaded.

The approach in the question that I've linked is to move the $_POST handler into the shortcode that renders the form in the first place, or into a separate shortcode that just processes the POST and outputs the 'thank you' message instead. That should address their comment about this code not being in a function. Or there are probably other theme or hook mechanisms to restrict this to a single page.


The first time I saw this pattern I didn't like it, but I'm not sure there are many better ways:

  • you can instead write some script to POST the form data as JSON to a new REST API endpoint, or to an old-style admin-ajax endpoint, but that relies on client side script.
  • or you could post to a non-WordPress PHP file, but I don't really like that either.

So I guess this pattern is OK: they just want you to restrict checking for form POSTs to the page that you'll be posting to. I'm not aware of any better no-script ways to do this.

(I don't however buy their comment that this will make your code slow and unwieldy, unless PHP lazy-initializes the $_POST global because it is expensive to do so - and I can't imagine either that it's lazy or that it is expensive, except for e.g. file uploads.)

1
  • This is a huge help out. I'll add the if ( is_admin() ) { test and resubmit the code. I'll let you know their answer further on, thanks.
    – Miguel
    Commented Jun 19, 2020 at 15:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.