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

Last created block overwrites data in previous same type blocks #3

Open
ATomilov opened this issue Aug 27, 2018 · 38 comments
Open

Last created block overwrites data in previous same type blocks #3

ATomilov opened this issue Aug 27, 2018 · 38 comments
Assignees
Labels
bug Something isn't working

Comments

@ATomilov
Copy link
Contributor

ATomilov commented Aug 27, 2018

Hey. If I create several blocks of the same type on the page, for example, testimonials, then the information entered in the last created block overwrites what was contained in the previous blocks of that type. I think it's a cache, but I can not figure out how to fix it. P.S. It's not a cache, just render elements with same id, name and etc. P.S.S Although, if the matter were in the same names, ids and etc, then changing data in any block of the same type overwrote old data in others, but it is not. P.S.S.S I think that happened because the same type blocks on one page have the same post_id. Look at the pictures:
image
image
Have you any ideas how fix it?) Sorry to trouble you)

@jcklpe
Copy link
Contributor

jcklpe commented Aug 27, 2018

Hey @ATomilov You got the plugin to work? Would you mind taking a look at the issue I logged? I think I am missing something super basic.

@jcklpe
Copy link
Contributor

jcklpe commented Aug 27, 2018

@ATomilov I managed to get it working but now I seem to be running into the same problem as you. Did you find a solution?

@rchipka
Copy link
Owner

rchipka commented Aug 27, 2018

A temporary solution would be to handle this as a repeater field

@rchipka
Copy link
Owner

rchipka commented Aug 27, 2018

Long term solution would be to store this data in block attributes and prefer to check there first

@jcklpe
Copy link
Contributor

jcklpe commented Aug 27, 2018

Hmmm repeater field would be difficult for my usecase since I place my title block in a lot of different locations interspersed through out the blog post.

In the thread on the ACF github page you mention the ability to post multiple testimonies. So it seems to be working correctly for you? How'd you do that then? Through the block attributes? Looking at your post though I can see on the TODO: you list storing field values as block attributes, so I'm assuming that you were saying that in the hypothetical but it doesn't work yet exactly? Just wanting to clarify. And thank you for your work again and communication.

@jcklpe
Copy link
Contributor

jcklpe commented Aug 28, 2018

Closed my previous issue and migrating this new issue over to here as it seems to be the same and is more narrowly focused on here:

image

image

@jcklpe
Copy link
Contributor

jcklpe commented Aug 28, 2018

Also I don't know if this is what you meant but this was my result trying to use a repeater field:

image

And

image

@jcklpe
Copy link
Contributor

jcklpe commented Aug 28, 2018

@ATomilov Hey I actually found a separate plugin that fixed this problem for me. You might want to give it a shot. https://github.com/rheinardkorf/advanced-custom-blocks

@rchipka You want want to check it out too as it has the same name as your project and seems to be further along in certain ways (though it doesn't hook into ACF specifically). If nothing else it might have some code you can or want to make use of.

@ATomilov
Copy link
Contributor Author

@thedonquixotic I don't know, maybe it's worst way to resolve this, but maybe we can use meta_key (block_id) and post_id to get meta_id and then get meta_value. I just try found some unique attribute :)
image
And ok, I'll try what are you offering.

@ATomilov
Copy link
Contributor Author

@thedonquixotic hah, no way. I have more trouble with that new plugin.

@ATomilov
Copy link
Contributor Author

ATomilov commented Aug 28, 2018

@thedonquixotic , @rchipka I do not know the galactic or intergalactic scale is bad my solution, but it seems to me that at least it will be useful (or not at all). Within the file /plugins/advanced-custom-blocks-master\advanced-custom-blocks.php I changed:

