Account created on 29 March 2017, about 7 years ago
  • Sr. Engineer - Drupal Back End at QED42ย  โ€ฆ
#

Merge Requests

Recent comments

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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 !

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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!

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ changed the visibility of the branch 3091285-composer-scaffolding-fails-preprocess to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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!

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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 !

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

Thanks !

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

vakulrai โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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);
  }
๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

Adding a patch to fix the if condition check.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

attaching the patch

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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 !

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

Attaching the patch for visibility:

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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:

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

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

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

I spent time on this and i could not reproduce the issue.

I am checking the issue on drupal: 8.9
Php: 7.4

Steps is did:
- I initailly configured a menu block using Alpha verison
- I configured a block so that we can have a block that folows clases alter done by the latest versions.
- I then moved to the latest dev version and properly clear caches and i could not see the error mentioned here: https://www.drupal.org/project/menu_multilingual/issues/3366412 ๐Ÿ› TypeError on latest dev Postponed: needs info .
As i suspect its maily block definition being cached so we can include a hook to rebuild the block definfition in a hook_update_n attached , please review.

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

Closing this issue , The problem is because of some configurations.
Thanks

๐Ÿ‡ฎ๐Ÿ‡ณIndia vakulrai

Adding a patch to fix this bug:

Highlight of the fix:
- Followed the behavioural Js standard for the Attached Js

Production build 0.69.0 2024