Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for dev/drupal#158 #19888

Closed
wants to merge 1 commit into from
Closed

Conversation

jaapjansma
Copy link
Contributor

@jaapjansma jaapjansma commented Mar 24, 2021

Overview

This fixes an annoying bug in Drupal 8 and Drupal 9 where Drupal does not recognize any urls defined in a newly installed extension.

The work aorund is to clear the drupal caches.

Before

After installing an extension drupal will return Page not found when going to a page defined in the extension.

After

After installing an extension drupal will show the content when going to a page defined in the extension.

Technical Details

There is a little hack needed, we need to check whether the Drupal class is available and if so we assume we are in drupal 8/9 and we rebuild the router.

Comments

I am not completely sure whether this is the right place to fix this.

See https://lab.civicrm.org/dev/drupal/-/issues/158 and https://lab.civicrm.org/dev/drupal/-/issues/37

@civibot
Copy link

civibot bot commented Mar 24, 2021

(Standard links)

@civibot civibot bot added the master label Mar 24, 2021
@homotechsual
Copy link
Contributor

homotechsual commented Mar 24, 2021

I think we should be doing \Drupal\Core\Cache\Cache::invalidateTags(['route_match']) since it has a lower performance impact. The rebuild() option is a bit like using a sledgehammer, it rebuilds every single route including core provided ones. Rebuilding the route_match tag only clears out non-static routes.

https://drupal.stackexchange.com/questions/288321/is-there-a-way-to-partially-rebuild-the-router-or-invalidate-a-route-cache

@seamuslee001
Copy link
Contributor

I agree Mikey but also I'm kind of a little nervous / concerned about having CMS specific code here I'm wondering if we need to create a function on the CRM_Utils_System_x classes instead?

@homotechsual
Copy link
Contributor

I agree Mikey but also I'm kind of a little nervous / concerned about having CMS specific code here I'm wondering if we need to create a function on the CRM_Utils_System_x classes instead?

I agree, I'd like us to do this when an extension is enabled ideally since that's when we need to do it, that and ideally when rebuilding Civi's menu.

@demeritcowboy
Copy link
Contributor

Thanks @jaapjansma for giving this a kick. It seems like the decision now is just where the call should go. My vote is for the other PR simply because this here would get called on every menu item not just related to extensions.

@jaapjansma
Copy link
Contributor Author

Fixed by PR 19906

@jaapjansma jaapjansma closed this Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants