Formatters for empty fields do not render with layout builder enabled

The setting has been moved to the block settings.
Schema fix.

Fixed wrong default value.

As a bug this will need test coverage.

Also since we are adding a new configuration "hide_for_empty" think we may need an upgrade path.

Definitely a change record for the new setting though.

I've replaced the latest patch with a merge request (with some fixes).

rerolled for 10.2.7

Creating a patch for 10.2.x

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, 10.2 is in security mode now.

Was previously using patch from #30.
After upgrading to 10.3.2, tried patch from #42. Didn't work, the values were not being saved.

After evaluating the differences between the three, #43 has the schema definition at formatter level and uses the config from the formatter key. #30 which was previously working, also uses the config from the formatter key.

#42 and the MR have the schema one level higher. Because of this, the values don't get saved.

Patch #43 works as expected.

First, I just want to note that we should not be iterating on patches since we're using an MR. Any work and iteration should happen in the MR.

Second, I'd propose that the logic of this new field be flipped: instead of "Hide when empty" and defaulting to checked, use "Show when empty" and default to unchecked. I will add an MR with this alternate approach.

MR !12052 refactors original approach to use a "show block when field is empty" checkbox rather than a "hide for empty" checkbox. This MR also includes updates to tests to test the new option.

As a result, I'm going to mark as Needs review and remove the needs tests tag.

My instinct is to also hide MR !5111, but I'll leave that to a maintainer.

New configuration key will need an upgrade path + upgrade path test

@smustgrave -

Sorry, but I want to make sure I'm understanding you correctly.

Currently the FieldBlock's default configuration has a value for the new config field that matches the FieldBlock's behavior pre-patch.

Are you saying that we need an update hook to update all FieldBlocks to set a value for the new config option and then a test to make sure update hook works properly? Or something else?

If this key could appear on any FieldBlock then it would need an upgrade path. Good rule of thumb if you go into a config page, make no changes, click save, config export and there is a change then it needed an upgrade path.

and then a test to make sure update hook works properly

That's correct, there are a few example in core but don't have them super handy right now.

Thank you, I understand now.

Note that the update hook should probably only target the default layouts (the entity view displays).

There could be thousands+ of nodes/entities with overridden layout data that have field blocks. Writing an update hook to load, check the nodes overridden layout for field blocks, then resave the nodes does not sound realistic. As long as the behavior for a field block that does not have any value for the new property is the same as the new property having empty value, I think this should be fine.

I added an update hook that updates FieldBlocks to set show_when_empty to FALSE, but I'm not sure if there's more I need to do there. See my previous note about whether more is needed.

But I am not sure how to go about writing a test for the update hook.

RemovePathKeyTest may be a good example

Essentially the test would be

Load a fieldBlock and verify the key DOES NOT exist

Run updates

Load the same block and verify the key DOES exist.

Thank you @smustgrave, I think I see how it could be done now. But I have run out of time in the short term, I'll try to get back to this to add the test of the upgrade path sooner rather than later.