Deprecate global constants in locale module

Reparenting to #3215707: [META] Modernize Locale module where it has more likeliness to be fixed now. I think this may be resolved piece by piece as various include files and global functions that use the constants become services. So it may be that no direct work will be needed in this issue.

I think grouped constants should be moved to backed enums:

For instance LOCALE_TRANSLATION_USE_SOURCE_LOCAL and LOCALE_TRANSLATION_USE_SOURCE_REMOTE_AND_LOCAL:

enum LocaleSourceType: string {

  case Local = 'local';
  case RemoteAndLocal = 'remote_and_local';

  public static function asOptions(): array {
    return [
      self::Local->value = t('Drupal translation server and local files'),
      self::Local->value = t('Local files only'),
    ];
  }
}

Same for LOCALE_TRANSLATION_OVERWRITE_* and LOCALE_TRANSLATION_* and LOCALE_TRANSLATION_SOURCE_COMPARE_*

Let's try...

Update IS

Update IS

Updated the CR

I'd propose that the constants in locale.translations.inc

• LOCALE_TRANSLATION_SOURCE_COMPARE_LT
• LOCALE_TRANSLATION_SOURCE_COMPARE_EQ
• LOCALE_TRANSLATION_SOURCE_COMPARE_GT

Be addressed in: #3569328: Modernize locale.translations.inc
It keeps everything nicely self contained.

Did some progress but I still need to reflect on BC. This issue is part of modernizing Locale meta, and I think we need some refactoring not only moving the code around.

Updating IS

This is ready for review

Couple of questions, I didn't finish my review.

Postponing on #3577671: Modernize locale file handling #3037031: Convert locale.compare.inc to a service and the batch conversion in .inc I haven't created yet.

I spoke briefly with @claudiu.cristeau about the overall order and he agreed this is ok.

#3581303: Convert locale batch callbacks

Looking at what remains, this is actually the easiest to do, we should probably do this last since it won't require any dependency injection changes.

The issue i just related looks like a duplicate https://www.drupal.org/project/drupal/issues/3401493

I began working through updating this.

I'm doing it from scratch solved most has changed, but I'm using the CR as a template.

I mostly agree, the local constants on the setting form I changed to an enum since they are used in tests, I noticed a few weren't directly changed since I assumed you didn't want dependencies.

I'm not sure that's the right route, but I've generally been following that too.

I tweaked some naming, I assigned myself so I can finish in the morning.

It actually ended up being quite a bit smaller with the refactoring that has happened since.

I went with backed enums for consistency in all cases, that also allowed us to drop the update hook.

I created a third option where I replaced the customized constant with an enum so it's easier to see where it is.

OK I changed the two tests, they already have a soft dependency on locale, so adding the enum there is fine.

I also added the two helpers suggested by @berdir, it cleans up quite a bit.

I created a follow up: #3619605: Clean up current workflow for locale source, also clean up saveSource

I'll update the CR and IS in the morning if this makes sense.

Updated the replacement table to reflect the current state of the MR

Thanks @feuerwagen!

I updated it again just now to match what I just pushed up.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Did another full pass, hopefully after that I can RTBC.

As to the why we had transaction deprecations in the previous MR, that is due to PoDatabaseWriter::writeItems() missing to handle an exception if that occurs during the transaction. A transaction is opened, then because of the SQL error due to

SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'customized' at row 1

an exception is thrown, but not managed. So it bubbles up and when the transaction object is losing scope, a commit-on-destruct is attempted and the deprecation thrown. I daresay that is a bug, because in general you may end up having only partial translations (if I understand the code here).

With explicit transaction commit, we now need to be carefully dealing with the unhappy path as well as with the happy one.

I enclosed the operations occurring during the transction in a try...catch construct, so that if a Throwable is occurring during the transaction the entire db transaction is rolled back, and now the deprecations are gone.

This opens an interesting question @catch @daffie - once we eventually remove the commit-on-destruct code, what shall we leave in its place? Do we still throw an out-of-order transaction exception to notify developers they have mishandled the db transaction, or do we simply silently rollback it (which is PHP's default behavior)?

Thanks @mondrake!

Created:
#3619927: [pp-1] Properly handle exceptions in PoDatabaseWriter
#3619926: [pp-1] Review replacing the 600 ttl for translation timeout

I addressed the latest round of feedback as well, thanks!

I think this is pretty OK now.

If I could pick, I'd rather do some more work on other cleanup, deprecation and refactoring issues which will likely reduce the number of constants we have/need. The SourceType for example might eventually just go away completely or at least become all internal to the TranslationSource object.

But this is one of the last two issues to fully deprecate the .module file, semi-blocking the formal deprecation of .module files, which we want to get done asap. We can also do the cleanup and refactoring issues after this, doesn't make those harder, at worst it results in some confusing double deprecations over time.

The decision when to use an enum vs class constants is not entirely obvious in cases, could go either way. And we didn't want to deal with changing API's too much that we'll likely refactor further or away completely in other issues, such as saveSource(), nor does it seem worth the hassle to convert the settings config object to use enum objects directly and deal with BC around that.

Committed and pushed 39b62a8288e to main. Thanks!

Needs a rebase for 11.x.

Haven't published the CR yet, I'll wait for the 11.x commit.

The conflict was the constant deprecations, so it was an easy resolution.

There were also three usages in 11.x not on main:
https://git.drupalcode.org/project/drupal/-/merge_requests/16930/diffs?c...

The backport looks good :) Committed d185904 and pushed to 11.x. Thanks!

Published the CR as well.