Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clarify how ACF save_post is triggered by programmatic methods (eg: post import) #32

Open
drzraf opened this issue Apr 24, 2018 · 11 comments

Comments

@drzraf
Copy link

drzraf commented Apr 24, 2018

When using wp_insert_post() like does wxr-importer we save posts from an export file.
Such a file may (or may not) contains the field_<uniqid>() meta (eg: if meta does not come from WordPress or fields unique ids changed)
But they do contain the actual meta-key/meta-value registered with ACF:

This is a sample (shortened) WXR file:

<?xml version="1.0" encoding="UTF-8"?>
<!-- generator="WordPress/4.9.1" created="2018-04-24 18:40" -->
<rss version="2.0" xmlns:excerpt="http://wordpress.org/export/1.2/excerpt/" xmlns:content="http://purl.org/rss/1.0/modules/content/" xmlns:wfw="http://wellformedweb.org/CommentAPI/" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:wp="http://wordpress.org/export/1.2/">
  <channel>
    <description/>
    <pubDate>Tue, 24 Apr 18 18:40:31 +0100</pubDate>
    <language>en-US</language>
    <wp:wxr_version>1.2</wp:wxr_version>
    <item>
      <title>foo</title>
      <pubDate>Wed, 07 Mar 12 20:17:20 +0000</pubDate>
      <dc:creator>me</dc:creator>
      <guid isPermaLink="false">https://localhost/368</guid>
      <description></description>
      <content:encoded><![CDATA[body]]></content:encoded>
      <excerpt:encoded><![CDATA[excerpt]]></excerpt:encoded>
      <wp:post_id>368</wp:post_id>
      <wp:post_date>2012-03-07 20:17:20</wp:post_date>
      <wp:post_date_gmt>2012-03-07 20:17:20</wp:post_date_gmt>
      <wp:comment_status>closed</wp:comment_status>
      <wp:ping_status>closed</wp:ping_status>
      <wp:post_name></wp:post_name>
      <wp:status>publish</wp:status>
      <wp:post_parent></wp:post_parent>
      <wp:menu_order></wp:menu_order>
      <wp:post_type>news</wp:post_type>
      <wp:is_sticky>0</wp:is_sticky>
      <wp:postmeta>
	<wp:meta_key>sources</wp:meta_key>
	<wp:meta_value><![CDATA[1]]></wp:meta_value>
      </wp:postmeta>
      <wp:postmeta>
	<wp:meta_key>sources_0_url</wp:meta_key>
	<wp:meta_value><![CDATA[a:2:{s:5:"title";s:15:"BBC NATURE";s:3:"url";s:36:"http://www.bbc.co.uk/nature/foo";}]]></wp:meta_value>
      </wp:postmeta>
    </item>
  </channel>
</rss>

The file get imported correctly and the backend UI correctly shows the custom fields value's (since these fields must have been registered beforehand).
The issue is that fields won't work in the frontend (get_field() is NULL) because field_ab546aff meta keys are missing although the post was processed using wp_insert_post() on the ACF-enabled WP instance.
(These are repeater fields. The issue does not arise with plain text fields)

  • Saving the post manually from the backoffice creates the needed meta.
  • Saving the post using wp_insert_post(get_post(368))); will not work (!)

I'd likely attempt to resave each imported post, so that meta get created, but I'd like to know which ACF function to call for each of them. I could not find such a helper so far.

  1. why ACF "save" hook(s) reserved to admin backend (and not working otherwise)?
  2. which ACF function could serve as a resave-to-recreate-internal-meta-key helper?

NB: this is a long-standing "issue", simply google +acf +wp_insert_post

thank you

@elliotcondon
Copy link
Contributor

Hi @drzraf

Thanks for the question. I'm happy to help with this and believe the issue is quite simple.

The .xml example you provided contains 2 meta values, these are 'sources' and 'sources_0_url'.
Alongside each 'value', ACF also saves a 'reference' which is used to find the field for a given meta value.

For example, you would expect to see this for each value:
'sources' = 1
'_sources' = 'field_123'

My question is, why doesn't your .xml example contain these meta rows?

  • Does your .xml export tool ignore meta keys that start with an underscore?

I believe that if your .xml file contains the correct "reference" values, they shoudl import correctly and the ACF functions (such as get_field and have_rows) will continue to work as they should.

I don't believe there is any specific issue here with the wp_insert_post() function, but I'm happy to hear more about this.

if you believe there is an issue from ACF's end, please setup a working example so we can both login and experience the problem first hand.

@drzraf
Copy link
Author

drzraf commented Apr 25, 2018

The reference does not exists because the import file is the result of an export from a Drupal 7 CMS using custom tool.

