soberwp / controller

Composer package to enable a controller when using Blade with Sage 9
MIT License
368 stars 43 forks source link

ACF error on archive and 404 pages. #114

Open christianmagill opened 5 years ago

christianmagill commented 5 years ago

I'm receiving the following warning on archive and 404 pages.


Warning: Invalid argument supplied for foreach() in /html/app/plugins/advanced-custom-fields-pro/includes/api/api-value.php on line 702

Stacktrace
----------

Fatal error: Uncaught ErrorException: Invalid argument supplied for foreach() in /html/app/plugins/advanced-custom-fields-pro/includes/api/api-value.php:702 Stack trace: 

#0 /html/app/plugins/advanced-custom-fields-pro/includes/api/api-value.php(702): App\{closure}(2, 'Invalid argumen...', '/home/...', 702, Array) 

#1 /html/app/plugins/advanced-custom-fields-pro/includes/api/api-template.php(213): acf_get_meta(0) 

#2 /html/app/plugins/advanced-custom-fields-pro/includes/api/api-template.php(169): get_field_objects(0, true) 

#3 /html/app/themes/adventhealth/vendor/soberwp/controller/src/Module/Acf.php(79): get_fields(Object(WP_Post_Type)) 

#4 /html/app/themes/adventhealth/vendor/soberwp/controller/src/Controller.php(143): Sober\Controller\Module\Acf->setData(true) 

#5 /html/app/themes/adventhealth/vendor/soberwp/controller/src/Controller.php(88): Sober\Controller\Contr in /html/app/plugins/advanced-custom-fields-pro/includes/api/api-value.php on line 702
darrenjacoby commented 5 years ago

@christianmagill Hmm, just want to make sure that you're on the latest version?

christianmagill commented 5 years ago

@darrenjacoby Thanks for the quick response!

Yes I am on 2.1.1, although the error also exists in 2.1.0. I updated in an effort to fix the issue to no avail.

Running ACF 5.7.10 as well.

darrenjacoby commented 5 years ago

@christianmagill does the error persist if you add an advanced custom field to a category/taxonomy? I haven't experienced this issue on 2.1.1, so any information to try recreate on my end would be helpful.

darrenjacoby commented 5 years ago

@christianmagill following up here

dotsam commented 5 years ago

I'm able to duplicate this on the 404 page, but not on an archive page.

The behavior I'm seeing on a 404 page seems to be completely an ACF bug, as I can replicate it on a stock WP 5.0.3 install w/ ACF 5.7.12 and the twentynineteen theme. Calling ACF's get_field_objects() on a 404 page here produces the same error. ACF has some logic that defaults to 'post' with an ID of 0 or null, and then assumes it's been able to get metadata for that combination and tries to foreach on it without checking if it's an array.

I'll file a bug report with ACF, but if you wanted to avoid this in controller, I would think a check for $query being false-y and then not running get_fields() is a reasonable workaround.

As to the issue occurring on a taxonomy/archive page, I haven't been able to replicate that either with controller or with a vanilla install.

christianmagill commented 5 years ago

@darrenjacoby Sorry for the late reply, I've been off of the related project for a bit.

The issue does not appear on category/taxonomy pages, but it does also appear on the front/index view when a page is not assigned.

I'm also running into the same issue on a new project with fresh roots/sage installation.

mmirus commented 5 years ago

Replicated on 404 and on a CPT archives.

Temporarily worked around it like this:

App.php

<?php

namespace App\Controllers;

use Sober\Controller\Controller;

class App extends Controller
{
    protected $acf = false;

    public function __construct()
    {
        if (acf_get_valid_post_id(get_queried_object())) {
            $this->acf = true;
        }
    }

// ...

My understanding is that get_fields accepts any of the following:

Controller passes get_queried_object() to get_fields(). In the case of a 404, get_queried_object() returns NULL, and for a CPT archives, it returns a WP_Post_Type object, neither of which are valid values.

Hope this helps towards a resolution!

christianmagill commented 5 years ago

@mmirus I believe your work around while preventing the error, would also prevent acf option fields from being loaded as well.

mmirus commented 5 years ago

@christianmagill - I just noticed that what I'm seeing is not 100% the same as what you're experiencing. I'm not actually getting an error, just a warning, and my message traces back somewhere slightly different:

Warning: Invalid argument supplied for foreach() in /mnt/c/Users/mmirus/valet/thelibreinitiative/wp-content/plugins/advanced-custom-fields-pro/includes/acf-meta-functions.php on line 95

They do still seem to share some characteristics, though, and probably the same root cause.

Looks like you're correct about my workaround dropping the options fields--good catch. If losing the options isn't an... option... for you, you can find an alternate approach (please share, if you do!) or downgrade to 2.1.0 until Darren has a chance to figure this out. I believe this was introduced in 2.1.1, so if you don't need the other changes in that update, you can just drop back a version for now.

mmirus commented 5 years ago

You might also be able to use my workaround and then just manually fetch the options in your controller with something like this:

<?php

declare(strict_types=1);

namespace App\Controllers;

use Sober\Controller\Controller;

class App extends Controller
{
    protected $acf = false;

    public function __construct()
    {
        if (acf_get_valid_post_id(get_queried_object())) {
            $this->acf = true;
        }
    }

    public function acfOptions() : array
    {
        return get_fields('options') ?: [];
    }
darrenjacoby commented 5 years ago

Sorry everyone, I'll get this fixed as soon as I get a chance! Been swamped under last month so still catching up a little.

mmirus commented 5 years ago

No worries, take your time. Appreciate all the work you've put into Controller to make it such a useful tool for us.

darrenjacoby commented 3 years ago

Pushed bee62f based on the above, not yet tagged but should remove the issue.

(Little late I know)