🇫🇷France @mattlc

To be tested

mattlc → created an issue.

Thank you Marcus, Just pushed fixes.

Classes based on "string names" is related to the fact that another module apart from address module may declare "address" field type.

But as we check that address module (which is unique) is enabled and the fact that it requires commerceguys/addressing we are good with the use of ruleIsAllowed() here.
So the UX will be consistent : no automation on "address" fields that would come from another module than "address". And the ai module won't crash trying to instantiate the Address automator because it does not "uses" any class from outside (coming from commerceguys/addressing and address module).

This is also why I didn't added the commerceguys/addressing as it is a requirement of the address module this automator depends on.

OK @markus_johansson I take care of it.

Comments fixed.
I didn't add the check before array_merge_recursive because it would duplicate the verify.
The verify could directly be called by runChatMessage but the check wont be in the right place regarding AiAutomatorRuleRunner::generateResponse implementation.

@MrDaleSmith & @seogow You're right.

The classes existence check should be dedup.
This all could be replaced by a module handler module exists call for address module and located in the "ruleIsAllowed()" method.
I cannot just focus on "field type id" in the plugin definition as nothing really enforce other module to declare "address" field type.
Also set "class names strings" as constant variables in the plugin class should be done.

To be honest, in my opinion, this plugin should part of the address module by itself as a "extra feature" of the address module. But also here, the dependencies with uses classes for inheritance may be tricky as the ai module should not be required by the address module.

I'll check also the array_merge_recursive point.

Thanks a lot for your feedback

Thank you @jurgenhaas, I was used to do this with the provided link in git push command response.

I don't know why I'm not able to create a merge request.
If someone is able to do so, please feel free to create it from my branch and review it.

mattlc → created an issue.

Added validation and dynamic address fields from field config.

Hi, Yes it's exactly the use case : get unstructured result (or any non structured contribution) and fill structured address.
I should definitely both refactor the prompt based on field configuration and add post process check for required fields.
Google places check could be an optional step also.
Thank you.

mattlc → created an issue.

MR updated.
Don't know why CSpell now fails in pipeline complaining with word 'outro' in src/Form/AiSetupForm.php.

@vivek-panicker sure !

Hello. Thank @vivek-panicker you for this issue.

Maybe this would be simpler to use foreach here as $key and array_key_exists() does not seems to bring any value ?

      if ($indexes = $this->entityTypeManager->getStorage('search_api_index')->loadMultiple()) {
        /** @var \Drupal\search_api\IndexInterface $index */
        foreach ($indexes as $index) {
          $backend = $index->hasValidServer() ? $index->getServerInstance()->getBackendId() : NULL;
          if ($backend === 'search_api_ai_search') {
            $return = TRUE;
            break;
          }
        }
      }

Duplicates https://www.drupal.org/project/ai/issues/3493474 🐛 Vector DB Explorer link does not appear in API Explorer Page Active

mattlc → created an issue.

Fixed #14 (& #15). CI is running.

Fixed phpcs and phpstan errors.

mattlc → changed the visibility of the branch 3368857-remove-duplication-from-form to hidden.

mattlc → changed the visibility of the branch 3368857-remove-duplication-from to hidden.

Just saw this. My bad.
I rework on it.
Sorry for that.

Hello,

Thank you @anand48, as discussed with both @nico059 and @ultimike at DC Bacelona Trait might be a better solution to keep a simple extension hierarchy.

I worked on the 3368857-remove-duplication-trait.

- minor code style issues fix
- rebased on 11.x
- force pushed to 3368857-remove-duplication-trait

So I think the MR to consider is 9792 from @nico059.

I set the status to Needs review.

I'm sorry, I don't know why functional tests do not pass.
As an example : Drupal\Tests\layout_builder\Functional\LayoutSection gives following error :

Layout section formatter access
       ┐
       ├ Behat\Mink\Exception\ResponseTextException: The text "Hello test world" was not found anywhere in the text of the current page.

As it does not relates on this issue, I don't know how to get the whole test pipeline work.

Thank you @larowlan & @smustgrave for your comments.

I rebased on 11.x and added missing part for contrib modules in BuildTestBase class.

Moving issue to Needs review.

mattlc → changed the visibility of the branch 3368857-remove-duplication-trait-rebased to hidden.

I tried to replace the abstract class by a trait in order to keep original class inheritance/interface implementation intact but the tests results are the same : the "Submit" button cannot be found by #14 stated failing tests.

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

Hello,

Sorry for answer delay.
Not sure to understand.

You want to strip entity wrapper ?

<article{{ attributes }}>
... the content of the node
</article>

becomes

... the content of the node

If you talk about the entity wrapper set in entity template, this module can't as it is only targeted to fields on a "per view mode" basis.
This may be easy to do in your theme by overriding entity template such as node.html.twig.

Regards.

Hi,
Thank you for this patch.
However it fails when content translation is not enabled in B.
As the code is directly in "ImportService", it should control more strictly the config sync between A and B.
I think that your code should be integrated in the language_fallback import processor and test if content translation is UP and active for current entity bundle.

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