Changes to Pathauto

[basvdheijden]

I've reviewed the current state of the pathauto module.
As I'm new to D8 development, I did some minor changes I saw during the reading of the code.

[davereid]

We should make sure to backport this change to the D7 version.

[Berdir]

We should. This code is kind of pointless in D8, at least with seven, because the new seven UI doesn't display it anymore. But core still has it everywhere too and is displayed if you use the frontent theme, unless that replaces it too.

[basvdheijden]

So for the D7 branch, should I post a patch in the pathauto issue queue? Would that be helpful?

[vijaycs85]

yep, I guess you can raise a issue with version 7.x-1.x-dev at https://www.drupal.org/node/add/project-issue/pathauto

[basvdheijden]

Good point! Didn't see that one.

[basvdheijden]

I've also removed the duplicate line.

[Berdir]

The reason that was moved to a protected method is to be able to easily mock it in unit tests if we would add those.

We could possibly instead inject the entity manager in the constructor now and get the storage from that, then just do $this->termStorage->loadTree(). That is however not important, we want to remove the term specific logic from the pathauto manager anyway, so it is temporary.

[basvdheijden]

Ok, thanks for the info. If you want to remove the taxonomy specific stuff, I'd gladly help. Do you already have an idea where to put it? Maybe inside the module file in the hook_entity_update? Or should the module file in D8 serve more as a controller (and therefore be thin and only act as the glue code for services)?

[basvdheijden]

I've looked at it more closely, and the module file is no good place at all because that would lead to code duplication on many places where PathautoManager::updateAlias is issued. I'm not sure what would be a good place though..

[Berdir]

Don't worry about it for now. We're working on making the different pathauto pattern types plugins in #15, so we can possibly easily add a method somewhere in here or even move most of the logic here into a default method of a plugin base class.