SDCs with optional images without examples cannot be placed

I don't see why we need this if 📌 Handle components without HTML wrappers Active is supposed to provide a general solution? 🤔 What am I missing?

📌 Handle components without HTML wrappers Active improves the preview canvas only.

This issue fixes the component settings form specifically for components containing optional images with no examples, which currently throw an error - the component settings cannot be changed at all.

This makes the component inputs form more robust by ignoring violations entirely - previously we only asserted on them, but the fact we try to catch violations at all makes ::clientModelToInput() act in a subtly different way for default/null values.

image-optional-without-example now fails with

An exception has been thrown during the rendering of a template ("[xb_test_sdc:image-optional-without-example/image.src] The property src is required.").

which I believe is in the remit of 🌱 [META] Robust component instance error handling Active - an SDC might change on us at deploy time and this sort of thing could happen there too.

which currently throw an error - the component settings cannot be changed at all.

I see! 🤪

Provide a system wide default/fallback image for SDCs without examples

👎 — that opens a whole new can of worms.

Mark SDCs with optional images and no examples as invalid, so they do not appear in the component library

Why do we need an example/default value if the image prop is declared optional? I

      if ($prop_source_array === NULL) {
        // The client didn't send this prop but should. This is an error OR the
        // data has been tampered with.
        throw HttpException::fromStatusCode(Response::HTTP_BAD_REQUEST);
      }

is still correct: the client must send the information that instructs the ComponentInputsForm on what field type + widget to use.

And, fortunately, the test coverage 📌 ComponentSource robustness: add `ComponentSourceTestBase::testSettings()` Active is adding DOES prove this works correctly: the image field type is used for this SDC:

      'sdc.xb_test_sdc.image-optional-without-example' => [
        'image' => [
          'field_type' => 'image',
          'field_storage_settings' => [],
          'field_instance_settings' => [],
          'field_widget' => 'image_image',
          'default_value' => [],
          'expression' => 'ℹ︎image␟{src↝entity␜␜entity:file␝uri␞␟url,alt↠alt,width↠width,height↠height}',
        ],
      ],

Conclusion: either the bug is in the ::getClientSideInfo() logic, or in the client.

So then next steps should AFAICT be:

