[1.0.x] Taxonaut

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.

• If you have not done it yet, you should enable GitLab CI for the project and fix the PHP_CodeSniffer errors/warnings it reports.
• For the time this application is open, only your commits are allowed.
• 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 will not be changed by this application; once this application is closed, you will be able to change the project status from Not covered to Opt into security advisory coverage. This is possible only 14 days after the project is created.
  
  Keep in mind that once the project is opted into security advisory coverage, only Security Team members may change coverage.
• Only the person who created the application will get the permission to opt projects into security advisory coverage. No other person will get the same permission from the same application; that applies also to co-maintainers/maintainers of the project used for the application.
• 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 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.
• Reviewers should show the output of a CLI tool only once per application.
• 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.

Thank you for submitting Taxonaut for review. After going through the module
based on current Drupal coding standards, here are the issues that need to be
addressed before approval.

1. FILE: src/Controller/TermController.php
   
   validateCsrfToken() is declared with a void return
   type but is called in a boolean context:

   if                             
     (!$this->validateCsrfToken($request))

   . Since void always
   returns null, !null evaluates to true
   on every call, meaning all write operations (add, update, delete, move, merge,
   bulk) will always return an error response. The return type must be changed
   to bool and the method must return true on a valid
   token.

2. FILE: src/Controller/TermController.php
   
   AccessDeniedHttpException is used inside
   validateCsrfToken() but is not imported. Add the missing use
   statement:

   use
     Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

3. FILE: src/Service/RevisionManager.php
   
   logOperation() uses ->condition('vid', $vid) to
   clear undone revisions, but the column name in the
   taxonaut_revisions table is vocabulary_id. The
   delete query never matches any rows, so the redo stack is never cleared.
4. FILE: src/Controller/SnapshotController.php
   
   saveSnapshot() queries taxonomy_term entities using
   ->condition('vocabulary_id', $vid), but the correct entity field
   name is vid. All saved snapshots will have an empty term count
   and empty hierarchy data.
5. FILE: src/Controller/SnapshotController.php
   
   listSnapshots() reads $snapshot->label, but the
   database column defined in the schema is name. Snapshot names
   will always be empty in the listing.
6. FILES: src/Controller/TermController.php,
   src/Controller/RevisionController.php,
   src/Controller/SnapshotController.php
   
   validateCsrfToken() is copied identically across all three
   controllers. Extract it to a shared trait at
   src/Traits/CsrfValidationTrait.php.
7. FILES: src/Controller/TreeController.php,
   src/Controller/TermController.php
   
   getAncestryPath() is duplicated across both controllers. Extract
   it to a shared trait or service.
8. FILES: Multiple OOP class files
   
   \Drupal:: static calls must not be used inside OOP classes. All
   dependencies must be injected via the constructor. Affected files:
   • src/Controller/TreeController.php —
     \Drupal::database()
   • src/Controller/TermController.php —
     \Drupal::csrfToken()
   • src/Controller/RevisionController.php —
     \Drupal::csrfToken()
   • src/Controller/SnapshotController.php —
     \Drupal::database(), \Drupal::time(),
     \Drupal::csrfToken()
   • src/Form/ImportForm.php —
     \Drupal::service('file_system')
9. FILES: Multiple OOP class files
   
   Constructor property promotion is not used in any of the injectable classes.
   Drupal 10+ projects should use PHP 8 constructor property promotion instead of
   separate property declarations and manual $this->x = $x
   assignments. Affected files:
   • src/Service/RevisionManager.php
   • src/Controller/TermController.php
   • src/Controller/TreeController.php
   • src/Controller/RevisionController.php
   • src/Controller/SnapshotController.php
10. FILE: src/Controller/TermController.php
    
    getAncestryPath() and isDescendant() have an untyped
    $storage parameter. Add the appropriate type hint
    (TermStorageInterface).
11. FILE: src/TaxonautPermissions.php
    
    Vocabulary::loadMultiple() is called as a static entity method in
    a non-static class. Inject entity_type.manager via the
    constructor instead.
12. FILE: taxonaut.module
    
    Hooks should be moved to an OOP hook class at
    src/Hook/TaxonautHooks.php using the #[Hook]
    attribute. Procedural #[LegacyHook] shims should remain in
    .module. See change
    record: OOP hooks.
13. FILE: taxonaut.services.yml
    
    Services use manual arguments: injection. Add a
    _defaults block with autowire: true and
    autoconfigure: true and remove the explicit
    arguments: arrays.
14. FILE: src/Form/ImportForm.php
    
    strpos($real_path, $upload_dir) !== 0 should be replaced with
    !str_starts_with($real_path, $upload_dir).
15. FILE: js/taxonaut.js
    
    attach: function (context) { violates the
    object-shorthand ESLint rule. Change to the method shorthand
    form: attach(context) {.
16. FILE: composer.json
    
    The authors section is missing. Add the maintainer name, email,
    role, and homepage.
17. MISSING FILES
    
    The following required files are not present:
    • .cspell.json — required for Drupal.org security coverage
      clearance. Must include a flagWords block per href="https://www.drupal.org/node/3524446">drupal.org/node/3524446 and
      project-specific words in the words array.
    • .gitlab-ci.yml — required for the CI pipeline on
      Drupal.org.
    • config/schema/taxonaut.schema.yml — required configuration
      schema file.
    • config/install/taxonaut.settings.yml — required default
      configuration file.

1. FILE: README.md

The README file is missing the required section - Configuration.

2. FILE: composer.json

There is no need to add the required Drupal version, since that is already added by the Drupal.org Composer façade.

3. FILE: taxonaut.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.

4. FILE: templates/taxonaut-tree.html.twig

Strings shown in the user interface must be translatable. That holds true also for strings used in template files.

5. FILE: src/Controller/RevisionController.php

  /**
   * The revision manager service.
   */
  protected RevisionManager $revisionManager;

  /**
   * Constructs a RevisionController.
   *
   * @param \Drupal\taxonaut\Service\RevisionManager $revisionManager
   *   The revision manager service.
   */
  public function __construct(RevisionManager $revisionManager) {
    $this->revisionManager = $revisionManager;
  }

FILE: src/Controller/TermController.php

  /**
   * The revision manager service.
   */
  protected RevisionManager $revisionManager;

  /**
   * The entity field manager.
   */
  protected EntityFieldManagerInterface $entityFieldManager;

  /**
   * Constructs a TermController object.
   */
  public function __construct(RevisionManager $revision_manager, EntityFieldManagerInterface $entity_field_manager) {
    $this->revisionManager = $revision_manager;
    $this->entityFieldManager = $entity_field_manager;
  }

FILE: src/Service/RevisionManager.php

  /**
   * The database connection.
   */
  protected Connection $database;

  /**
   * The current user.
   */
  protected AccountProxyInterface $currentUser;

  /**
   * The time service.
   */
  protected TimeInterface $time;

  /**
   * Constructs a RevisionManager.
   *
   * @param \Drupal\Core\Database\Connection $database
   *   The database connection.
   * @param \Drupal\Core\Session\AccountProxyInterface $currentUser
   *   The current user.
   * @param \Drupal\Component\Datetime\TimeInterface $time
   *   The time service.
   */
  public function __construct(
    Connection $database,
    AccountProxyInterface $currentUser,
    TimeInterface $time,
  ) {
    $this->database = $database;
    $this->currentUser = $currentUser;
    $this->time = $time;
  }

New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.

6. Enable Gitlab CI

I would suggest enabling GitLab CI for the project, follow the Drupal Association .gitlab-ci.yml template and fix the PHP_CodeSniffer errors/warnings it reports.

Thank you @avpaderno for the guidance and @zeeshan_khan and @vishal.kadam for the detailed review. All issues from both rounds of feedback have been addressed in the latest commits on the 1.0.x branch.

**Round 1 fixes (commit 6cac49c):**

Bug fixes:
- Fixed `validateCsrfToken()` calling pattern in TermController — the void method was used in a boolean context (`if (!$this->validateCsrfToken(...))`), causing all write operations to always return an error response. Now calls directly and lets the exception propagate.
- Fixed `RevisionManager::logOperation()` delete condition from `vid` to `vocabulary_id` to match the actual DB column.
- Fixed `SnapshotController::saveSnapshot()` entity query from `vocabulary_id` to `vid` (correct entity field name).
- Fixed `SnapshotController::listSnapshots()` from `$snapshot->label` to `$snapshot->name` to match DB column.

Code quality:
- Extracted `validateCsrfToken()` into shared `src/Traits/CsrfValidationTrait.php`, used by all three controllers.
- Extracted `getAncestryPath()` into shared `src/Traits/AncestryPathTrait.php`, used by TreeController and TermController.
- Replaced all `\Drupal::` static calls with constructor dependency injection across TermController, RevisionController, SnapshotController, TreeController, ImportForm, and TaxonautPermissions.
- Applied PHP 8 constructor property promotion to all injectable classes.
- Added `TermStorageInterface` type hint to `isDescendant()` parameter.
- TaxonautPermissions now implements `ContainerInjectionInterface` with injected `EntityTypeManagerInterface`.
- Replaced `strpos()` with `str_starts_with()` in ImportForm.
- Added `autowire: true` and `autoconfigure: true` defaults to `taxonaut.services.yml`.
- JS `attach: function(context)` changed to method shorthand `attach(context)`.
- Added authors section to `composer.json`.
- Added `AccessDeniedHttpException` use statement to TermController.

New files:
- `.cspell.json` with flagWords per drupal.org requirements
- `.gitlab-ci.yml` with standard Drupal Association template includes
- `config/schema/taxonaut.schema.yml`
- `config/install/taxonaut.settings.yml`

**Round 2 fixes (commit 10b1a4d):**
- Added required Configuration section to `README.md`.
- Removed `drupal/core` from `composer.json` require (handled by Drupal.org Composer façade), kept `php: >=8.1` only.
- Changed library version from `VERSION` to literal `1.0.x` (VERSION is only for core modules).
- Made all template strings translatable using `{{ '...'|t }}` filter in `taxonaut-tree.html.twig`.

Will enable GitLab CI on the project settings. Setting back to Needs review.