<?php
if ($_GET['context'] === 'edit') {
          ob_start();
          $fields = acf_get_fields($field_group);
          acf_render_fields($fields, $attributes['post_id']);
          $output .= ob_get_contents();
          ob_end_clean();
        } else {
          $custom_block_fields_group = acf_get_fields( $GLOBALS['ACF_FIELD_GROUP_ID'] );
          foreach( $custom_block_fields_group as $field ) :
            foreach( $field as $key => $value ) :
              $current_group_fields_meta_keys[] = $field['key'] . '_' . $GLOBALS['ACF_BLOCK_ID'];
            endforeach;
          endforeach;
          $_POST['custom_block_fields_meta_keys'] = array_unique( $current_group_fields_meta_keys );
          $output = apply_filters('acf/render_block', $output, $attributes);
          $output = apply_filters('acf/render_block/name=' . $attributes['block_name'], $output, $attributes);
        }

@ATomilov
Copy link
Contributor Author

ATomilov commented Aug 28, 2018

@thedonquixotic , @rchipka continue.
and in functions.php
image

This allowed me to get the following:
image
var_dump outputs the data what I need. The thing is left for small - to set up a beautiful html. But it's not tested with more than one group on one page yet.

@rchipka
Copy link
Owner

rchipka commented Aug 28, 2018

@thedonquixotic the expected behavior was working for me when I tested it.

Although it should work as-is now, storing the data in the block's attributes would be a much more reliable solution.

@ATomilov ATomilov reopened this Aug 28, 2018
@rchipka
Copy link
Owner

rchipka commented Aug 28, 2018

@thedonquixotic I have taken a look at all of the other implementations I could find, and while some are better than others they all have limitations, like not being ACF fields or like not being repeatable, extensible, or location targetable.

@rchipka
Copy link
Owner

rchipka commented Aug 28, 2018

@ATomilov I'm glad you were able to create an implementation that works for you, but this solution is far from ideal.

Luckily storing the ACF field data inside the block attributes should be pretty straightforward.

@jcklpe
Copy link
Contributor

jcklpe commented Aug 28, 2018

@rchipka Yeah, your implementation seems to be more robust for sure. I'll do the ReadMe stuff and see if I can reproduce that other bug in the meantime.

@ATomilov
Copy link
Contributor Author

ATomilov commented Sep 3, 2018

@rchipka Hi. I understand the reason of this bug, but I can't figure out how fix it) The reason - if we use same type blocks without put they in to repeater, each next same type block has fields with same keys. And when we edit, for example, second block in admin editor it take right data because to get it we use get_post_meta, but on front-end we have value of field which overwrites last same type block. Logic of it like a field = variable.

@rchipka
Copy link
Owner

rchipka commented Sep 14, 2018

@ATomilov Currently, we hack our way around this by using key + block_id as the meta key.

Ideally we only do this as a fallback and also store the field data inside the block's attributes (which are stored hidden in post_content).

This would couple the fields with their corresponding values more tightly than the current solution.

@rchipka
Copy link
Owner

rchipka commented Sep 14, 2018

Technically, we don't have to store anything as post meta at all.

We could achieve the same result by using block attribute data alone.

The only reason to store the block data as post meta as well is so that we can still run WP_Query and mysql queries for the data.

@rchipka
Copy link
Owner

rchipka commented Oct 4, 2018

Try again with latest version

@asadrahbar
Copy link

This still only works within a repeater field. BTW, I like the new template system, my functions.php was getting crowded.

@rchipka
Copy link
Owner

rchipka commented Oct 5, 2018

@asadrahbar Which version did you try with? I recently made a change that might further help with this.

Right now I've switched using update_field on the fly for repeater rows, which is really inefficient, but it'll suffice until we can lock down the load_field hooks for repeaters.

@asadrahbar
Copy link

Still has the bug in 2.1.3. Looks like it works in a repeater field but still can't have more than one of a repeater field block on the page.

@rchipka
Copy link
Owner

rchipka commented Oct 9, 2018

Can someone with this issue please post the raw post_meta values for the post as well as the raw post_content from the database?

I cannot debug this issue without that information.

@notasausage
Copy link

Still seeing this bug in 2.1.4 on a site with only ACF and ACB plugins installed.

@rchipka
Copy link
Owner

rchipka commented Oct 18, 2018

Please try again with the version 2.1.5.

In this version I hook into get_post_meta instead of hooking into ACF's load_field hook.

