Problem/Motivation

Fix the t() function in Controllers that do not have the StringTranslationTrait.

Steps to reproduce

Proposed resolution

Fix Controllers in non tests that do not have the StringTranslationTrait for the sniff 'DrupalPractice.Objects.GlobalFunction.GlobalFunction'
That should be these files

  • core/modules/system/src/Controller/SystemInfoController.php
  • core/modules/file/src/Controller/FileWidgetAjaxController.php
  • core/modules/config/src/Controller/ConfigController.php

Remaining tasks

Update patch in accord with the new scope
review
commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3338541-fix-controllers-with-6.patch4.45 KBrassoni

Issue fork drupal-3338541

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

quietone created an issue. See original summary.

royalpinto007’s picture

Assigned: Unassigned » royalpinto007

royalpinto007’s picture

Assigned: royalpinto007 » Unassigned
Status: Active » Needs review

Hi @quietone,

With this commit, I've refactored the ConfigController class by implementing the ContainerInjectionInterface and utilizing dependency injection to pass in its required dependencies. This change improves the code maintainability and flexibility.

Please let me know if you have any questions or concerns.

Thanks!

quietone’s picture

Status: Needs review » Needs work

@royalpinto007, Welcome to Drupal! Thanks for the description of the changes!

The change is failing the pre commit checks. You can run these locally before submitting a change, Run core development checks.

rassoni’s picture

Status: Needs work » Needs review
StatusFileSize
new4.45 KB

Fixed failing the pre commit checks.

Status: Needs review » Needs work

The last submitted patch, 6: 3338541-fix-controllers-with-6.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review

@Rashmisoni, thanks for working on this. It is preferred that we stay with the existing merge or patch workflow instead of switching. As a reviewer it can take a while to figure out which should be reviewed. Oh, It is not necessary to test a patch on every combination of php and database. Just take the default option. If you look at the failures you will see that it is a FunctionalJavascript test that is failing. Drupal has a number of those tests that fail randomly. You can learn more about that in #2829040: [meta] Known intermittent, random, and environment-specific test failures.

Setting back to Needs review,

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

The random failure in #6 seems to be unrelated.

Applied patch locally and verified instances of t() have been replaced with this->t()

The issue summary mentions 3 files and that's all that was covered so good there.

Think this is good.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, thanks!

  • catch committed 54dda45b on 10.1.x
    Issue #3338541 by royalpinto007, Rashmisoni, quietone: Fix Controllers...
jonathan1055’s picture

Title: Fix Controllers with out trait in non tests for 'DrupalPractice.Objects.GlobalFunction' » Fix Controllers without trait in non tests for 'DrupalPractice.Objects.GlobalFunction'
Issue summary: View changes

Fixed spelling in title and corrected the sniff in issue summary.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.