🇮🇳India @vakulrai

Changes look good to me @michaellander .

Hi @marcus_johansson , Please review the changes and guide me if any other changes needed.

Thanks !

I will check this , assigning to myself.

I have added PHPCS fixes to the files as well. Marking the status to review now.

vakulrai → created an issue.

Mr is updated with the change.

Sure @marcus_johansson, i will update the MR.

vakulrai → created an issue.

vakulrai → created an issue.

vakulrai → created an issue.

I was able to test the feature and i can see its able to pick up the image field by extracting the media reference field.

Test case:
- Created and used media reference field in the Provider setting.
- The Ai output was able to understand the image and able to generate a description.

vakulrai → created an issue.

I tried to debugged why : core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php its faling and that is because the method : taxonomy_term_is_page($term) is checking for the route \Drupal::routeMatch()->getRawParameter('taxonomy_term') and the test is runing on a view that has a contextual filter argument as : arg_0 .

So , the above check $variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term); will always be false , so some additionla logic i needed to that covers the scenario of view.

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

vakulrai → created an issue.

Thank you @marcus_johansson for a review andpointing me to the code.

I have updated the MR accordingly and found one more issue about default value not being passed to "Other Joiner" select type, so i have fixed that in the same mr.

Hope its fine :)

Created a MR to fix the issue , i can see the default value is being considered on the settings form.
Please review !

vakulrai → created an issue.

Hi

Can you please add more information about the issue , like if this is happening on Drupal CMS or vanilla drupal instance ?

I am on Drupal version : 10.4.0 and using the 1.0.3 version of AI but the issue is not reproducible

Thanks !

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

Kristen Pol → credited vakulrai → .

vakulrai → created an issue.

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

All test Passed , moving to review !

All the tests are passing now , moving to ready for review !

I have rerolled to 11.x and have brought down the test failure to 6 :), please review the reroll and suggest path forward to fix the remaining test failures.
Thanks !

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

I have added a fix for the failed test https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676914 , and along with that i saw 2 tests are failing unexpectedly , which looks unrelated to the changes added here , I am adding the failures from the test here for visibility so can someone do a quick review and suggest a path forward.

1. https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676912

ErrorException: Method "IteratorAggregate::getIterator()" might add
"\Traversable" as a native return type declaration in the future. Do the
same in implementation "Drupal\Core\Entity\ContentEntityBase" now to avoid
errors or add an explicit @return annotation to suppress this message. in
/builds/issue/drupal-2910353/vendor/symfony/error-handler/DebugClassLoader.php:337

2. https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676917

 RuntimeException: Adding non-existent permissions to a role is not allowed.
    The incorrect permissions are "use text format oqcx2iea".

Thanks!

Updated the comments in the MR , please review !

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

11.x branch has merge conflicts can someone rebase it.

vakulrai → changed the visibility of the branch 3091285-composer-scaffolding-fails-preprocess to hidden.

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

Re-rolled patch as its reproducible for 11.x branch and for the tests as the errors are more due to the JS mutation observer is missing a check and the error in the console cant be tracked in the Tests so can anyone suggest a way to have a test coverage here Else we can move this ticket forward.

Thanks!

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

Converted the patch as a MR , will try to update the https://www.drupal.org/project/drupal/issues/2910353#comment-15031176 🐛 Prevent saving config entities when configuration overrides are applied Needs work in the next commit.

Thanks !

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

Hi @wim-leers i tried to add my comments on the pending feedback , please review.

Points covered :

• https://git.drupalcode.org/project/drupal/-/merge_requests/5132#note_255633
• https://git.drupalcode.org/project/drupal/-/merge_requests/5132#note_255644

Thanks !

I have added the constraint validators to the core/modules/image/config/schema/image.schema.yml , please review.

Thanks !

vakulrai → changed the visibility of the branch 3395616-img-settings-config-validation-constraint to hidden.

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

I tried to reproduce it on 10.x branch by doing the below changes but the issue is not happening anymore:

• I intentionally added chmod 444 to sites/default/default.settings.php file while on a 9.5.x branch
• Moved to Drupal 10.x branch and run composer update but what i can see is that the 444 got converted to 644

I think this ReplaceOp::process() method is managing the files in a correct manner :

/**
   * {@inheritdoc}
   */
  public function process(ScaffoldFilePath $destination, IOInterface $io, ScaffoldOptions $options) {
    $fs = new Filesystem();
    $destination_path = $destination->fullPath();
    // Do nothing if overwrite is 'false' and a file already exists at the
    // destination.
    if ($this->overwrite === FALSE && file_exists($destination_path)) {
      $interpolator = $destination->getInterpolator();
      $io->write($interpolator->interpolate("  - Skip <info>[dest-rel-path]</info> because it already exists and overwrite is <comment>false</comment>."));
      return new ScaffoldResult($destination, FALSE);
    }

    // Get rid of the destination if it exists, and make sure that
    // the directory where it's going to be placed exists.
    $fs->remove($destination_path);
    $fs->ensureDirectoryExists(dirname($destination_path));
    if ($options->symlink()) {
      return $this->symlinkScaffold($destination, $io);
    }
    return $this->copyScaffold($destination, $io);
  }

I think these 2 files are usually should be copied and then updated before any integration and any modification to these files can cause permission hardening.

sites/default/default.services.yml
sites/default/default.settings.php

I came across this problem while moving from 9.5.x to 10.x and i feel + 1 to modify the scaffolding process in a manner that if the permission of incoming version differs from what is locally present try to do the below:

• Store the existing permission and do a chmod 775 on sites/default
• replace or overwrite based on the scaffolding process set in composer.json
• Then restore the existing permission from bullet 1

Else fallback to the default behaviour.

vakulrai → created an issue.

Adding a patch to fix the if condition check.

vakulrai → created an issue.

attaching the patch

vakulrai → created an issue.

Updating the patch for D10 compatability.

Hi @dave Thank you for the suggestions.

Actually the problem that we are facing is for existing content where we have those attributes set already like :
<drupal-entity data-entity-url-title data-no-margin data-entity-url-target>

now the problem is when we move to CKE5 , the users are not able to see the changes controlled by the attributes above, as they were not moved under
data-entity-embed-display-settings automatically .

The idea is there should be a way to migrate the existing attributes to data-entity-embed-display-settings to support the existing functionality.

Thank you !

Attaching the patch for visibility:

Adding a patch to fix the issue , this is definitely can be made configurable but to start off i am adding it so that others facing the same can use it.

Thanks

vakulrai → created an issue. See original summary → .

vakulrai → created an issue.

vakulrai → created an issue.

vakulrai → created an issue.

Uploading the patch diff from : https://git.drupalcode.org/project/token/-/merge_requests/36/diffs for visibility.

I am adding a patch to support the feature , i know this is not the very prominent or reliable solution but just for other projects having the some problem:

vakulrai → created an issue.

vakulrai → created an issue.

Reuploading : used the urong issue id:

Re-rolling the last patch to add the code within the password_policy_update_8304 to avoid any conflicts with the upcoming releases.

After my analysis i have added the update hook to make sure the show_policy_table: TRUE is set to true for existing configs.

vakulrai → created an issue. See original summary → .