[1.0.x] Content expiry tracker

FILE: content_expiry_tracker.module

<?php
/**
 * @file
 * Primary module hooks for Content Expiry Tracker module.
 */

The usual description for a .module file is “Hook implementations for the [module name] module”, where [module name] is the module name given in the .info.yml file.

Use #config_target in ConfigFormBase forms

File: SettingsForm.php

With Drupal 10 and Drupal 11, there is no longer need to use #default_value for each form element, when the parent class is ConfigFormBase: It is sufficient to use #config_target, as in the following code.

    $form['image_toolkit'] = [
      '#type' => 'radios',
      '#title' => $this->t('Select an image processing toolkit'),
      '#config_target' => 'system.image:toolkit',
      '#options' => [],
    ];

Using that code, it is no longer needed to save the configuration values in the form submission handler: The parent class will take care of that.
For this change, it is necessary to require at least Drupal 10.3, but that is not an issue, since Drupal 10.2.x is no longer supported.

Use Constructor Property Promotion

File: ContentExpiryEvent.php

- New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use property promotion. Applies to all classes.
Example:

  public function __construct(
    protected EntityTypeManagerInterface $entityTypeManager,
    protected EntityRepositoryInterface $entityRepository,
    protected DateFormatterInterface $dateFormatter,
    TranslationInterface $translation,
  ) {
    $this->setStringTranslation($translation);
  }

Add tests

Id like to recommend adding some tests to the code. Adding tests is a great way to make sure edge cases work as expected and that you don’t introduce regressions as you update your module on the future.

@circuitcipher

It is preferable to wait for a project moderator before posting the first comment on newly created applications. Project moderators will do some preliminary checks that are necessary before any change on the project files is suggested.

Thank you for applying!

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.

The important notes are the following.

• New releases are not necessary for these applications, which could require changes that are not backward-compatible. Not creating new releases avoids any possible issue.
• Please do not change the branch to review once reviews started, except in the case the used branch needs to be deleted.
• If you have not done it yet, enable GitLab CI for the project, and fix what reported from the phpcs job. This help to fix most of what reviewers would report.
• For the time this application is open, only your commits are allowed. No other people, including other maintainers/co-maintainers can make commits.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application; when the application status is changed to Fixed, you will be able to change it and opt it into security advisory coverage.
• Nobody else will get the permission to opt projects into security advisory policy. If there are other maintainers/co-maintainers who need to get that permission, they need to apply with a different module.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

• It is preferable to wait for a Project moderator before commenting on newly created applications. Project moderators will do some preliminary checks that are necessary before any change on the project files is suggested.
• Reviewers should show the output of a CLI tool only once per application. The configuration used for these tools needs to be the same configuration used by GitLab CI, stored in the GitLab Templates repository.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

1. The README file is missing the required sections - Requirements and Configuration.

2. FILE: composer.json

    "require": {
        "drupal/core": "^11.0"
    },

It is not necessary to add the Drupal core requirements in the composer.json file: The Drupal.org Composer Façade will add them.

3. FILE: content_expiry_tracker.libraries.yml

version: VERSION

VERSION is only used by Drupal core modules. Contributed modules should use a literal string that does not change with the Drupal core version a site is using.

I reviewed the 1.0.x branch of content_expiry_tracker (commit e0591b3).

### Previous issues status (vishal.kadam, Comment #8)

All three resolved:
1. README missing sections -- fixed, now includes Requirements, Installation, Configuration, and Usage.
2. `drupal/core` in composer.json -- fixed, removed.
3. `version: VERSION` in libraries.yml -- fixed, now uses `1.0.x`.

### Security

No security issues found. All routes have proper permission-based access control. The custom access check on the mark-reviewed route uses both module permission and entity-level update access. Entity queries use parameterized conditions (no SQL injection). Output uses `$this->t()` with `@` placeholders (XSS-safe). Forms go through Form API (CSRF protected). No raw database queries, no debug code, no bundled third-party libraries, no `\Drupal::` static calls in service classes.

### Issues found

1. **Missing leading backslash in `.install` file** (lines 23, 29): `Drupal::entityDefinitionUpdateManager()` should be `\Drupal::entityDefinitionUpdateManager()`. There is no `use` statement for the `Drupal` class in this procedural file.

2. **Loose comparison** in `ContentExpiryEntityHooks.php` line 188: `$calculatedExpiry != $originalExpiry` should use `!==` (both values are strings).

3. **Missing cache metadata on `AccessResult` objects** in `ContentExpiryEntityHooks::controlExpiryFieldAccess()`: The forbidden result on line 121 depends on the `enable_email_recipients` config value but does not call `->addCacheableDependency($config)`. The permission-based forbidden result should call `->cachePerPermissions()`. Same for `ExpiryController::checkReviewAccess()` -- should add `->cachePerPermissions()`.

4. **Stale docblock** in `CronHooks.php` line 34: Says "between 2am and 3am server time" but this restriction was removed.

5. **Incorrect PHPDoc** in `ExpiryManager.php` line 172: `/** @var \Drupal\node\NodeInterface $nodes[] */` should be `/** @var \Drupal\node\NodeInterface[] $nodes */`.

6. **Dead code** in `ExpiryManager::getExpiredNodes()` line 274: `$currentTime` is assigned but never used.

7. **README.md**: Missing "Maintainers" section per the [Drupal README template](https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...).

The module is well-structured overall -- good use of OOP hooks with `#[Hook]` attributes, constructor property promotion, dependency injection, proper config schema, and a clean event-driven architecture. All issues above are quick fixes. Setting to **Needs work**.

@oivanov since u are using AI to help drafting your feedback, u could at least make an effort to format the output from Markdown to html or plain text so that people can easily read it...

me personally - I prefer Markdown to everything else, so I make an effort not to have to deal with HTML (definitely) or plain text (when I can).
If markdown is not acceptable - we can add that to the guidance. Let me know if you want me to do that

All 7 fixes look good to me, everything has been properly addressed.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or does not correctly use the Drupal API; the single points are not ordered, not even by importance

src/Controller/ExpiryController.php

Since that class does not use methods from the parent class, or it uses a single method from the parent class, it does not need to use ControllerBase as parent class.

Controllers do not need to have a parent class; as long as they implement \Drupal\Core\DependencyInjection\ContainerInjectionInterface, they are fine.

Thank you for your contribution and for your patience with the review process!

I am going to update your account so you can opt into security advisory coverage any project you create, including the projects you already created.

These are some recommended readings to help you with maintainership:

• Dries' post on Responsible maintainers
• Maintainership
• Git version control system
• Issue procedures and etiquette
• Maintaining and responding to issues for a project
• Release naming conventions.

You can find more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!
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 also all the reviewers for helping with these applications.