corcel / acf

Advanced Custom Fields (ACF) plugin for Corcel
126 stars 96 forks source link

Repeater error #23

Open dave-calleja opened 7 years ago

dave-calleja commented 7 years ago

I am getting the error below when trying to access a repeater field's values:

FatalThrowableError in Repeater.php line 101: Call to a member function get() on null

The data is definitely there as shown below:

screen shot 2017-03-08 at 08 23 17

I am accessing the data as shown below:

$currentQuestion = Post::find(17); return $currentQuestion->acf->repeater('answers');

Thanks!

jgrossi commented 7 years ago

Hi @dave-calleja! Did you debug the error? Which line is it being triggered? Did you find where is the error? Can you send a PR to fix that? Thanks, JG.

dave-calleja commented 7 years ago

To be honest I wanted to confirm that my implementation ($currentQuestion->acf->repeater('answers')) was correct before it tried to fix it as documentation on the repeater here is a bit sparse, for example when in the repeater, do you have to specify the types of the inner fields? If so how?

In answer to your question: Repeater.php line 101

jgrossi commented 7 years ago

@dave-calleja good point, thanks. Actually the Repeater and FlexibleContent fields might be tested more. We have a large gap between what they should do in ACF plugin and what we have in corcel/acf plugin.

The Repeater should return a collection of fields, so you don't have to specify what you need. For example, you can give a collection of Text and Image fields. But as I said before, this should be tested more. I guess this warning is in the README file.

I suggest you to debug the 101 line, to find where the result is returning to null and why it's null, debuging SQL queries and checking them to find where's the problem. And then we can discuss here or in a PR the best way to fix that.

Corcel's ACF plugin has a lot to improve, even about performance. We're doing so many queries, like the original WP plugin does, but it's possible to improve ours and make it better ;-)

And thanks for helping and contributing. JG

gabdl commented 7 years ago

@dave-calleja, @jgrossi could it be something related to php 7? I'm having the same issue after change my environment to php7.

jordanyim1 commented 7 years ago

It appears this function is missing the 'type' parameter which is causing the ->get() call after it to break. $field = FieldFactory::make($meta->meta_key, $post); https://github.com/corcel/acf/blob/master/src/Field/Repeater.php#L102

I managed to make it work by always setting the type to 'text'.
$field = FieldFactory::make($meta->meta_key, $post, 'text');

However this will only work for text fields. If anyone knows how to dynamically get the type and place it there, the bug will be fixed.

jgrossi commented 7 years ago

Hi @jordanyim1 thanks for contributing with Corcel.

Below you can see the code that finds the field type dynamically, that's why the $type is null in this case.

https://github.com/corcel/acf/blob/master/src/FieldFactory.php#L49

Be welcome for contributing more and sanding a PR if you find a bug or want to include a new feature, right? Thanks!

adinca commented 7 years ago

Looks like the problem is in the query which fetches the field type. It searches for posts with post_type = 'acf-field' but at least in my setup, there's no post with this type. All fields for a repeater are stored in the postmeta table, as serialized array. The bad part is the search query that needs to be implemented since you need to use the like condition to look for the fieldKey

Here's the code I've used:

public function fetchFieldType($fieldKey)
    {
        /*
        $post = Post::on($this->post->getConnectionName())
                   ->orWhere(function ($query) use ($fieldKey) {
                       $query->where('post_name', $fieldKey);
                       $query->where('post_type', 'acf-field');
                   })->first();

        if ($post) {
            $fieldData = unserialize($post->post_content);
            $this->type = isset($fieldData['type']) ? $fieldData['type'] : 'text';

            return $this->type;
        }
        */
        $postMeta = $this->postMeta
            ->where('meta_value', 'like', '%' . $fieldKey . '%')
            ->first();
        if($postMeta) {
            $meta = unserialize($postMeta->meta_value);
            if(isset($meta['sub_fields'])) {
                foreach($meta['sub_fields'] as $subfield) {
                    if($subfield['key'] == $fieldKey) {
                        return $subfield['type'];
                    }
                }
            }
        }

        return null;
    }

It works, but I'm not happy with the solution. Would appreciate any feedback to improve it

jgrossi commented 7 years ago

Hi @adinca thanks for the contribution. Actually there's a acf-field post_type in the wp_posts table, like you can see on this screenshot:

image

When using the code you wrote, are all the phpunit tests passing?

adinca commented 7 years ago

@jgrossi unfortunately in my wp_posts table there's no acf-field post type. This is the curious part. What ACF / WP versions are you using?

I didn't run any tests yet

jgrossi commented 7 years ago

@adinca I'm using ACF Pro 5.3.9.2. In the tests/config/database.sql.zip you can find a SQL sample file to run tests. Whenever you can please run corcel/acf tests.

Those acf-field posts are inserted when you save a page/post that has a custom field on it, like a page with a custom field called page_link for example. You'd see post_excerpt=page_link and post_type=acf-field on wp_posts table.

adinca commented 7 years ago

I'm using ACF 4. This must be the reason for the different data. I'm using corcel on one of my existing projects (forgot to mentioned that), and haven't yet upgraded ACF to v5.

jgrossi commented 7 years ago

@adinca hum good to know that. I thought there were no big differences about architecture between ACF 4 and 5. We can write some tests to make sure the plugin is working for ACF 4 as well.

dimahali commented 7 years ago

@adinca is this code really working...

DanDvoracek commented 6 years ago

Hi there!

Frst of all, thanks for the plugin! Very seducing!

Is there any update about this thread? I'm wondering how we are meant to use the repeater field in the frontend. So far I have to do some kind of nested foreach loop to get what I want (and I have to cast the data...)

<?php $slides = (array) $post->acf->slide; ?> 

@foreach($slides as $slide)
  @foreach($slide as $item)
    {{ $item['slide_title'] }}
  @endforeach
@endforeach

What did I not understand?

The documentation in general would need a bit more to be honest. You have a great plugin idea but a bad documentation could kill it after all.

jgrossi commented 6 years ago

@DanDvoracek Corcel ACF plugin is a relatively new package. I started it by myself and after some contributions from the community, we've covered all ACF fields. Of course, they should be improved for performance reasons, and there's no documentation yet, just an explanation about the package.

You're welcome to take a look at the code and contribute to the documentation. Just send a PR ;-)

DanDvoracek commented 6 years ago

No worry @jgrossi ;) I'm gonna try to have a look at it properly once I get more time. Thanks!