[1.0.x] Simple Content Notifications

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

1. FILE: simple_content_notifications.info.yml

core_version_requirement: ^8 || ^9

The Drupal Core versions before 8.7.7 do not recognize the core_version_requirement: key.

2. FILE: src/Form/ContentNotificationSettingsForm.php and src/Form/ContentReviewNotificationSettingsForm.php

  /**
   * {@inheritdoc}
   */
  public function validateForm(array &$form, FormStateInterface $form_state) {

  }

You can remove this method since it is not used.

Rest looks fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

Please fix the below PHPCS issue:

FILE: /simple_content_notifications/README.md
----------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------
 8 | WARNING | Line exceeds 80 characters; contains 84 characters
----------------------------------------------------------------------------------

Please fix the below PHPCS issues:

FILE: /simple_content_notifications/src/Form/ContentReviewNotificationSettingsForm.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------
  62 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 154 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------------------

Automated Review

FILE: /var/www/html/web/modules/contrib/simple_content_notifications/simple_content_notifications.module
-----------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\node\Entity\Node.
-----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/simple_content_notifications/src/Form/ContentReviewNotificationSettingsForm.php
-----------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\ConfigFactoryInterface.
-----------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------------

Manual Review Suggestions for Improving Code

Coding Style and Naming:

1. Make sure that your code is consistent in using similar words throughout. For example, if you use "Needing" in one place, use it consistently instead of mixing it with "Needs", for instance, your controller name is \Drupal\simple_content_notifications\Controller\NeedsReview, but all the routing and form names are Content Needing Review, etc. This will help avoid confusion.

Fixing a Missing Slash:

2. From ContentReviewNotificationSettingsForm, a forward slash is missing. Instead of saying @var Drupal\Core\State\StateInterface, it should be @var \Drupal\Core\State\StateInterface.

Simplifying Code:

3. There's an opportunity to simplify your code by using a function called str_contains instead of complex logic. For instance, this code:

$on_live_site = (strpos($base_url, $live_domain) !== FALSE) ? TRUE : FALSE;

Can be made simpler like this:

$on_live_site = str_contains($base_url, $live_domain);

Improving Code Reusability:

4. To make your code more efficient and easier to understand, consider using the "Code Reusability" concept. Instead of repeating the same code to fetch configuration settings, you can consolidate it. For instance, this:

$logging_enabled = \Drupal::config('simple_content_notifications.review_settings')->get('review_notifications_logging');
$due = \Drupal::config('simple_content_notifications.review_settings')->get('review_notifications_due');
$field = \Drupal::config('simple_content_notifications.review_settings')->get('review_notifications_field');
$published_only = \Drupal::config('simple_content_notifications.review_settings')->get('review_notifications_published');

Could be improved to:

$config = \Drupal::config('simple_content_notifications.review_settings');
$logging_enabled = $config->get('review_notifications_logging');
$due = $config->get('review_notifications_due');
$field = $config->get('review_notifications_field');
$published_only = $config->get('review_notifications_published');

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the Slack #contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the reviewers.