🇪🇸Spain @isholgueras

isholgueras → made their first commit to this issue’s fork.

I'm going to start working on this.

wim leers → credited isholgueras → .

I'm Ok with this. Chris has been helping me with the latest releases and he's taking care of the issue queue better than me.

Isn't this a duplicate of 🐛 Disable additionalProperties in slots, props in SDC json schema Active : Disable additionalProperties in slots, props in SDC json schema from core?

I think it is. The validation is done in the ComponentValidator.php in core. Slot names don't seem to be validate.

Confirmed you can create slot with emojis in the name,

and this breaks the site.

Note: breaks the site but you can remove the component and everything works

isholgueras → made their first commit to this issue’s fork.

In the ticket 📌 Provide granular permissions for pages Active there were these 3 permissions (create, edit and delete).

As I didn't have any option for overview or a general "administer", I could only use "edit xb_page" as the permission to use whenever a editor wants to edit it.

Maybe I was mistaken or maybe it's not clear.

I've:

• - Changed all string literal for permissions
• - Replaced them by their own constants
• - Add a reference in the file for a better developer experience
• - Add a new constant for the permission administer components

isholgueras → created an issue.

I've updated the README.md and the CONTRIBUTING.md to match the new requirements (Drupal >= 11.1.2) and to include better information about the ddev-drupal-xb-dev ddev addon.

What I don't know is if we should include sections like:

1. Prerequisites: to include it requires php8.3, Drupal 11.1.2, ...
2. Code structure overview: To explain how's the code structure (tests, tests modules, ui, ...)
3. Contribution workflow: I assume this is implicit in any Drupal contribution, but we can add a specific section about the usage of phpunit, phpstan, ... It's, more or less, added to both sections, but maybe something clearer.
4. Development best practices: Besides of the "use dependency injection,..." type of things, practices or contracts made in this specific project. Maybe like ADRs but not that strict.
5. Links to other projects related with XB?

Any other thing to add things to add?

isholgueras → created an issue.

Can we use 📌 Introduce unit test coverage for both ComponentSource plugins (Block + SDC) Active to use xb_test_block as blocks to update?

Tests refactored to @depends to speed them up and make it quicker.

I've tested it and it works well for me, test included.

The only issue I've found, that I guess only affects my environment and, even though, is not important as it's a test module is that I can't uninstall the module:

╰─ ddev drush pmu xb_test_article_fields                                                                            ─╯

In ExceptionHandler.php line 52:
                                                                                                                      
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual tha  
  t corresponds to your MariaDB server version for the right syntax to use near 'LIMIT 1 OFFSET 0' at line 5: SELECT  
   1 AS "expression"                                                                                                  
  FROM                                                                                                                
  "node_revision__field_xbt_path" "t"                                                                                 
  WHERE                                                                                                               
  LIMIT 1 OFFSET 0; Array                                                                                             
  (                                                                                                                   
  )                                                                                                                   
                                                                                                                      

In StatementWrapperIterator.php line 113:
                                                                                                                      
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual tha  
  t corresponds to your MariaDB server version for the right syntax to use near 'LIMIT 1 OFFSET 0' at line 5          
                                                                                                                      

I'll let others to test in case they also have the same issue with the uninstall.

Note: I haven't reinstalled the site.

I think I've finished the new permissions, replace administer xb_page by access content , create xb_page , edit xb_page and delete xb_page.

What I've done is adapt the permissions and review the tests that were already done, by maybe is something else needed to do.

Hi @wim-leers,

I've added the first draft of the BlockComponentTest. To do so, I've created a xb_test_block to populate a basic block and I'm testing the plugin id and the cache tags just to see if this is the correct approach for this test. If so, I'll continue adding new tests to it.

Thanks!

It is not possible to uninstall this module when there's a config entity depending on it. Did you force the uninstallation using drush, perhaps?

Yes, exactly. I've used drush. You know, when your developing a module you could go back and forth with the installations and you could uninstall something by mistake, or an incomplete or unstable config,...

assert() only is executed on dev environments, and probably only when writing PHP code, not when developing SDCs.

Yes, I've commented the asserts (to mimic a non dev environment) and I get the typical errors of "Unable to get ->requiresExplicitInput() from null" and so forth. But couldn't reach that try.

That's right. I'll try to investigate it as a SDC developer POV :). Let's see how far I can get.

Thanks for the guidance!

See the GIF in #3508326: Display "Dynamic Components" in top bar popover 😊

Oh! there it is :D. Thanks!

I wanted to start with this issue but I'm unable to create a Block under XB.

Is there an option within the UI or should be done in a custom code/module?

Thanks!

Ok, after a deep search, I think it's currently well handle with 2 asserts.

I've created a custom module (nacho) with one simple component:

// nacho/components/nacho-component.twig
It's me, {{ text }}!

