Problem/Motivation

Hello project maintainers,

This is an automated issue to help make this module compatible with Drupal 10.

To read more about this effort by the Drupal Association, please read: The project update bot is being refreshed to support Drupal 10 readiness of contributed projects

Periodically patches will be added to this issue that remove Drupal 10 deprecated API uses. To stop further patches from being posted simply close this issue (any status besides Active, Needs review or Needs work) or remove the "ProjectUpdateBotD10" tag from the issue.

The patches will be posted by the Project Update Bot official user account. This account will not receive any issue credit contributions for itself or any company.

Proposed resolution

You have a few options for how to use this issue:

  1. Accept automated patches until this issue is closed

    If this issue is left open (status of Active, Needs review or Needs work) and the "ProjectUpdateBotD10" tag is left on this issue, new patches will be posted periodically if new deprecation fixes are needed.

    As the Drupal Rector project improves and is able to fix more deprecated API uses, the patches posted here will cover more of the deprecated API uses in the module.

  2. Leave open but stop new automated patches.

    If you want to use this issue as a starting point to remove deprecated API uses but don't want new automated patches simply leave this issue open but remove the "ProjectUpdateBotD10" tag from the issue.

    You can use Drupal Rector yourself to make these patches.

    If you want to receive automated patches again, add back the "ProjectUpdateBotD10" tag.

  3. Close it and don't use it

    If the maintainers of this project don't find this issue useful, they can close this issue (any status besides Active, Needs review or Needs work) and no more automated patches will be posted here.

    If the issue is reopened, then new automated patches will be posted.

    If you are using another issue(s) to work on Drupal 10 compatibility it may be useful to other contributors to add those issues as "Related issues" when closing this issue.

Remaining tasks

Using the patches

  1. Apply the patch in the comment by Project Update Bot.
  2. Thoroughly test the patch. These patches are automatically generated so they haven't been tested manually or automatically.

Providing feedback

If there are problems with one of the patches posted by the Project Update Bot, such as it does not correctly replace a deprecation, you can file an issue in the Drupal Rector issue queue. For other issues with the bot, for instance if the issue summary created by the bot is unclear, use the Infrastructure project issue queue using the component “Bot: Drupal Rector”.

CommentFileSizeAuthor
#16 Screen Shot 2022-12-15 at 8.31.20 pm.png147.93 KBmatthew.hallsworth
#16 Screen Shot 2022-12-15 at 8.19.25 pm.png729.5 KBmatthew.hallsworth
#15 Screen Shot 2022-12-15 at 4.23.21 pm.png58.14 KBmatthew.hallsworth
#10 elasticsearch_connector-8.7.x-drupal10-3297048-10.patch24.88 KBnicxvan
#4 elasticsearch_connector.7.x-dev.rector-3297048-4.patch10.24 KBnadim hossain
#2 elasticsearch_connector.7.x-dev.rector.patch9.65 KBproject update bot

Issue fork elasticsearch_connector-3297048

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Project Update Bot created an issue. See original summary.

project update bot’s picture

project update bot commented
Status: Active » Needs review
StatusFileSize
new elasticsearch_connector.7.x-dev.rector.patch9.65 KB

This is an automated patch generated by Drupal Rector. Please see the issue summary for more details.

It is important that any automated tests available are run with this patch and that you manually test this patch.

Drupal 10 Compatibility

According to the Upgrade Status module, even with this patch, this module is not yet compatible with Drupal 10.

Currently Drupal Rector, version 0.13.0, cannot fix all Drupal 10 compatibility problems.

This patch does not update the info.yml file for Drupal 10 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot to post additional Drupal 10 compatibility fixes as they become available in Drupal Rector.

Debug info

Bot run #139

This patch was created using these packages:

  1. mglaman/phpstan-drupal: 1.1.24
  2. palantirnet/drupal-rector: 0.13.0
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue summary: View changes
Issue tags: +Drupal 10 porting day

Working during porting day. Tagging for visibility.

The bot says this patch isn't complete so we need someone to analyze the patch, run upgrade_status, and try out the module to see if they can figure out what needs to be added, if anything.

nadim hossain’s picture

nadim hossain commented
StatusFileSize
new elasticsearch_connector.7.x-dev.rector-3297048-4.patch10.24 KB

Hi,
It is missing the core requirement dependency to D10 and I have updated that in this patch.

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

nicxvan opened merge request !29

nicxvan’s picture

nicxvan commented

I converted the patch to a merge request to make it easier to collaborate.

The only change in the first round is I had to remove the core: 8 in info.yml since it caused a white screen.

nicxvan’s picture

nicxvan commented

I fixed the remaining issues.

The only bit I wasn't sure on was replacing the removed ->pass method with an exception.

nicxvan’s picture

nicxvan commented

After all of these changes it shows as compatible with Drupal 10. I was able to test on a site with this and things worked. I'm going to create a patch to run tests.

nicxvan’s picture

nicxvan commented
StatusFileSize
new elasticsearch_connector-8.7.x-drupal10-3297048-10.patch24.88 KB
nadim hossain’s picture

nadim hossain commented

Thanks for adding that nicxvan. I will start reviewing this. I was also confused on which one is the correct version to create the patch for. Cause most recent merge happened on 8.0 branch, but 8.7 looks more appropriate.

matthew.hallsworth’s picture

matthew.hallsworth commented

Hey @Nadim Hossain and @nicxvan

I've tested this manually in a DDEV sandbox for the listed versions and it works as expected.

Testing steps:
- Installed Drupal 9 recommended in a DDEV container
- Installed modules upgrade_status and elasticsearch_connector
- Added origin for patch and installed patch into module for D10
- Ran the upgrade_status report - report shows that branch is compatible with D10
- Attempted to connect to a known ES cluster we use for another project, connected fine
- Was able to index correctly

Once code has been reviewed, happy for it to be moved to RTBC

Cheers

nadim hossain’s picture

nadim hossain commented

Thanks @Matthew for testing this. I have done the code review and it looks good to me.

nadim hossain’s picture

nadim hossain commented
Status: Needs review » Reviewed & tested by the community
matthew.hallsworth’s picture

matthew.hallsworth commented
Issue summary: View changes
StatusFileSize
new Screen Shot 2022-12-15 at 4.23.21 pm.png58.14 KB

Screenshot attached of upgrade_status confirming that https://git.drupalcode.org/issue/elasticsearch_connector-3297048 works as expected in D9 and is compatible with D10

matthew.hallsworth’s picture

matthew.hallsworth commented
StatusFileSize
new Screen Shot 2022-12-15 at 8.19.25 pm.png729.5 KB
new Screen Shot 2022-12-15 at 8.31.20 pm.png147.93 KB

Screenshot also of successfully connecting to cluster, attached here as well as successfully doing a reindex here and seeing the items in the index. I'm happy with the results.

marcgr’s picture

marcgr commented

I tested zipping up the patch version of the git code and adding to a new ddev drupal 10 site. I was able to install, but not enable. There appears to be an issue with a dependency of a dependency needing an older version of php. Drupal 10 uses php 8.

nodespark/des_connector > elasticsearch/elasticsearch

sokru’s picture

sokru commented

Thanks everyone, I'll commit this and make new release during the xmas holidays.

sokru’s picture

sokru commented
Status: Reviewed & tested by the community » Needs work

There's bunch of TypeError: Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher::dispatch(): Argument #1 ($event) must be of type object, string given, called in... issues.

Needs changes like this:

-     $event = $dispatcher->dispatch(PrepareIndexEvent::PREPARE_INDEX, $prepareIndexEvent);
+     $event = $dispatcher->dispatch($prepareIndexEvent, PrepareIndexEvent::PREPARE_INDEX);

Eg. trying to add a new Search index at /admin/config/search/search-api/add-index gives fatal error.

nicxvan’s picture

nicxvan commented

I can take a look at finding those changes

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

I've made those changes, I can't review it at the moment so I'm just marking it needs review.

nicxvan’s picture

nicxvan commented

I was able to test it again, I was able to edit, delete and add indexes and clusters.

  • sokru committed 149fd0d6 on 8.x-7.x authored by nicxvan 
    Issue #3297048 by nicxvan, sokru, Nadim Hossain, manu manu, karishmaamin...
sokru’s picture

sokru commented
Status: Needs review » Fixed

Made small alterations because UpdateBot or drupal-rector didn't follow a recommendations from https://www.drupal.org/node/3159012.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

This had an outdated branch in the main project repo, which I deleted just now.