1. We should be able to add an explicit test for this to verify that at least the server sends the correct information to the client, by adding a ::testGetClientSideInfo().

   (That is 📌 Expand `::getClientSideInfo()` test to all ComponentSource plugins Active was intended for. We don't need to be blocked on it though — we can add it only to \Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponentTest in this issue, and leave generalizing it to all component sources for #3518832.)

2. Hopefully that will prove that this is a server-side bug. Otherwise, we'll know for a fact it's actually a bug on the client side.

With 📌 ComponentSource robustness: add `ComponentSourceTestBase::testSettings()` Active in, 📌 Expand `::getClientSideInfo()` test to all ComponentSource plugins Active can now be worked on, and will help pave the path for this.

FYI I asked @isholgueras to prioritize the bit in #3518832's MR that would answer #9.

This is a bug in the client. Steps to reproduce:

1. Add "XB test SDC with optional image and heading" to a page. Observe this being sent in the model:

{
	"model": {
		"f38a2473-8c11-4ab9-b93e-18657837aae9": {
			"name": "XB test SDC with optional image and heading",
			"resolved": {
				"heading": [],
				"image": {
					"alt": "A good dog",
					"height": 402,
					"src": "gracie.jpg",
					"width": 601
				}
			},
			"source": {
				"heading": {
					"expression": "ℹ︎string␟value",
					"sourceType": "static:field_item:string",
					"value": []
				},
				"image": {
					"expression": "ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}",
					"sourceType": "static:field_item:entity_reference",
					"sourceTypeSettings": {
						"instance": {
							"handler": "default:media",
							"handler_settings": {
								"target_bundles": {
									"image": "image"
								}
							}
						},
						"storage": {
							"target_type": "media"
						}
					},
					"value": []
				}
			}
		}
	}
}

2. Change the heading to "test". Observe the model changes:

{
	"model": {
		"resolved": {
			"heading": "test",
			"image": {
				"alt": "A good dog",
				"height": 402,
				"src": "gracie.jpg",
				"width": 601
			}
		},
		"source": {
			"heading": {
				"expression": "ℹ︎string␟value",
				"sourceType": "static:field_item:string",
				"value": "test"
			},
			"image": {
				"expression": "ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}",
				"sourceType": "static:field_item:entity_reference",
				"sourceTypeSettings": {
					"instance": {
						"handler": "default:media",
						"handler_settings": {
							"target_bundles": {
								"image": "image"
							}
						}
					},
					"storage": {
						"target_type": "media"
					}
				},
				"value": {
					"alt": "A good dog",
					"height": 402,
					"src": "gracie.jpg",
					"width": 601
				}
			}
		}
	}
}

Note that the source and resolved values for the heading have changed, as expected. But note also that the source value for the image has also been changed - but this is not correct in two ways: the source prop value is expected to be an entity ID, and we shouldn't have any value here at all when it is the default value.

Although actually this behaviour appears to be expected as per the comment in inputBehaviors.tsx?

          // Set the value from resolved values. This might duplicate the value
          // in the resolved key for components where the source and resolved
          // values are the same, however this method is generally called before
          // a patchComponent request to Drupal which will remove values from
          // the source key if it duplicates the resolved value. So for a simple
          // component with e.g. a string property, we would have duplication
          // here but this would be removed from the model returned from Drupal
          // during patchComponent and hence the model stored in the redux store
          // after this request. For a component with an expression such as an
          // image component - at this point both resolved and source may be a
          // media entity ID. When patchComponent is called in that instance,
          // Drupal will retain the media entity ID in the source value, but
          // return the evaluated expression for the resolved values - e.g. this
          // might be the src, alt, height and width for the media entity.

So perhaps this is expected from the client and we are filtering incorrectly on the server side, because the source value is considered invalid?

Re #9 I realise I made a typo in the IS; I meant

Mark SDCs with required images and no examples as invalid, so they do not appear in the component library

I am going to go ahead with this approach in this MR which will solve the test failure.

The MR is currently failing for optional images without examples, investigating.

So, fixed that, but now the test runs into a fundamental SDC problem:

1) Drupal\Tests\experience_builder\Kernel\ApiLayoutControllerPostTest::testImageComponentPermutations with data set "1" ('image-optional-without-example', '')
Twig\Error\RuntimeError: An exception has been thrown during the rendering of a template ("[xb_test_sdc:image-optional-without-example/image.src] The property src is required.").

We have an optional image, with no example. But the src property of the optional image is required, and this is enforced by ComponentValidator.

This is actually checked twice, once in GeneratedFieldExplicitInputUxComponentSourceBase::validateComponentInput() - although in this MR we now ignore validation errors here when trying to render the form - but we still can't actually render the Twig template, because it calls the validator in the same way (directly from the compiled template, via the Twig extension) which blows up with the same exception at render time.

So I think the choices we have here are:

1. Mark SDCs with optional images and no examples as invalid, so they do not appear in the component library
2. Allow SDCs that fail rendering (such as image-optional-without-example, when it has no image set) to still be placed and be interacted with, but the preview will not appear until any validation issues are fixed

Went with #17.2; when the image-optional-without-example fails to render we catch that at render time and currently output nothing at all. This allows it to be placed, and as soon as an image is actually selected then it starts to work.

I am not sure what the expected behaviour is of an SDC with optional image prop that contains other markup; as far as I can see SDC will just refuse to render it if the image prop is not present even if it's marked optional.

The bug is most definitely server side.

Unfortunately, neither of the options considered in #17 🐛 SDCs with optional images without examples cannot be placed Active is the correct behavior. 😔 It must be possible to have optional images without a default value. That is a completely valid state and configuration to have since the image is optional. Based on these requirements, image-required-without-example was already added as a valid configuration (and has been documented as such in image-optional-without-example.component.yml). The src should be only required when an image is provided.

This seems at least somewhat related to 🐛 Default image is not loaded after removing media Active .

Added initial test coverage for this specific case, we had no PHP test coverage for ComponentInputsForm at all until now.

I don't understand the Cypress test fail, locally the checkbox checks and unchecks for me with no errors.

1. ✅ Manual testing confirms the fix!
2. ✅ Test-only CI job fails as expected.
3. ❌ The hardcoded assumptions/expectations might change in reality without us knowing ⇒ additional test coverage necessary (better still: rely on end-to-end tests, but that's not pragmatic today)

Worked with @longwave on the MR — we're much better set for future regressions/hardenings in this area now! 👍