Inject and allow for the translation of the modules various strings

Created on 5 May 2023, over 1 year ago
Updated 19 May 2023, over 1 year ago

Problem/Motivation

The module uses the translate function in places and should use the injected service.
Not all strings are made available for translation, these should also use the translation service.
e.g.: $this->t("Site Guardian has not been activated for this site. Please contact a site administrator.")

Steps to reproduce

View the code
Install the module
Try to translate the various strings

Proposed resolution

Replace the translation function with the injected service and make all strings translatable.

Remaining tasks

Find all strings
Ensure they are available to the translation service
Replace the translation function with the injected service

User interface changes

More strings available

API changes

As above

Data model changes

None

✨ Feature request
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom the_g_bomb

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

Comments & Activities

Not all content is available!

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

  • Issue created by @the_g_bomb
  • Assigned to i-trokhanenko
  • πŸ‡ΊπŸ‡¦Ukraine i-trokhanenko Lutsk πŸ‡ΊπŸ‡¦
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡¦Ukraine i-trokhanenko Lutsk πŸ‡ΊπŸ‡¦

    Injected a StringTranslationTrait service to the SiteGuardianService and made some strings translatable.
    I haven't touched the logger messages because usually the Drupal community doesn't translate errors/warnings/notices.
    Please review the patch.

  • πŸ‡¬πŸ‡§United Kingdom the_g_bomb

    Patch looks good to me, will try to give it a more in depth look asap. Thank you

  • Assigned to seeduardo
  • πŸ‡¬πŸ‡§United Kingdom the_g_bomb
  • First commit to issue fork.
  • Issue was unassigned.
  • πŸ‡ΊπŸ‡¦Ukraine i-trokhanenko Lutsk πŸ‡ΊπŸ‡¦

    Injected a TranslationInterface to SiteGuardianService
    Please review patch #8

  • Status changed to Needs work over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom the_g_bomb

    Seeing problems now when I try to access the endpoints at:
    /admin/config/development/site_guardian/endpoints

    Path: /site_guardian/enabled_modules_and_updates?site_guardian_key=pPRsS080_LgN6Z3x1jt5Gu-AogzHUo80hylFW1Z9nXEVG926tnuLEoxEPBf22y1hYh4JicwGEA. Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException: in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 118 of /var/www/html/docroot/core/lib/Drupal/Core/Routing/AccessAwareRouter.php).

    I suspect it has something to do with the fact that the accessCheck in the service SiteGuardianService.php is using the 'Checks passed' string which is now being translated in the checkActivationAndSiteGuardianKey method.

    I wonder if instead of doing:
    return $this->t('Checks passed');
    it might be better to just do:
    return TRUE then check for the TRUE instead of a String.

  • @the_g_bomb opened merge request.
  • Status changed to Needs review over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom the_g_bomb
  • πŸ‡¬πŸ‡§United Kingdom the_g_bomb

    I am not wanting to hold up this ticket, but I was wondering if the code could be improved here as well.

        $saved_key = $this->settings->get('site_guardian_key');
        if ($saved_key && (empty($site_guardian_key) || $site_guardian_key != $saved_key)) {
          \Drupal::logger('site_guardian')->warning("Site Guardian request received with an incorrect key, access was denied.");
          return FALSE;
        }
    

    It could become

        $saved_key = $this->getKey(');
        if (!empty($saved_key) || !empty($site_guardian_key) || ($site_guardian_key != $saved_key)) {
          \Drupal::logger('site_guardian')->warning("Site Guardian request received with an incorrect key, access was denied.");
          return FALSE;
        }
    
  • Status changed to RTBC over 1 year ago
  • πŸ‡ΊπŸ‡¦Ukraine i-trokhanenko Lutsk πŸ‡ΊπŸ‡¦

    MR#1 works fine on Drupal 9.5.8 and Drupal 10.0.8. Checked with simplytest.me. Marking as RTBC. Thanks!

  • Also checked with simplytest.me, code looks good too, thanks both.

  • Status changed to Fixed over 1 year ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024