This should be much more reliable.

@asadrahbar
Copy link

Still not working in 2.1.5, actually it's worse. In 2.1.4, the field data was saved properly, just not displayed. Now I add 2 blocks with different content, publish, looks fine in editor. View the page and both blocks have same content (from 2nd block). Edit the page and now both blocks have same content in editor.

post_content:

<!-- wp:acf/ma-popover {"post_id":580,"block_id":"5342d841-8b85-4071-a74d-2fb261d7177d"} /-->

<!-- wp:acf/ma-popover {"post_id":580,"block_id":"1928bc10-ab66-4f47-811e-e0cf91cd5b4d","acf_fields":{"field_5bb707dc6fdfb":"link2","field_5bb706dd287b6":"","field_5bb706dd28837":"557","field_5bb706dd288ba":"link2 title","field_5bb706dd2893b":"link2 content"}} /-->

<!-- wp:acf/ma-tabs {"post_id":580,"block_id":"f8c1a94c-80a5-4ed3-bb3c-072c4314d8a0","acf_fields":{"field_5b9f54e8ec102":["470","238"]}} /-->

ma-popover block had values of link1 and link2 when published, now both showing link2.

ma-tabs block is testing this issue

  • This block pulls in the_content of posts selected in a relationship field. One of the posts selected has a custom block to display a post grid as well as 2 copies of the popover block. The post that is pulled in displays the post grid, but ma-tabs receives no data.

Also sorry to note that popover blocks from the ma-tabs post are picking up data from the current page rather than the post called.

page id 580 - page containing tabs block
post id 470 - tab content

postmeta:

