Problem/Motivation

Getting following error/warnings

FILE: /app/modules/contrib/content_moderation_notifications/README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------
50 | WARNING | Line exceeds 80 characters; contains 85 characters
75 | WARNING | Line exceeds 80 characters; contains 100 characters
84 | WARNING | Line exceeds 80 characters; contains 104 characters
85 | WARNING | Line exceeds 80 characters; contains 87 characters
87 | WARNING | Line exceeds 80 characters; contains 105 characters
93 | WARNING | Line exceeds 80 characters; contains 99 characters
94 | WARNING | Line exceeds 80 characters; contains 94 characters
----------------------------------------------------------------------

FILE: /app/modules/contrib/content_moderation_notifications/src/ContentModerationNotificationsAccessController.php
------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------
24 | WARNING | Possible useless method overriding detected
------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/content_moderation_notifications/src/Form/ContentModerationNotificationsFormBase.php
---------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
150 | ERROR | [x] Use null coalesce operator instead of ternary operator.
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/content_moderation_notifications/src/Notification.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
----------------------------------------------------------------------------------------------
129 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
141 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
159 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
184 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
206 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/content_moderation_notifications/tests/src/Functional/Form/CrudFormTest.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 16 WARNINGS AFFECTING 16 LINES
------------------------------------------------------------------------------------------------------------------------------------------
73 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
86 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
91 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
124 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
126 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
141 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
148 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
152 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
159 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
163 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
164 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
165 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
178 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
180 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
181 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
182 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/content_moderation_notifications/tests/src/Kernel/TokenNotificationsTest.php
-------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------
110 | WARNING | Unused variable $notification.
-------------------------------------------------------------------------------------------------------

Time: 1.5 secs; Memory: 12MB

Steps to reproduce

Run following command

phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/content_moderation_notifications/

Proposed resolution

Above error/warnings need to be fixed

Note:

I have removed following line of code as it only called it's parent checkAccess function nothing else from src/ContentModerationNotificationsAccessController.php file. We can add this later if there is any specific code for the same.

/**
 * {@inheritdoc}
 */
public function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) {
  // No special access handling. Defer to the entity system which will only
  // allow admin access by default.
  return parent::checkAccess($entity, $operation, $account);
}
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:

  • 3328951-attempt3 Comparecompare
  • 3 hidden branches
  • 8.x-3.x Comparechanges, plain diff MR !14
  • 3328951-attempt2 Comparecompare
  • 3328951-fix-the-issues Comparecompare

Comments

samit.310@gmail.com created an issue. See original summary.

samitk’s picture

Assigned: samitk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new16.44 KB

Error/warnings are fixed.

gaurav-mathur’s picture

Assigned: Unassigned » gaurav-mathur
gaurav-mathur’s picture

Assigned: gaurav-mathur » Unassigned

Verified and tested patch #2 on Drupal 10.1.x-dev and do not see any error or warnings.
Patch applied successfully.
Thank You.

sourabhjain’s picture

Status: Needs review » Reviewed & tested by the community

+1 RTBC

avpaderno’s picture

Title: Drupal Coding Standards Issues | phpcs » Fix the issues reported by phpcs
Category: Bug report » Task
Priority: Normal » Minor
Status: Reviewed & tested by the community » Needs work
Issue tags: -, - +Coding standards
-Message:  (Can support all the same Twig options, but will run template output through the selected
-           Text Format's (Basic HTML, Full HTML, etc) Input Filters  before it will be mailed)
+Message:  (Can support all the same Twig options, but will run template output
+ through the selected
+           Text Format's (Basic HTML, Full HTML, etc) Input Filters  before
+            it will be mailed)

through the selected is much shorter than 80 characters.
The last two lines are wrongly indented.

-    arguments: ['@current_user', '@entity_type.manager', '@plugin.manager.mail', '@module_handler', '@content_moderation_notifications.notification_information', '@?token.entity_mapper']
+    arguments: ['@current_user',
+                '@entity_type.manager',
+                '@plugin.manager.mail',
+                '@module_handler',
+                '@content_moderation_notifications.notification_information',
+                '@renderer',
+                '@config.factory',
+                '@?token.entity_mapper']

Those lines are wrongly indented. Either arguments value is kept on a single line, or the single values are each placed on a single line.

arguments:
  - '@container.namespaces'
  - '@module_handler'
  - '@cache.discovery'
  - '@string_translation'
  - '@class_resolver'
  - '@entity.last_installed_schema.repository'
-  public function __construct(AccountInterface $current_user, EntityTypeManagerInterface $entity_type_manager, MailManagerInterface $mail_manager, ModuleHandlerInterface $module_handler, NotificationInformationInterface $notification_information, TokenEntityMapperInterface $token_entity_mappper = NULL) {
+  public function __construct(AccountInterface $current_user,
+                              EntityTypeManagerInterface $entity_type_manager,
+                              MailManagerInterface $mail_manager,
+                              ModuleHandlerInterface $module_handler,
+                              NotificationInformationInterface $notification_information,
+                              RendererInterface $renderer,
+                              ConfigFactoryInterface $config_factory,
+                              TokenEntityMapperInterface $token_entity_mappper = NULL) {

As per Drupal coding standards, method declarations can be on a single line, if they are more readable. The existing code is already correct.

nitin_lama’s picture

Assigned: Unassigned » nitin_lama

nitin_lama’s picture

Status: Needs work » Needs review

Please review.

nitin_lama’s picture

Assigned: nitin_lama » Unassigned
roberttabigue’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new611.85 KB
new90.07 KB

Hi,

Confirmed no PHPCS errors after applying the latest MR to the Content Moderation Notifications module with version 8.x-3.x-dev and with Drupal core version 9.5.8.

Marking this now as RTBC.
Kindly refer to the attached screenshots, please.

avpaderno’s picture

Status: Reviewed & tested by the community » Needs work

The report shows errors/warnings for six files, but the MR changes seven files. The number of reported files and the number of changed files must match.

bkosborne made their first commit to this issue’s fork.

bkosborne changed the visibility of the branch 3328951-attempt2 to hidden.

bkosborne changed the visibility of the branch 3328951-fix-the-issues to hidden.

bkosborne changed the visibility of the branch 8.x-3.x to hidden.

bkosborne’s picture

Status: Needs work » Closed (duplicate)

Looks like the commits were made into the main 8.x-3.x branch of the fork and I'm having a really hard time merging upstream into it. I'm going to close this issue and create a new one, fix the issue there, and transfer contribution credits.

See #3328951: Fix the issues reported by phpcs

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.