When new post is inserted (whether from a WXR import, a CSV, or other programmatic mean), one would commonly provide ACF meta fields (using the format expected by ACF), but is unlikely to create "reference" fields because of their unpredictable nature (and the fact that during data transfer, a WordPress may be partially configured (plugins disabled, ACF fields unsynchronised or not registered yet, or fields may even have been registered simultaneously on two different instances (developer vs -prod) so reference are not the same, ... ...)

Since ACF can autogenerate them without problem when saving through the admin UI, I'd expect a function to exist to do the very same job using the internal WP/ACF API.
I'd even expect wp_insert_post() to trigger hooks that do that automatically.

The issue can be easily reproduced by

$id = wp_insert_post( [
   'post_content' => 'foo',
   'post_title' => 'bar',
   'post_status' => 'publish',
   'post_type' => 'page'  // ACF field named "source" (a repeater of links fields) is registered/enabled for "page"
] );

add_post_meta(368, "sources", 1);
add_post_meta(368, "sources_0_url", serialize(["title" => "BBC NATURE", "url" => "http://www.bbc.co.uk/nature/foo"]), 1);

print_r( get_field( "sources", $id ) );
/* expected:
Array
(
    [0] => Array
        (
            [url] => Array
                (
                    [title] => BBC NATURE NEWS
                    [url] => http://www.bbc.co.uk/nature/foo
                    [target] => 
                )
        )
) 
*/

/* actual:
1
*/

// Ok, then we expect to insert the post again in order to trigger ACF handlers so that (as with the admin UI) reference fields will be regenerated:

wp_insert_post( get_posts( $id ) );

print_r( get_field( "sources", $id ) );
/* actual (even less expected)
1
*/

@elliotcondon
Copy link
Contributor

Hi @drzraf

Thanks for the reply and clarifying how the .xml file was created.

This is definitely something we can look into in the future, but currently, there is no magic code that looks up and creates the "field references" during the 'save_post' or 'insert_post' actions.

These references are not random, but are infact the field's keys. If you load a field, you can access it's key. This is similar to a post and it's ID.

ACF is able to save these references correctly during the "Admin UI save" because they are included in the $_POST data.

This would be hard to do "magically" because multiple field's can have the same name.

Please note that ACF can still function without these "reference values". The get_field() and have_rows() functions will be able to lookup the field using it's name (instead of the key). If the field is found, everything will work as expected.

The only issue with this is that ACF allows multiple fields to have the same name, so there is a chance for failure using this "name lookup" approach - hence the need for saving the reference values in the first place.

This isn't something that I will be looking to investigate in the short term, so if you are in need of a solution now, I suggest that you hook into the "save_post" or "insert_post" WP actions and use the meta data to lookup ACF fields (using the get_field_object function) and then create the reference value.

Please take a look inside the ACF core code for the acf_update_value() function which will help show how to create the reference.

Thanks
E

@drzraf
Copy link
Author

drzraf commented Apr 26, 2018

The get_field() and have_rows() functions will be able to lookup the field using it's name (instead of the key). If the field is found, everything will work as expected.

but not with repeater then?

function remap_acf( $post_id ) {
  // xxx() finds ACF registered for that post and returns an associative array of $field_reference => $field_name
  $fields = xxx( $post_id );
  // ensure $field_name is unique among $fields (to avoid the risk of conflict) and abort otherwise
  foreach( get_post_meta( $post_id ) as $key => $value ) {
    if ( in_array( $key, $fields, TRUE ) ) {
      add_post_meta( $post_id, "_" . $key, "field_" . $field_reference, TRUE );
     }
   }
}

That xxx() function is probably similar to the one generating the <input> (to be posted in the admin UI).

@elliotcondon
Copy link
Contributor

Hi @drzraf

Thanks for the reply.

I've just spent some time looking through the ACF template functions to better understand the issue and believe I have discovered a bug.

Contrary to my belief, ACF is not providing a 'fallback' lookup technique to find field's via their names.
This explains why the get_field() and have_rows() functions are not working as expected for your data.

Can you please help test this bug by editing the function acf_maybe_get_field() located in the "includes/api/api-field.php' file and change the 3rd parameter from $strict = true to $strict = false

With this parameter default set to false, ACF should now be able to find the field object using just it's name.

After making this change, can you please test your issue and let me know if things start to work as expected.

@drzraf
Copy link
Author

drzraf commented Aug 24, 2018

I just had the opportunity get on this again. The change you suggested made it! Thank you!
(But the change was not part of 5.7.2)

@elliotcondon
Copy link
Contributor

Hi @drzraf

Thanks for the reply and PR.
I introduced this change back in 5.7.0 and had to revert it later in 5.7.1.

Unfortunately, this change causes issues with the get_field() function whereby the 'default_value' of a field is returned instead of null.

I'm happy to look into this again in the future, but won't have any time at the moment.

@drzraf
Copy link
Author

drzraf commented Jan 4, 2019

Don't you want to reopen to avoid forgetting it? (See also #54)

@elliotcondon
Copy link
Contributor

Hi @drzraf

Sure. I'm happy to re-open this ticket, but am unsure when I will be able to look into it.
I have a lot of plans for improving the core foundations of ACF and will hopefully be able to address this limitation then.

@elliotcondon elliotcondon reopened this Jan 6, 2019
@drzraf
Copy link
Author

drzraf commented May 8, 2019

Hi @elliotcondon ,
I got bitten again by this one today. Could you point me to the issue that forced you to revert $strict?
I'd be glad to help solving this one in some way (maybe even via a configurable hook?)

@elliotcondon
Copy link
Contributor

Hi @drzraf

I believe the particular ticket is over on our Helpdesk (not GitHub), so i won't be able to share with you the exact details, but I am happy to discuss the issue.

There were two main issues found when changing the $strictparameter fromtruetofalse`.

1. Incorrect formatting.
When you load a value via the get_field() function, ACF attempts to find the "field" that is connected to that value. In an optimal case, the field is called "home_hero_image" and is the only field with this name. Loading this field is safe and accurate.

The problem occurs when two or more fields use the same name, for example "image". Although these fields share the same name, they have completely different settings, and maybe are even different field types.

If the lookup is not strict, ACF may incorrectly find the wrong "image" field (because it will lookup via name, not key). This can lead to incorrectly formatted values.

2. Default values.
As a flow on effect from the issue above, this change allowed a field to be found even if no "meta" value existed for the given post.

For example, if a new post was created, but no value was entered into the "image" field, ACF would incorrectly find a field called "image" and attempt to "format" the null value.

This would change the null value to a string, int, or other data structure. This caused many complaints from developers who's if statements failed to work as normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants