Feature/sub page aliases #46
Conversation
|
When reviewing ignore the failing functional js test. It seems to be an issue that potentially affects more than our repo #47 |
localgov_elections.module
Outdated
| * Returns nothing. | ||
| */ | ||
| function localgov_elections_generate_election_aliases(NodeInterface $entity) { | ||
| if ($entity->getEntityTypeId() == 'node' && $entity->getType() == 'localgov_election') { |
There was a problem hiding this comment.
$entity->getEntityTypeId() == 'node': this is kind of redundant because we know $entity is of type NodeInterface :)
There was a problem hiding this comment.
That is true. Fixed.
localgov_elections.module
Outdated
| $storage = \Drupal::entityTypeManager()->getStorage('path_alias'); | ||
| $query = $storage->getQuery(); | ||
| $query->condition('alias', $alias); | ||
| $query->accessCheck(FALSE); | ||
| $result = $query->execute(); | ||
|
|
||
| if ($result) { | ||
| $result = reset($result); | ||
| $result = $storage->load($result); |
There was a problem hiding this comment.
How about something like this:
$alias_entities = $storage->loadByProperties(['alias' => $alias]);
if ($alias_entities) {
try {
current($alias_entites)->delete();
}
...
}
or even
$alias_entities = $storage->loadByProperties(['alias' => $alias]);
array_walk($alias_entities, fn(Drupal\path_alias\PathAliasInterface $alias_entity) => $alias_entity->delete())
There was a problem hiding this comment.
I went with the first suggestion but added a guard to deal with current. If that array was empty, that method might not exist.
localgov_elections.module
Outdated
| * @return void | ||
| * Returns nothing. | ||
| */ | ||
| function localgov_elections_delete_election_aliases($election) { |
There was a problem hiding this comment.
In the absence of doc comment for the $election parameter, it would be good to have a type declaration e.g. NodeInterface $election
There was a problem hiding this comment.
Fair point. Added in.
|
@Adnan-cds Addressed the points you raised I think. |
Should close #29.