Remove t() calls from classes

Created on 3 June 2020, about 4 years ago
Updated 31 October 2023, 8 months ago

In the src/MessageDigestIntervalListBuilder.php file, t() is called instead of using \Drupal\Core\StringTranslation\StringTranslationTrait and calling $this->t(). This needs to be changed, since classes should not use t().

📌 Task
Status

Needs review

Version

1.0

Component

Code

Created by

🇮🇳India mo_farhaz Bangalore

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    Patch Failed to Apply
  • 🇮🇳India Ashutosh Ahirwal India

    Providing new patch with phpcs issue fixes.

  • Status changed to Needs work about 1 year ago
  • 🇮🇳India arpitk

    I reviewed the patch #9. The patch is failing to apply. Moving it to Needs Works to fix the issue and update the patch.

    Thanks!

  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -    $schema->addIndex('message_digest', 'aggregate', ['timestamp', 'sent', 'notifier'], $spec);
    +    $schema->addIndex('message_digest', 'aggregate', [
    +      'timestamp', 'sent', 'notifier',
    +    ], $spec);

    Since this issue focused on removing any call to t() from classes, let's focus on that change. Otherwise, this becomes just another Fix the issues reported by phpcs issue.

  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    36 pass, 1 fail
  • @shivam_tiwari opened merge request.
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    36 pass, 1 fail
  • Status changed to Needs review 12 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    37 pass
  • 🇮🇳India Ajeet Tiwari

    Please review.

  • Status changed to Needs work 12 months ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Test classes should not use translated strings. The last patch does not change the code for the MessageDigestIntervalFormTest class.
    Also, since a MR has been used, it would be better to keep using that, instead of creating patches.

  • Assigned to nitin_lama
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    36 pass, 1 fail
  • Issue was unassigned.
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    +  /**
    +   * MessageDigestIntervalListBuilder constructor.
    +   *
    +   * @param \Drupal\Core\StringTranslation\TranslationInterface $stringTranslation
    +   *   The string translation service.
    +   */
    +  public function __construct(TranslationInterface $stringTranslation) {
    +    $this->setStringTranslation($stringTranslation);
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) {
    +    $entity_type_manager = $container->get('entity_type.manager');
    +    return new static(
    +      $entity_type,
    +      $entity_type_manager->getStorage($entity_type->id()),
    +      $entity_type_manager->getDefinitions()
    +    );
    +  }
    

    The correct code is similar to the following one.

    /**
     * Constructs a new \Drupal\message_digest\MessageDigestIntervalListBuilder object.
     *
     * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type
     *   The entity type definition.
     * @param \Drupal\Core\Entity\EntityStorageInterface $storage
     *   The entity storage class.
     * @param \Drupal\Core\StringTranslation\TranslationInterface $stringTranslation
     *   The string translation service.
     */
    public function __construct(EntityTypeInterface $entity_type, EntityStorageInterface $storage, TranslationInterface $string_translation) {
      parent::__construct($entity_type, $storage);
      $this->setStringTranslation($string_translation);
    }
    
    /**
     * {@inheritdoc}
     */
    public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) {
      return new static($entity_type,
        $container->get('entity_type.manager')->getStorage($entity_type->id()),
        $container->get('string_translation')
      );
    }
    

    Test classes should not use translated strings. I think that is what test classes in Drupal core do.

  • Status changed to Needs review 12 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    36 pass, 1 fail
  • 🇮🇳India chaitanyadessai

    Please review patch.

  • First commit to issue fork.
  • Merge request !15Resolve #3145304 "Remove some ts" → (Open) created by bluegeek9
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 8 months ago
    37 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 8 months ago
    37 pass
  • 🇺🇸United States bluegeek9

    I updated src/Plugin/Notifier/DigestBase.php. On my local machine, the assert function was not throwing an exception.

    I updated it to throw a InvalidDigestGroupingException, but now I am not sure if this is the best exception to throw.

Production build 0.69.0 2024