Broken findMatchingRedirect() call

@veronicaSeveryn fixed the issue and created Patch.

Hi Abhikeh Gupta,

Lets keep using findMatchingRedirect rather than switching to a different method while fixing this issue since findMatchingRedirect was working previously and was tested.

To improve workflow, we will need Merge Requests and not patches.

Having the same issue.

Call to undefined method Drupal\Core\Entity\Sql\SqlContentEntityStorage::findMatchingRedirect() in Drupal\easy_breadcrumb\EasyBreadcrumbBuilder->getRequestForPath()

I don't understand how it's supposed to work. Previously, the method for the service RedirectRepository was called.

$redirect = \Drupal::service('redirect.repository')
  ->findMatchingRedirect($redirect_path, [], $this->languageManager->getCurrentLanguage()
  ->getId());

As mentioned in the original post, the code was changed:

$redirect = $this->entityTypeManager->getStorage('redirect')->findMatchingRedirect($redirect_path, [], $this->languageManager->getCurrentLanguage()->getId());

It loads the storage for the redirect entity and calls ::findMatchingRedirect() on it. Maybe I'm missing something, but the redirect entity doesn't define its own custom storage that would implement that method. In that case, Drupal falls back to the default storage, which is SqlContentEntityStorage for content entity types. But this storage is clearly not implementing that method, which leads to an error.

Patch attached. Using injection for redirect.repository and findMatchingRedirect function

looks good, can we get a MR for Git lab CI testing?

Was thinking ok lets do the work of converting the patch to am MR ( can't be that hard )

Learned a lot about Tests and CI ;-)

Got all tests to pass that where passing before the MR

I am still not sure If it is the right solution to the problem since it adds a dependency on the redirect module

The last submitted patch, 7: easy_breadcrumb_3440889_Broken_findMatchingRedirect_call_7.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Ok I give up on trying to fix or get pass the 2 Remaining PHP unit tests...

I am not even sure if it is worth spending the time on fixing the testing of 3 Year old update code...
Problem seems to be that the Update function is checking the config against the already installed Schema

one would need to create SQL files for those tests...

So the status of the merge request is:

reverted the Problematic code from Issue 3421456
added a // @phpstan-ignore-next-line to not create a dependency against redirect module
added redirect module and path_alias (maybe useful for future tests..)
fixed the testAdministration Test..

my Suggestion would be to remove the 2 failing tests (test8006DefaultConfigurationUpdate, test8006AfterFormSaveConfigurationUpdate)
to make contributions not that frustrating.. ( although I learned a lot )

Unfortunately the MR now contains lots of unrelated fixes. I agree with most of these improvements but they must be fixed separately. Here we want to fix the issue mentioned in the title.

The most important thing to understand is that we must not inject a service that might not exist. Since the redirect module is not a dependency of the easy_breadcrumb module the redirect.repository service was called only inside a request: if ($this->moduleHandler->moduleExists('redirect') { ....

I will now open another MR to solely revert the unfortunate changes from 🐛 Fix tests Fixed . This will fix this issue. We might then open follow-up issues to fix the other stuff.

Let's get MR !112 in. For me it fixes the issue.

+ 1 for this simple merge (was carried away.. by trying to get fixes for test)

Ok. I think I did that right.

For the tests, lets try again to fix what we can, for annoyingly difficult tests on old update hooks and such, lets just remove the problem tests. The important bit with the tests is that the tests pass, not that we have great coverage.

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

spuky → closed merge request !110