Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mkalkbrenner created an issue. See original summary.

ruloweb’s picture

Hi, attached is a patch that swap to the Drupal eventDispatcher. SearchSubscriber class requires to be updated a little bit the as well.

It only applies to last dev, not to 8.x-1.12, because of the few last commits.

ruloweb’s picture

The last patch was missing the event service definition, anyway it has a few bugs, for example the events in SearchSubscription class should only be called with the current search is using the Acquia connector.

Attached is a new version, is a different (and better I think) approach so I doesn't make sense to upload a interdiff.

dorficus’s picture

This patch is still referenced in the Acquia docs. Is it still needed for 1.16.0 to use Solr Devel? If so, it does not apply cleanly and will need a reroll.

dorficus’s picture

Rerolled to work with 1.16.0

dorficus’s picture

Oh wow, it's way too early for me and it's Friday. I made a mistake and tested the wrong patch against 1.16.0 and saw issues. This "reroll" is exactly the same as #3.

Don't mind me...

dorficus’s picture

Status: Needs review » Reviewed & tested by the community

This patch worked. I tested it on an Acquia dev environment and I was able to get Solr Devel working to debug my queries on the Dev server. Marking RTBC.

kyberman’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.62 KB

Hi James, the patch can't be applied against the current code, unfortunately.
Here is a reroll for current dev that is applicable also for 1.18, but not tested yet.

Dane Powell’s picture

Status: Needs review » Needs work

The test failures in #8 need to be resolved. It looks like the search subscriber needs to implement PluginInterface?

Dane Powell’s picture

Title: Swap the event subscriber within the SolrConnector » Incompatibility with Solr Devel
Status: Needs work » Needs review
FileSize
2.52 KB
2.81 KB

The attached patch should pass tests. Can someone please test that it fixes the original issue for all of this, which I believe is compatibility with Solr Devel?

#8 wasn't working because it was trying to set a protected property on the parent plugin. So when it tried to register that plugin later on, it was passing a null reference instead of the SearchSubscriber instance. I went back to the old way of creating that plugin. I don't see a clean way to handle this with service injection given that the create method is static, and anyway that's refactoring not exactly related to this patch.

  • Dane Powell authored 1f0494c on 8.x-1.x
    Issue #2838963 by ruloweb, dorficus, kyberman, Dane Powell:...
Dane Powell’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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