// nacho/components/nacho-component.component.yml
$schema: https://git.drupalcode.org/project/sdc/-/raw/1.x/src/metadata.schema.json
name: Nacho component
props:
  type: object
  properties:
    text:
      type: string
      title: Name
      description: The name to show.
      examples: ['Nacho']

After clearing cache, the component appears in the admin UI, so I add this component to the page, add a name and save it.

The /node/1 page looks good.

Then I uninstall nacho module and I get 2 errors in 2 different pages, the /node/1 and /xb/node/1/editor. Don't know if expected, but 2 errors because the source of the component no longer exist.

The error in /node/1 is triggered in `ComponentTreeHidrated.php:48`.

      $component_id = $tree->getComponentId($uuid);
      $component = Component::load($component_id);
      assert($component instanceof Component);

$component is null so the assert throws an error.

The second error, happens in /xb/node/1/editor, and is triggered by ApiLayoutController.php:192

      $source = $tree->getComponentSource($component_instance_uuid);
      \assert($source instanceof ComponentSourceInterface);
      if ($source->requiresExplicitInput()) {
        $model[$component_instance_uuid] = $source->inputToClientModel($source->getExplicitInput($component_instance_uuid, $item));
      }

$source is null so the assert throws an error.

Even though, I still couldn't reproduce the initial issue. The createInstance method is well covered by the asserts. I've tested Kernel and Unit tests and I'm still unable to mock a scenario where this Exception happen in the experience_builder module.

So at this point I think is up to you (maintainers). Ignoring it and including it for an edge case, and I believe that this edge case ever happen will be easily identified and solved.

I think can see a way to reproduce: install module foo containing SDC bar. XB will create a Component config entity for foo:bar if it meets XB's requirements. Then delete the foo:bar SDC from the filesystem, as if you're actively developing it.

Makes sense. I'll try to reproduce it and if so, I'll make a test.

Thanks!

how did you end up in this situation? 🤔 🙏

I was unable to reproduce again. It happened after a couple of fresh install ddev xb-setup --force and visiting the first time the xb/node/1. I've got an error and I saw the ComponentNotFoundException in the dblog, but only once.

Since then, I'm unable to reproduce, even with some specific KernelTest with an unstable state. Maybe My installation was corrupted and the component listed wasn't the same as the components in the cache (from... the previous installation 🤔?) Don't know.

Also, I saw PhpStorm reporting that these 2 calls have an Unhandled exception:

So these were the 2 "ways" I've ended with the exception.

Maybe we can ignore this try/catch until we can find a reliable and stable test.

isholgueras → created an issue.

Rerolled

isholgueras → made their first commit to this issue’s fork.

Tested and works fine. Thanks! Ready for the 4.0.22 version.

isholgueras → made their first commit to this issue’s fork.

Thanks for the work. Merged into 4.x and ready for the 4.0.22

isholgueras → made their first commit to this issue’s fork.

Hi @capysara,

I cannot reproduce the issue. I've set the bg_color and fg_color and all works well. Can you give more steps to reproduce?

hi @cweiske,

Drupal 9 end of life was November 2023 and Drupal 10 requires PHP 8.1+. We can't support outdated versions.

I suggest you to upgrade to Drupal 10 or stick environment indicator to 4.0.19, if that version works for you.

I hope you can understand.

Thanks!

I've tested in in a Drupal 10.3, sharethis version 3.0.2 and #2 fixes the issue. RTBC for me.

Ups, I've seen it's fixed in 1.1.2. Thanks!

isholgueras → created an issue.

It's strange, because if I run:

ddev drush ev "
\$config = \Drupal::configFactory()->getEditable('environment_indicator.settings');
\$config->clear('version_identifier')->save();
"

and

ddev drush ev "\Drupal::keyValue('system.schema')->set('environment_indicator', 8000);"

in branch 4.x, the site breaks. After I run drush updb all works.

If I run both commands in the branch of MR83, the site works well, and if I run drush updb the update prompts well, runs and everything works well.

I'm going to merge MR83 and release it in the 4.0.21.

Thanks all for the work and feedback. Much appreciated!

I was able to reproduce the issue by running the following command to delete the config:

drush ev "
\$config = \Drupal::configFactory()->getEditable('environment_indicator.settings');
\$config->clear('version_identifier')->save();
"

And this happens because the module was updated but the environment_indicator_update_8101 didn't run.

I've created a new MR and I prefer to set a default value in case the config is not set. It seems that the problem is that the hook_update_N is not working well, but I think this will fix the issue safely.

By default I've added the same values we have in the environment_indicator.settings.yml

Maybe I'm missing something but I don't know how this patch could solve the issue.

For the value for version_identifier could be environment_indicator_current_release
https://git.drupalcode.org/project/environment_indicator/-/blob/4.x/conf...

The available options for the version_identifier are:
- environment_indicator_current_release
- deployment_identifier
- drupal_version
- none

https://git.drupalcode.org/project/environment_indicator/-/blob/4.x/src/...

