Drupal core's coding standards forbid translated exceptions, but does that anyway — f.e. FileValidationException

Created on 2 February 2023, almost 2 years ago

Updated 14 February 2023, almost 2 years ago

Problem/Motivation

Discovered at #3325654-32: Improve the user experience of having your staged update deleted before it was applied → .

Well well well, 🐛 phpcs stopped working since the switch to testing on Drupal 10.0.x by default Fixed is already striking:
----------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
----------------------------------------------------------------------
 699 | WARNING | Exceptions should not be translated
     |         | (DrupalPractice.General.ExceptionT.ExceptionT)
 704 | WARNING | Exceptions should not be translated
     |         | (DrupalPractice.General.ExceptionT.ExceptionT)
 712 | WARNING | Exceptions should not be translated
     |         | (DrupalPractice.General.ExceptionT.ExceptionT)
----------------------------------------------------------------------
… and it's wrong, because \Drupal\automatic_updates\Form\UpdateReady::buildForm() will catch this and present it to the end user:
  public function buildForm(array $form, FormStateInterface $form_state, string $stage_id = NULL) {
    try {
      $this->updater->claim($stage_id);
    }
    catch (StageOwnershipException $e) {
      $this->messenger()->addError($e->getMessage());
      return $form;
    }
…
… but of course, we do have to comply with Drupal core's phpcs rules.

Not sure how to proceed here. It seems that "exceptions for user-facing strings (that should be translated)" is a bad practice, but this module has embraced that. It's technically out of scope to fix here though.

So … worked around it, and tagging .

Steps to reproduce

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

None.

API changes

TBD

Data model changes

TBD

📌 Task

Status

Active

Version

3.0

Component

Code

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.

core-post-mvp

Comments & Activities

Issue created by @wim leers
Comment almost 2 years ago →
🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Actually, \Drupal\package_manager\Exception\StageValidationException literally contains validation errors, which of course need to be translated.

And core has precedent for this too: \Drupal\file\Upload\FileValidationException.

Conclusion: the phpcs rule simply does not make sense!
Comment almost 2 years ago →
🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
As of https://git.drupalcode.org/project/automatic_updates/-/merge_requests/63..., 📌 Improve the user experience of having your staged update deleted before it was applied Fixed is now referring to this issue to eventually rectify this in Drupal core.
Comment almost 2 years ago →
🇺🇸United States tedbow Ithaca, NY, USA

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024