Pune
Account created on 26 May 2014, over 10 years ago
  • Sr. Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇮🇳India chandu7929 Pune

I have fixed most of the failing tests only 2 similar test are failing.

  1. Drupal\Tests\quickedit\FunctionalJavascript\LayoutBuilderIntegrationTest::testArticleNode - This caused by empty body field value
  2. Drupal\Tests\quickedit\FunctionalJavascript\CKEditor5IntegrationTest::testArticleNode - Not sure why saving state isn't available for body field
🇮🇳India chandu7929 Pune

The remaining tests are failing because the library quickedit.inPlaceEditor.formattedText is not attached during testing. I will investigate further to determine the root cause.

🇮🇳India chandu7929 Pune

@damienmckenna Thank you for your insights. I’ll mark this issue as closed (won't fix).

🇮🇳India chandu7929 Pune

Add all commits from 📌 GitLab CI testing Needs review to make sure all test are passing for this issue as well.

🇮🇳India chandu7929 Pune

@capysara - The fix was designed to log warnings for assets that have already been deleted from the DAM, particularly when the queue process attempts to delete assets that no longer exist.

I found that it just generated new batches of warnings/errors on every cron afterwards. It was never the same messages in the logs. That make me think that this isn't the complete solution.

Could you please confirm if you're seeing multiple warnings for the same assets that were processed in the previous cron run, particularly for the integration delete?

About the new batches of warnings and errors: These fixes enable the queue to process items without terminating the process. Therefore, whenever the queue is processed, any issues with the asset references will generate warnings and errors in the logs once.

🇮🇳India chandu7929 Pune

Verified recent changes and it looks good to me. I would request another sets of eyes.

🇮🇳India chandu7929 Pune

@Rajeshreeputra - Requested some changes, please take a look.

🇮🇳India chandu7929 Pune

I have verified the attached patch which fixes the issue. Needs review.

"patches": {
            "drupal/acquia_search": {
                "Acquia Search can lock threads excessively leading to denial of service on cache clear": "https://www.drupal.org/files/issues/2024-09-10/acquia_serach-3466306.patch"
            }
        }
🇮🇳India chandu7929 Pune

Added test and CI is green, hence requesting review.

🇮🇳India chandu7929 Pune

Hello Yuvania lets make changes related to D11 compatibility fixes only. We should create separate issue for phpcs, eslint etc.

🇮🇳India chandu7929 Pune

Due to dependency conflict between core-recommended 9.5 and league/uri-interfaces 7.0.0 composer is not able to download any 7.x version of league/uri-interfaces. In beta version the class League\Uri\UriString doesn't exists which causes test failure, hence dropping support for D9.

🇮🇳India chandu7929 Pune

Only previous major is failing with following errorException: Error: Class "League\Uri\UriString" not found and I think its due to the version of league/oauth2-server (8.5.4)that requires league/uri: ^6.7 || ^7.0

🇮🇳India chandu7929 Pune

Created MR!67 with required changes, hence moving ahead and requesting review.

🇮🇳India chandu7929 Pune

With my further analysis, it looks like the generated auth_token is invalid, because using postman I am able to make request successful. I will try to investigate further if I can narrow down issue.

🇮🇳India chandu7929 Pune

We also need to include this change for D9 from js/moderation_sidebar.js

  // information is already available.
  $('.toolbar-icon-moderation-sidebar').on('click', function (e, data) {
    if ($('.moderation-sidebar-container').length && (!data || !data.reload)) {
      $('#drupal-off-canvas-wrapper, #drupal-off-canvas').dialog('close');
      e.stopImmediatePropagation();
      e.preventDefault();
    }
🇮🇳India chandu7929 Pune

Hello @ japerry , Can you please confirm if we have to keep support for D9 with respect to the comment 📌 Automated Drupal 11 compatibility fixes Needs review otherwise we are good.

🇮🇳India chandu7929 Pune

What is preventing these changes from being released?.

Today I updated my blog site with Drupal 10.3 which uses like_and_dislike module that depends on this votingapi module, and when I visited one of my page, I can see WSOD

I had to use this MR's changes to patch the module, which fixes the issue.

"patches": {
            "drupal/votingapi": {
                "3341513 - Voting Storages does not check access on entity query": "https://git.drupalcode.org/project/votingapi/-/merge_requests/5.diff"
            }
        }
🇮🇳India chandu7929 Pune

PR !36 changes looks good to me, though D11 ci is failing hence its can't be consider for review. Hence needs work.

🇮🇳India chandu7929 Pune

Considering there's no issue in this MR, hence moving back to RTBC

🇮🇳India chandu7929 Pune

chandu7929 made their first commit to this issue’s fork.

🇮🇳India chandu7929 Pune

Created separate MR!17 with minimum required changes for D11, fixes hence requesting review.

🇮🇳India chandu7929 Pune

@acbramley the class EntityTestMulRevPub requires entity_test_mulrevpub schema to be enabled which is currently not happening and hence tests are failing for all the versions of Drupal. To fix this we need to either switch test to kernelTestBase or use node::create in BrowserTestBase.

deepakkm I don't agree with this explanation; we just have to add these two lines of code, which will do the work; no update is required in any of the tests files.

diff --git a/tests/src/Functional/ModerationSidebarTest.php b/tests/src/Functional/ModerationSidebarTest.php
index a678462..ea3f71f 100644
--- a/tests/src/Functional/ModerationSidebarTest.php
+++ b/tests/src/Functional/ModerationSidebarTest.php
@@ -62,6 +62,10 @@ class ModerationSidebarTest extends BrowserTestBase {
       'name' => $this->randomMachineName(),
     ]);
     $entity->save();
+
+    $user = $this->createUser([], '', true);
+    $this->drupalLogin($user);
+
🇮🇳India chandu7929 Pune

Hello dqd , Can you please provide some more details for the error or warning you are getting? May be there are steps to reproduce it?

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

In Drupal 10.3, multivalue fields now have a field label.
See the change record at: New method getLabel() on FieldItemDataDefinition , hence updated minimum core requirement to 10.3.

In order to support below 10.3 we need to update function definition ofrequestOpenApiJson() from openapi module to process expectations data something like below:

     // As per changes request https://www.drupal.org/node/3404250
    // now FieldItemDataDefinition has new key title added automatically
    // for core 10.3 below lets remove these keys
    if (version_compare(\Drupal::VERSION, '10.3.0', '<')) {
      $expected = $this->updateExpectations($file_name);
    }

...

protected function updateExpectations() {
    // Data update logic here.
  }
🇮🇳India chandu7929 Pune

I will remove this temp commit once review done. Requesting review.

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

@ Rajeshreeputra - I have update MR with change requested.

🇮🇳India chandu7929 Pune

Updated MR with required changes, hence requesting review.

🇮🇳India chandu7929 Pune

Since all tests are passing, hence requesting review.

🇮🇳India chandu7929 Pune

I think we should keep minimum core_version_requirement as ^10.2 || ^11 with respect to changes introduced in 10.2.0 (Refer: https://www.drupal.org/project/drupal/issues/3347291 📌 Combine field storage and field instance forms Fixed ) and update test accordingly.

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

Hello AdamPS , I have updated MR with the requested changes and also created a separate issue 📌 Broaden test coverage. Active for broaden test coverage. Hence requesting review.

🇮🇳India chandu7929 Pune

Now CI is green, can we get this merge and release?

🇮🇳India chandu7929 Pune

All tests are passing for MR!26, hence requesting review. I will go ahead revert the Temp commit: Get require-dev using vcs.

🇮🇳India chandu7929 Pune

Made changes as per suggestions, hence requesting review.

🇮🇳India chandu7929 Pune

I can see some of the commits are missing from 3.1.x, when I tried rebase this with 3.0.x(I don't know what was the purpose to skip that commit)

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

Since all test are passing on D9.5 & D10 as well as on D10 & D11 hence I will remove all the temp commit and move this issue in needs review.

🇮🇳India chandu7929 Pune

Fixed all failing test for D10 & D11 need to work on failing test for D9.5 though D11 fork branch of quickedit 📌 Automated Drupal 11 compatibility fixes for quickedit Needs work has removed D9 support.

🇮🇳India chandu7929 Pune

Changes looks good to me, hence RTBC.

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

Looks good to me. Hence RTBC

🇮🇳India chandu7929 Pune

chandu7929 made their first commit to this issue’s fork.

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

Fixed phpstan and phpcs issue, hence requesting review.

🇮🇳India chandu7929 Pune

Rajeshreeputra - We should have a commit where all CI is green, then you can revert that commit by adding its CI link, so that we know everything is working with added dependencies. May be you can pull using vcs which are not compatible for next major and run the CI

🇮🇳India chandu7929 Pune

Changes look good to me, unless we want to drop support for 10.0, similar changes I have done here: #3431601

🇮🇳India chandu7929 Pune

Now I have updated the MR with required change hence requesting review.

🇮🇳India chandu7929 Pune

chandu7929 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India chandu7929 Pune

Hello bircher ,

Hi a href=" https://www.drupal.org/u/bircher ">bircher,

I hope you’re doing well and that your vacation was great!

When you have a moment, could you please look into doing a release? It would be really helpful for unblocking Acquia CMS common .

Thanks a lot!

Production build 0.71.5 2024