meta_id meta_id post_id meta_key meta_key meta_value meta_value
4930 580 _edit_lock 1540278645:1
4935 580 _edit_last 1
4936 580 _yoast_wpseo_content_score 30
4937 580 page_title_hide 0
4938 580 _page_title_hide field_5b997ccc4c3a2
4939 580 featured_image_hide 0
4940 580 _featured_image_hide field_5b997d014c3a3
4941 580 tab_title
4942 580 _tab_title field_5b9f53f93887a
4943 580 popover_link_text link2
4944 580 _popover_link_text field_5bb707dc6fdfb
4945 580 popover_link
4946 580 _popover_link field_5bb706dd287b6
4947 580 popover_image 557
4948 580 _popover_image field_5bb706dd28837
4949 580 popover_title link2 title
4950 580 _popover_title field_5bb706dd288ba
4951 580 popover_content link2 content
4952 580 _popover_content field_5bb706dd2893b
4953 580 tab_content a:2:{i:0;s:3:"470";i:1;s:3:"238";}
4954 580 _tab_content field_5b9f54e8ec102
4973 580 block_1928bc10-ab66-4f47-811e-e0cf91cd5b4d a:10:{s:17:"popover_link_text";s:5:"link2";s:18:"_popover_link_text";s:19:"field_5bb707dc6fdfb";s:12:"popover_link";s:0:"";s:13:"_popover_link";s:19:"field_5bb706dd287b6";s:13:
"popover_image";s:3:"557";s:14:"_popover_image";s:19:"field_5bb706dd28837";s:13:"popover_title";s:11:"link2
title";s:14:"_popover_title";s:19:"field_5bb706dd288ba";s:15:"popover_content";s:13:"link2 content";s:16:"_popover_content";s:19:"field_5bb706dd2893b";}
4974 580 block_f8c1a94c-80a5-4ed3-bb3c-072c4314d8a0 a:2:{s:11:"tab_content";s:34:"a:2:{i:0;s:3:"470";i:1;s:3:"238";}";s:12:"_tab_content";s:19:"field_5b9f54e8ec102";}
5011 580 block_5342d841-8b85-4071-a74d-2fb261d7177d a:10:{s:17:"popover_link_text";s:5:"link1";s:18:"_popover_link_text";s:19:"field_5bb707dc6fdfb";s:12:"popover_link";s:0:"";s:13:"_popover_link";s:19:"field_5bb706dd287b6";s:13:
"popover_image";s:3:"405";s:14:"_popover_image";s:19:"field_5bb706dd28837";s:13:"popover_title";s:11:"link1
title";s:14:"_popover_title";s:19:"field_5bb706dd288ba";s:15:"popover_content";s:13:"link1 content";s:16:"_popover_content";s:19:"field_5bb706dd2893b";}

@signaturesteve
Copy link

same, gonna have to move off this plugin for an alternative.

@rchipka
Copy link
Owner

rchipka commented Oct 23, 2018

@asadrahbar You need to make sure to allow the separate Ajax save to happen on each field group. Thank you for providing helpful details and debugging information.

@signaturesteve This open source software is offered free with absolutely no guarantees. Seems like it might be best to wait for the official ACF release instead, where you can get customer support without taking things into your own hands.

I have no intentions on providing customer support for this plug-in. It works for me.

It's open source so that we can all take a look at these issues and share an effort in figuring them out.

I laid the foundation to make this happen, and I'll help fix bugs, but again, please do your due diligence by providing details and attempting to research it yourself first.

@asadrahbar
Copy link

Thank you for building this awesome plugin before ACF implements what they said they would! Can you tell me how to allow the separate Ajax save? I found this but not sure if it's what I need or how to adapt it for use.

@rchipka
Copy link
Owner

rchipka commented Oct 24, 2018

@asadrahbar @notasausage @signaturesteve Looks like I forgot to commit a change that should have been in 2.1.5. Sorry about that.

I've published v2.1.6 that contains this change, which is essential for multi-block functionality.

@rchipka
Copy link
Owner

rchipka commented Oct 24, 2018

I have this exact code for v2.1.6 running on a test site (with latest Gutenberg and ACF) and everything is working well.

There are some quirks with the AJAX save when adding new ACF blocks. If you run into this, my recommendation for right now is to save the post after adding another block and refresh the post edit page.

I'll need to add even more aggressive AJAX saving for ACF block changes. The problem with that is the block temporarily changes to an uneditable "saving" view.

@signaturesteve
Copy link

got an alert for that message ^,

just an example... if you create a custom block and have a post object field in it, then try and create 2 on a page - you are unable to set them to seperate items and have them both work individually on a page. they both use the same post object,

@rchipka
Copy link
Owner

rchipka commented Oct 25, 2018

@signaturesteve can you try with another field type?

@asadrahbar
Copy link

Still have the issue, but now it is saving the fields correctly on the edit screen. But both blocks pick up content from the second block when displayed on the page. Interestingly, it's not the second block added, but the second block on the page - changing the block order changes the output. I refreshed the edit screen after adding and updating each block. Also, after deleting the old blocks and adding a new one, it's pre-populated with old content.

Published page

post_content:

<!-- wp:acf/ma-popover {"post_id":580,"block_id":"0f5b0d73-9468-40bf-81f4-a7e877913a57","acf_fields":{"field_5bb707dc6fdfb":"Pop link 2","field_5bb706dd287b6":"","field_5bb706dd28837":"575","field_5bb706dd288ba":"Link 2 title","field_5bb706dd2893b":"link 2 content"}} /-->

<!-- wp:acf/ma-popover {"post_id":580,"block_id":"2c98e6ba-f51b-4941-86da-03de812f788c","acf_fields":{"field_5bb707dc6fdfb":"Pop link 1","field_5bb706dd287b6":"","field_5bb706dd28837":"405","field_5bb706dd288ba":"Link 1 title","field_5bb706dd2893b":"link 1 content"}} /-->

postmeta:

meta_id
post_id
meta_key
meta_value
4930
580
_edit_lock
1540573543:1
4935
580
_edit_last
1
4936
580
_yoast_wpseo_content_score
30
4937
580
page_title_hide
0
4938
580
_page_title_hide
field_5b997ccc4c3a2
4939
580
featured_image_hide
0
4940
580
_featured_image_hide
field_5b997d014c3a3
4941
580
tab_title

4942
580
_tab_title
field_5b9f53f93887a
4943
580
popover_link_text
Pop link 1
4944
580
_popover_link_text
field_5bb707dc6fdfb
4945
580
popover_link

4946
580
_popover_link
field_5bb706dd287b6
4947
580
popover_image
405
4948
580
_popover_image
field_5bb706dd28837
4949
580
popover_title
Link 1 title
4950
580
_popover_title
field_5bb706dd288ba
4951
580
popover_content
link 1 content
4952
580
_popover_content
field_5bb706dd2893b
4953
580
tab_content
a:2:{i:0;s:3:"470";i:1;s:3:"238";}
4954
580
_tab_content
field_5b9f54e8ec102
4973
580
block_1928bc10-ab66-4f47-811e-e0cf91cd5b4d
a:10:{s:17:"popover_link_text";s:5:"link2";s:18:"_popover_link_text";s:19:"field_5bb707dc6fdfb";s:12:"popover_link";s:0:"";s:13:"_popover_link";s:19:"field_5bb706dd287b6";s:13:"popover_image";s:3:"557";s:14:"_popover_image";s:19:"field_5bb706dd28837";s:13:"popover_title";s:11:"link2 title";s:14:"_popover_title";s:19:"field_5bb706dd288ba";s:15:"popover_content";s:13:"link2 content";s:16:"_popover_content";s:19:"field_5bb706dd2893b";}
4974
580
block_f8c1a94c-80a5-4ed3-bb3c-072c4314d8a0
a:2:{s:11:"tab_content";s:34:"a:2:{i:0;s:3:"470";i:1;s:3:"238";}";s:12:"_tab_content";s:19:"field_5b9f54e8ec102";}
6343
580
__block_f8c1a94c-80a5-4ed3-bb3c-072c4314d8a0
a:2:{s:11:"tab_content";s:34:"a:2:{i:0;s:3:"470";i:1;s:3:"238";}";s:12:"_tab_content";s:19:"field_5b9f54e8ec102";}
6344
580
__block_1928bc10-ab66-4f47-811e-e0cf91cd5b4d
a:10:{s:17:"popover_link_text";s:5:"link2";s:18:"_popover_link_text";s:19:"field_5bb707dc6fdfb";s:12:"popover_link";s:0:"";s:13:"_popover_link";s:19:"field_5bb706dd287b6";s:13:"popover_image";s:3:"557";s:14:"_popover_image";s:19:"field_5bb706dd28837";s:13:"popover_title";s:11:"link2 title";s:14:"_popover_title";s:19:"field_5bb706dd288ba";s:15:"popover_content";s:13:"link2 content";s:16:"_popover_content";s:19:"field_5bb706dd2893b";}

@rchipka
Copy link
Owner

rchipka commented Oct 26, 2018

@asadrahbar In order for it to work at all, there must be a post_meta key with ___block_{block_id} where block_id matches one of the block_id's in the post_content.

It appears the post_content has block ids 0f5b0d73-9468-40bf-81f4-a7e877913a57 and 2c98e6ba-f51b-4941-86da-03de812f788c, neither of which seem to have a corresponding __block_{block_id} post_meta entry.

Since you have some post_meta in there from older versions, I recommend creating a new post and trying again from scratch.

@rchipka
Copy link
Owner

rchipka commented Oct 26, 2018

@asadrahbar The only reason that it appears to be working now is that it will be forced to fall back to the real field values for each field on the post, which will always be set to the last saved ACF Block's field values.

Without a corresponding post_meta entry, the plugin has no way to efficiently override the field values for that specific block.

The post meta entries for each block happen using the block attributes during the block's AJAX save call (instead of during rendering) in order to have better performance during block render.

An AJAX save must occur on each block in order for things to work correctly.

You'll know the AJAX save is working if your ACF Block looks like this at some point:

screen shot 2018-10-26 at 1 39 09 pm

If the AJAX save is not working, check your browser's Javascript console for any errors.

@rchipka
Copy link
Owner

rchipka commented Oct 26, 2018

@asadrahbar If you could provid a temporary login to this site, I wouldn't mind taking a look at what's going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants