Module is not 8.8 compatible

Thanks for your contribution.

Just changing the field name wont do though, as we still need to be compatible with 8.6 and 8.7. I've added a version check. Lets see if I managed to get automated testing of patched set up too.

This isn't a long term solution though, as 8.9 obsoletes the AliasStorage we're overriding, so for that a new solution needs to be found.

OK, another try.

Committed. Would be nice if someone would confirm that it works in practice on 8.8 before i roll 1.2.

I've added a version check.

I'll push the 8.9 problem until 8.9 becomes next up. Unless someone would like to tackle it early.

Whoops, pushed now.

The catchException can be fixed by including it in the class.

Testing 1.x with Drupal 8.8.x. No success so far :/

LanguageNeutralAliasesStorage::save() isn't called at all when I create a node, yet the path alias is generated as usual.

 * @deprecated \Drupal\Core\Path\AliasStorage is deprecated in drupal:8.8.0 and
 *   is removed from drupal:9.0.0. Use the "path_alias.repository" service
 *   instead, or the entity storage handler for the "path_alias" entity type
 *   for CRUD methods.

Means that we need to override path_alias.repository instead of path.alias_storage for Drupal 8.8.x. And additionally add entity hooks for path_alias entity type.

Another bad thing: I have noticed that path_alias.repository serviced is already altered by the core's workspaces module. Meaning that workspaces will be incompatible with language_neutral_aliases.

Not sure if I'm 100% correct.

Thinking of solutions...

@Leksat
Yeah, I figured this was just a temporary hack that wouldn't work for long.

> Meaning that workspaces will be incompatible with language_neutral_aliases.

Not necessarily, if we're aware what they do and how to work with that. Can you point to the core code you think is the problem?

I was thinking we might have to implement language_neutral_aliases by hooking into the new entity life-cycle instead, but I haven't had time to look into it.

> Meaning that workspaces will be incompatible with language_neutral_aliases.

Not necessarily, if we're aware what they do and how to work with that. Can you point to the core code you think is the problem?

Both workspaces and language_neutral_aliases override the path alias service by replacing its class:
- https://git.drupalcode.org/project/drupal/blob/4dda65baccc3df77947b559cc...
- https://git.drupalcode.org/project/language_neutral_aliases/blob/383b518...

But I think there are other ways to override/extend services in D8. Will try to take a look today.

According to my tests:
- we need to change path alias language to "und" in hook_entity_presave
- there is no need to override path_alias.repository service because it already fallbacks to "und" language

So here is a funny patch :) (it will fail because I haven't updated tests)

@Xen, will this approach work for you?

@Leksat
Sorry for not replying sooner, have been a bit too busy lately.

I like that it's less code. Less code means less possible bugs.

What I don't like (for irrational reasons) is that it's a hook.

There's one thing that the old approach did that the patch doesn't address: it would fix up lookups to look for the language neutral version too. This basically allowed us to "defang" the existing non-neutral aliases in the database without updating them, and always use the neutral ailas. I would assume that using this new approach that existing aliases would work as they used to. Fixing this leads us back to overriding AliasRepository.

But there is a way to mess with services without overriding them completely: service decorators: https://www.phase2technology.com/blog/using-symfony-service

Looking around core code, I was wondering if we could get away with decorating the AliasRepository service and just change the four lookup methods to lookup an alias with any language code (preferring LanguageInterface::LANGCODE_NOT_SPECIFIED if there's multiple hits), and not actually changing aliases when saved. If it works, the module might be non-destructive. A bit more code, but we'd be messing with less data.

@Sam152

Does your tests cover the case of installing the module on an existing site with non-neutral aliases, updating the alias for an existing node and checking that it's indeed the neutral alias that's used, regardless of the users language?

This has been fixed in the 2.x branch.

It now overrides PathAliasStorage of the path_alias entity to ensure that aliases are saved language neutral and decorates the AliasRepository service to only return neutral aliases. Subclasses the path_alias entitys list builder to only list neutral aliases.

If we could get some more people testing (ideally on existing installations), we can cut the 2.0 release.

I've installed dev-2.x (bfe54de) on a test site with seven languages and 30+ content types. So far it's working well, but we're testing it manually thoroughly before pushing to production.