Changes to Pathauto #21
Conversation
| var path = $('.form-item-path-alias input').val(); | ||
| var automatic = $('.form-item-path-pathauto input').attr('checked'); | ||
| var path = $('.form-item-path-alias input', context).val(); | ||
| var automatic = $('.form-item-path-pathauto input', context).attr('checked'); |
There was a problem hiding this comment.
We should make sure to backport this change to the D7 version.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
So for the D7 branch, should I post a patch in the pathauto issue queue? Would that be helpful?
There was a problem hiding this comment.
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
|
Good point! Didn't see that one. |
|
I've also removed the duplicate line. |
| @@ -530,7 +530,7 @@ public function updateAlias(EntityInterface $entity, $op, array $options = array | |||
| * depending on the $load_entities parameter. | |||
| */ | |||
| protected function getTermTree($vid, $parent = 0, $max_depth = NULL, $load_entities = FALSE) { | |||
| return taxonomy_get_tree($vid, $parent, $max_depth, $load_entities); | |||
| return \Drupal::entityManager()->getStorage('taxonomy_term')->loadTree($vid, $parent, $max_depth, $load_entities); | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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.
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.