Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs

This blocks 📌 [later phase] [PP-2] Prepare for multiple component types: prefix Component config entity IDs with `sdc` Postponed .

I want to pick this up but before that, can I get some doubts:

1. Do we need to make changes to ComponentTreeStructure so that the said format of config are generated?
2. All of these changes are only resticted to the field ComponentTreeItem's tree prop?
3. To make it work in the existing system locally, I will just need to create a layout using these said components?

Yay, thank you, @abhisekmazumdar! 😊

1. No: zero changes are necessary to that. Only docs changes, like this:
   

   diff --git a/src/Plugin/DataType/ComponentTreeStructure.php b/src/Plugin/DataType/ComponentTreeStructure.php
   index afbc656d..7685c241 100644
   --- a/src/Plugin/DataType/ComponentTreeStructure.php
   +++ b/src/Plugin/DataType/ComponentTreeStructure.php
   @@ -56,6 +56,8 @@ use Drupal\Core\TypedData\TypedData;
     * @see \Drupal\experience_builder\Plugin\Validation\Constraint\ComponentTreeStructureConstraintValidator
     * @see \Drupal\Tests\experience_builder\Kernel\DataType\ComponentTreeStructureTest
     *
   + * @phpstan-type ComponentConfigEntityId string
   + *
     * @todo Implement ListInterface because it conceptually fits, but … what does it get us?
     */
    #[DataType(
   @@ -243,7 +245,10 @@ class ComponentTreeStructure extends TypedData {
       * @param string $component_instance_uuid
       *   The UUID of a placed component instance.
       *
   -   * @return string
   +   * @return ComponentConfigEntityId
   +   *   A Component config entity ID.
   +   *
   +   * @see \Drupal\experience_builder\Entity\Component
       */
      public function getComponentId(string $component_instance_uuid): string {
        if (!in_array($component_instance_uuid, $this->getComponentInstanceUuids(), TRUE)) {
   @@ -257,7 +262,7 @@ class ComponentTreeStructure extends TypedData {
      }
    
      /**
   -   * @return array<string>
   +   * @return array<ComponentConfigEntityId>
       */
      public function getComponentIdList(): array {
        return array_values(array_unique(array_column($this->getComponents(), 'component')));
   
   

2. No, it also affects config, in particular: config/optional/field.field.node.article.field_xb_demo.yml. But it's the exact same change, because both content and config are validated by \Drupal\experience_builder\Plugin\Validation\Constraint\ComponentTreeStructureConstraintValidator(). That is the file you'll have to make logic changes in. See the first paragraph of https://wimleers.com/xb-week-12 for context.
3. No, "just" doing that using the UI is impossible until all the code has been updated first 😅 I recommend starting with updating the expectations in ComponentTreeStructureTest to match what I've described in the issue summary.

D'oh, I guess this issue interestingly sits at the intersection of and : it's an update to the data model to correctly use future config management stuff.

Undoing what I did in #4 🙈

This makes sense to me. I will make these changes.

🎉 Thank you! :)

This also blocks 📌 Decorate the SDC plugin manager and allow components defined in code Active .

abhisekmazumdar → changed the visibility of the branch 3469609-component-tree-component-config-entity-ids to hidden.

abhisekmazumdar → changed the visibility of the branch 3469609-component-tree-component-config-entity-ids to active.

I will need some more help here. This is what I understand so far:

1. I'm able to stop the debugger for ComponentTreeStructure and see the different structure for the components which now don't have the sdc names.
2. In the WIP MR, I have made the changes suggested in #6(Point 1) and description.
3. I export config and compare the newly created YAML for field.field.node.article.field_xb_demo, which doesn't have much of a difference in the config nor for the default content.
4. I can run phpunit -c core modules/contrib/experience_builder/tests/src/Kernel/DataType/ComponentTreeStructureTest.php and see it all green.
5. I understand that I still need to make changes to Constraint\ComponentTreeStructureConstraintValidator but I'm not sure how and where.

This is what I need to understand:

1. How can I ensure that what I'm doing for the ComponentTreeStructureTest is correct? I don't fully understand the tests.
2. Again, what changes will be needed for ComponentTreeStructureConstraintValidator, and how can I debug it?

I also understand this is a critical issue, and unassigning this from me. If someone already has the experience to do it quickly, please take it over.
I will pick it up if I get my answer or figure it out.

#14:

1. That config is not in the MR? You can't just export it though, you'd need to manually modify that default config right now.
2. The only reason that test passes is because you haven't updated the validator yet (next point). 😄
3. This is the crucial next step. 😊

Besides updating the validator, the other crucial next step is to update the logic in the XB field type and the hydrated computed property's logic due to the tree field property now containing component config entity IDs instead of SDC plugin IDs. Did that for you in https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... 👈 Thanks to this commit, as soon as you update the default config (#14.3), everything should render once again. But you probably need to disable the validator temporarily, until you've updated its logic, because that will (should) complain about invalid config.

WRT understanding:

1. These are tightly scoped tests. They test only a single validator. I'm confident you can make sense of them 😊 Tip: the names (keys) of the test cases returned by the provider describe what the test case is testing.
2. I've made one example update to the test cases in https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... — all other test cases must be updated similarly. You can see that this is exactly like the changes to docs/data-model.md. You'll see that now the test starts to fail, so it points to how to validator logic will need to be updated :)

🏓 Back to you — you can do it! 😊

Thank you, @Wim Leers, for the detailed input & believing in me 😁

🏓 The MR is still a work in progress, so it is not completely ready for review. However, I seek some answers to the questions I have asked over the MR. Really appreciate your help.

I do totally believe in you! 😄

Also, I very much messed up the sample commits and issue summary 🙈

Issue summary fixed, and mistake rectified on the MR: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2....

Done:

• Made all the suggested changes to the best of my knowledge.

Todo:

1. The ComponentPropsForm is not working for the above said reasons.
2. I see the CI is failing for phpcs which are unrelated to this MR.
3. The Cypress tests are also failing for which I'm not sure about.

Please review and give feedback.

I will check and rebuild the XB with a fresh setup. Some of the outstanding TODO have been fixed:

Remaining TODO:

1. I see the CI is failing for phpcs which are unrelated to this MR.
2. The Cypress tests are also failing for which I'm not sure about.

For these, I still need feedback.

Wow those phpcs failures are super weird! 🤪

The first next step: making the phpunit tests pass — they have many failures at the moment: https://git.drupalcode.org/issue/experience_builder-3469609/-/jobs/2697530

Those need to pass first, before the UI will be able to work and before the Cypress E2E tests can possibly pass 😊

I tried setting up the xdebugger on my local machine to make it work with the current local setup I have. Setting up the xdebugger correctly will give me a much clearer idea of what is breaking during the test.
Yet I was unable to make Xdebugger work for the unit test cases.

Okay, I was successfully able to make the debugger work. It works out of the box, but I need to click the continue button one more time to stop it at the required mark.

@Wim Leers

I see mostly the \Drupal\Tests\experience_builder\Kernel\DataType\ComponentTreeStructureTest::testValidation is creating problem. For our case should we just update providerValidation data to match it with what its actually trying to assert?

Or should I be looking at why the assertion is breaking ?

Thanks for pushing this forward, I'll take a look at where you got the MR 😊

(Started with merging in upstream, thanks to 📌 Update default config to make a fresh install result in an XB UI with an empty canvas Fixed landing yesterday, this MR now has to make fewer changes, but it required a conflict to be resolved.)

#25 was accurate, yet incomplete: more changes are necessary. The ComponentTreeStructure validator needed to be updated too, for example: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2.... That alone fixes a number of failures.

Done for the day. @abhisekmazumdar, could you push this across the finish line? 😊🙏

Overall this is the way I think we need to go but Wim's comments and my comments need addressing.

Thanks, @longwave!

I think anybody could tackle the remaining feedback :)

Addressed all feedback.

Saving attribution.

Automatically closed - issue fixed for 2 weeks with no activity.