Allow removing locales

[jakubtobiasz]

Q	A
Branch?	1.13
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	fixes #10183
License	MIT

Only unused locales can be removed. If a locale appears in any translation, automatically, it cannot be removed.

[NoResponseMate]

I don't really see a point in limiting it only to CLI, UX-wise.
Since we can only delete not used locales, then I'd assume it'll be mostly used for accidental locales, if it'd be in UI, no problem, just click a thing on the list.
With it being in CLI, after I create a locale by mistake I need to either go on the server directly or inform someone that manages it to run the command.
It's much more hassle for the same effect. The only upside I can see is settings a cron to periodically remove unused locales, but that way can have so many potential issues that I doubt anyone would use it.

The situation would be completely different if we actually allowed deleting locales that are used, which we don't.

I'd be for changing this to solution 2, but without the conditional showing of the button, and just keeping it in line with other resources that sometimes cannot be deleted (show an error flash and leave the resource be)

[GSadee]

I have not commented before, but after thinking about it and the comment from @NoResponseMate, I would also favour the second option. To add the ability to delete only unused locales in the admin panel, I don't really see any real benefit from adding this functionality to the CLI only 🤔

I am also interested in other opinions

[jakubtobiasz]

Refactored 👯‍♂️. I see behats are failing, but you can still review 🕺🏻.

[lchrusciel]

That's a lot of additional queries for simple delete. We would request up to n queries equal to the number of translatable entities in the system. Can't we base it on ForeignKeyConstraintViolationException or something similar?

[jakubtobiasz]

There's no foreign key to sylius_locale.

I know it might seem like many queries, but:

• Removing a locale is not a standard action for e-commerce (usually, store administrators don't delete a locale every morning)
• This operation might be performed by a small group of people
• In the best scenario it'd be only 1 query, at worst a number of *Translation entities.

I believe it won't affect any store.

[jakubtobiasz]

@lchrusciel all changed applied.
Admin custom action -> sylius.locale.pre_delete event listener
API custom action -> decorated data persister

[Rafikooo]

Wdyt?

Suggested change

	Sylius\Bundle\LocaleBundle\Checker\Exception\LocaleIsUsedException: 422
	Sylius\Bundle\LocaleBundle\Checker\Exception\LocaleInUseException: 422

[jakubtobiasz]

As we don't have any standard for this, I don't have any strong opinion. I guess both names are correct.

I left the initial name, but I'm leaving this suggestion unresolved for any other opinions.

[TheMilek]

Thank you, Jacob! 🎉