but not environment_indicator.current_release

I'm also unable to reproduce the error.

From what version are you updating? 4.0.19 or earlier? What error do you get when you update from 4.0.19 to 4.0.20?

Thanks!

I was creating the issue for 4.0.21 until I realized you also created it.

I was wondering to if it makes sense to plan this release (maybe one more release) to be the latest release before 4.1. I wouldn't like to maintain 2 different version unless it's strictly necessary.

I would like to add to this release these issues (some leftovers from 4.0.20 release and some new)

Release-blocker issues (Bug fixes)

• 🐛 Gin vertical toolbar submenu's overlap Active

Good-to-fix issues (Good to have)

• 📌 Fix cspell issues Active
• 🐛 Coding standards and best practices Active
• 📌 Documentation guide update Needs review
• 🌱 Move toolbar integration into submodule. Active
• 🐛 Hostname field isn't a hostname but a base URL Active
• #3065135: Add support for multilingual domain sites →

There are more issues I would like to work on and add to the release, but at least this is a good amount of issues to include in 4.0.21. What do you think?

Yes, I like the idea to move it to sub modules.

Hi @trackleft2,

I've just released the 4.0.20 ( https://www.drupal.org/project/environment_indicator/releases/4.0.20 → ), but I liked this idea of track and discuss what is going to be released in the following release.

One of the things that I would like to have is only one release for all versions, Drupal 10 and 11 and Toolbar, Gin and Navigation integration.

I'm going to close this issue, as we release the 4.0.20, but I'm going to create the 4.0.21 (that could be 4.1.0 if we integrate Navigation and it's difficult to maintain) and take all tickets that was here but not merged.

Thanks for your work.

isholgueras → made their first commit to this issue’s fork.

I'm going to add this to the 4.0.20 release. Thanks for the work

Excelent work. I'll release it in the 4.0.20 version.

isholgueras → made their first commit to this issue’s fork.

fjgarlin → credited isholgueras → .

Very good addition. Thanks!

isholgueras → made their first commit to this issue’s fork.

isholgueras → made their first commit to this issue’s fork.

I've tested in local and I haven't seen any issue with an existing site or a new installed site. Thanks!

isholgueras → made their first commit to this issue’s fork.

In addition to the Tess's tests, when it's shown its sorted correctly. The config switcher exported also exports correctly the weight. Thanks for the work!

isholgueras → made their first commit to this issue’s fork.

Maybe I'm testing it wrong, but if I export, enabling the toolbar integration, the config `drush cex` or get only this settings `drush config:get environment_indicator.settings` I always get the following:

11:35:43 isholgueras@AidanMini drupal10 ddev drush config:get environment_indicator.settings
_core:
  default_config_hash: Bi0EyyiH6m5wpHRfxlKhqTIhAZayhRQudheDzAcrotU
toolbar_integration:
  - toolbar
favicon: true

I'm I missing something?

This is the current version I'm using in the ddev environment I use to test it:

11:34:25 isholgueras@AidanMini drupal10 ddev drush status
Drupal version   : 10.3.6                                     
Site URI         : https://drupal10.ddev.site                 
DB driver        : mysql                                      
DB hostname      : db                                         
DB port          : 3306                                       
DB username      : db                                         
DB name          : db                                         
Database         : Connected                                  
Drupal bootstrap : Successful                                 
Default theme    : olivero                                    
Admin theme      : claro                                      
PHP binary       : /usr/bin/php8.1                            
PHP config       : /etc/php/8.1/cli/php.ini                   
PHP OS           : Linux                                      
PHP version      : 8.1.29                                     
Drush script     : /var/www/html/vendor/bin/drush             
Drush version    : 12.5.3.0                                   
Drush temp       : /tmp                                       
Drush configs    : /var/www/html/vendor/drush/drush/drush.yml 
Install profile  : standard                                   
Drupal root      : /var/www/html/web                          
Site path        : sites/default                              
Files, Public    : sites/default/files                        
Files, Temp      : /tmp   

I agree, The delete process works well with the default `EntityDeleteForm`

isholgueras → made their first commit to this issue’s fork.

Thanks for catching it. Merged to 4.x and I'll add in the release soon.

isholgueras → made their first commit to this issue’s fork.

Thanks for the work. I've fixed the conflicts and now the branch looks good.

isholgueras → made their first commit to this issue’s fork.

You're right. I've missed the _ui module. Thanks!

Thanks! Will be added in the next release.

isholgueras → made their first commit to this issue’s fork.

Patch #12 works for me too

I've reproduced the issue on 10.2, the patch #6 applies and fixes the issue.

Committed to 4.x and I will include it in the next release.

Thanks!

I forgot to check the checkboxes for credit in the merge but I've added later an empty commit with the appropriate credit https://git.drupalcode.org/project/environment_indicator/-/commit/e01de5....

I'll ask to see how it should be done if this is not the way.