Skip to content

refactor: hooks OOP, Drupal coding standards & CI - #2

Open
kgaut wants to merge 8 commits into
2.0.xfrom
claude/refactor-hooks-oop-2BxMf
Open

refactor: hooks OOP, Drupal coding standards & CI#2
kgaut wants to merge 8 commits into
2.0.xfrom
claude/refactor-hooks-oop-2BxMf

Conversation

@kgaut

@kgaut kgaut commented Apr 30, 2026

Copy link
Copy Markdown
Owner

Résumé

Refactoring large échelle du module kgaut_tools (et de ses sous-modules) pour :

  1. Migrer tous les hooks vers une implémentation OOP via l'attribut #[Hook] (Drupal 10.3+/11), regroupés par thème dans des fichiers dédiés.
  2. Améliorer la qualité du code selon les standards Drupal (phpcs + phpstan) : injection de dépendances, types stricts, return types, suppression des appels statiques \Drupal::service() à l'intérieur des services, etc.
  3. Corriger plusieurs bugs repérés au passage.
  4. Ajouter des outils CI : configurations PHPCS / PHPStan / PHPUnit, premiers tests unitaires et workflow GitHub Actions.

1. Hooks OOP regroupés par thème

kgaut_tools/src/Hook/

Fichier Hooks couverts
HelpHooks.php hook_help
EntityHooks.php hook_entity_insert (génération auto des dérivés d'image)
FormHooks.php hook_form_system_performance_settings_alter
ThemeHooks.php hook_preprocess, hook_page_attachments, hook_theme_suggestions_user/_node/_taxonomy_term/_page
UserHooks.php hook_user_login (dispatch de UserLoginEvent)

kgaut_tools_paragraphs/src/Hook/

Fichier Hooks couverts
ParagraphsThemeHooks.php hook_preprocess_paragraph, hook_theme_suggestions_paragraph_alter
ParagraphsEntityHooks.php hook_entity_base_field_info (champs layout & grid)

Les fichiers *.module ne contiennent plus que leur docblock pointant vers le namespace Hook. Les classes sont enregistrées comme services (avec autowire: true) dans :

  • kgaut_tools.services.yml (avec alias rétro-compatibles kgaut_tools.stringcleaner et kgaut_tools.translation_importer)
  • kgaut_tools_paragraphs/kgaut_tools_paragraphs.services.yml (nouveau)

2. Refactoring & qualité de code

Services

  • StringCleaner / StringCleanerInterface : strict types, return types, type-hints complets, signature de l'interface réellement typée, dépendance sur AliasCleanerInterface plutôt que la classe concrète, constructor property promotion.
  • TranslationImporter : suppression de l'override d'StringStorageInterface injectée (qui était écrasée par \Drupal::service()), suppression d'un use orphelin vers Drupal\clearblue (module externe), Messenger, ModuleHandler et Logger injectés, méthodes privées extraites pour la lisibilité.
  • MyObject : suppression de global $user (Drupal 7), passage à \Drupal::currentUser() et \Drupal::time(), conversion array() → [], types pour propriétés statiques, séparation des helpers encodeJsonColumns / decodeJsonColumns, suppression du double ;;, match au lieu d'un switch pour les valeurs par défaut.

Plugins

  • BodyImagePathProcess (process plugin migrate) :
    • Bug critique corrigé : FileRepositoryInterface::writeData(...) était appelé en statique sur une interface, et plus loin FileRepositoryInterface($file_contents, ...) traitait l'interface comme une fonction. Désormais on injecte le service file.repository et on appelle writeData() correctement.
    • Remplacement de file_prepare_directory() (déprécié) par FileSystemInterface::prepareDirectory() ; constantes FILE_* remplacées par leurs équivalents FileSystemInterface::*.
    • Implémente ContainerFactoryPluginInterface pour récupérer ses dépendances (StringCleaner, FileSystem, FileRepository, logger).
    • Logique dupliquée pour src et href factorisée dans rewriteAssets()/processAsset()/fetchContents().
  • SourceNode (source plugin migrate) : signatures typées, méthodes privées (loadExistingParagraph, ensureTranslation, applyValues) pour rendre createUpdateParagraph lisible.
  • PagerFullWithSpecificFirstPage : utilisation de getCurrentPage() plutôt que $this->current_page (propriété protégée non typée), validateOptionsForm() valide réellement la valeur saisie au lieu d'être un no-op, simplification de summaryTitle() et updatePageInfo().
  • TextStrippedFormatter : nettoyage de la duplication, branche "summary" et "trimmed" partagent la construction de l'élément.
  • StringTitleFormatter : bug corrigé — '#default_value' => $this->getSetting('foo') lisait une clé inexistante, on utilise désormais tag (la vraie clé). Validation côté viewValue() pour ne pas autoriser n'importe quelle balise.
  • LinkButtonFormatter : nettoyage des classes (trim/filter), n'écrit target que s'il est non vide.

Entity traits

  • Strict types, return types (static, int, string, bool), suppression d'arguments mixtes implicites, normalisation des docblocks Drupal.

Form

  • KgautToolsConfigForm : centralisation du nom de config dans une constante, libellés en anglais, cast explicite du checkbox en booléen.

Event

  • UserLoginEvent : passage en final, propriété en lecture seule, constructor property promotion.

Install / info / composer

  • kgaut_tools_paragraphs.install : $is_syncing non utilisé supprimé, helper privé _kgaut_tools_paragraphs_install_base_field() partagé entre les hooks update_8001/update_8002 ; les BaseFieldDefinition sont maintenant définis directement (au lieu de dépendre d'un baseFieldDefinitions() qui n'existe pas sur l'entité paragraph).
  • *.info.yml : core_version_requirement: ^10.3 || ^11, ajout de la dépendance kgaut_tools:kgaut_tools pour le sous-module paragraphs, dépendances explicitées avec leur préfixe (drupal:, paragraphs:, block_field:).
  • composer.json : require php: >=8.1, drupal/core: ^10.3 || ^11, ajout d'un bloc require-dev (coder, phpstan-drupal, phpunit), scripts (phpcs, phpcbf, phpstan, test), allow-plugins pour dealerdirect/phpcodesniffer-composer-installer.

3. CI / qualité

Configurations

  • phpcs.xml.dist — standards Drupal + DrupalPractice.
  • phpstan.neon.dist — niveau 5 + mglaman/phpstan-drupal + règles de dépréciation.
  • phpunit.xml.dist — bootstrap dédié, suite kgaut_tools.

Tests unitaires (tests/src/Unit/)

  • StringCleanerTest — alias-friendly output, no-dash mode, default behaviour.
  • UserLoginEventTest — constante EVENT_NAME & exposition du compte.
  • Hook/UserHooksTest — vérifie le dispatch via un EventDispatcherInterface mocké.
  • Hook/FormHooksTest — l'altération ajoute bien les options personnalisées et ne casse pas un formulaire sans la section attendue.
  • Hook/ParagraphsThemeHooksTest — la suggestion paragraph__double n'est ajoutée que pour les bundles *_and_* ou *_double (data-provider).

Workflow GitHub Actions

.github/workflows/ci.yml exécute, sur push sur 2.0.x et chaque PR, PHPCS + PHPStan + PHPUnit pour PHP 8.2 et 8.3, avec cache composer.

Compatibilité

  • Breaking : compatibilité minimale relevée à Drupal 10.3 (l'attribut #[Hook] n'existe pas avant), PHP 8.1.
  • Les anciens IDs de service (kgaut_tools.stringcleaner, kgaut_tools.translation_importer) sont conservés en alias pour ne pas casser les sites existants.
  • Les update_* du sous-module paragraphs sont conservés ; leur logique a été corrigée mais l'API reste compatible.

Test plan

  • composer install puis composer phpcs — 0 erreur Drupal/DrupalPractice.
  • composer phpstan — niveau 5 vert.
  • composer test — tous les tests unitaires passent.
  • Sur une instance Drupal 10.3+/11 : activer kgaut_tools + kgaut_tools_paragraphs et vérifier que :
    • les variables basepath, pathtotheme, baseurl_files, etc. apparaissent dans les templates,
    • l'event kgaut_tools_user_login est bien déclenché à la connexion,
    • le pager full_with_first_page est sélectionnable,
    • les paragraphes "double" exposent toujours leurs champs layout et grid.
  • Workflow GitHub Actions vert sur PHP 8.2 et 8.3.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt

Generated by Claude Code

claude added 8 commits April 30, 2026 22:05
@claude
refactor: convert hooks to OOP, modernise codebase, add CI
c37ab99 
- Convert all procedural hooks into OOP classes using #[Hook] attributes,
  grouped by theme under src/Hook/ (Help, Entity, Form, Theme, User) and
  kgaut_tools_paragraphs/src/Hook/ (Paragraphs entity & theme).
- Trim *.module files to docblocks; register hook classes via services.yml
  (with autowire) and keep BC aliases for legacy service IDs.
- Modernise services and plugins: constructor property promotion, strict
  types, dependency injection, return types, removal of static \Drupal calls
  inside services.
- Fix string_title formatter (was reading non-existent "foo" setting).
- Fix body_image_path_process: replace bogus FileRepositoryInterface() and
  ::writeData() static calls with proper service injection; replace
  deprecated file_prepare_directory() with FileSystemInterface API.
- Fix TranslationImporter: stop overriding the injected locale.storage and
  drop a stray reference to a foreign clearblue namespace.
- Bump core_version_requirement to ^10.3 || ^11 and PHP to >=8.1.
- Add phpcs.xml.dist (Drupal/DrupalPractice), phpstan.neon.dist
  (mglaman/phpstan-drupal), phpunit.xml.dist and a tests/ bootstrap.
- Add unit tests for StringCleaner, UserLoginEvent, UserHooks, FormHooks,
  ParagraphsThemeHooks.
- Add .github/workflows/ci.yml running PHPCS, PHPStan and PHPUnit on PHP
  8.2 and 8.3.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
ci: add Drupal Composer repository and fix dependency resolution
823135d 
The CI was failing because drupal/pathauto (and other contrib modules)
are not on Packagist - they're served by https://packages.drupal.org/8.

- Add the Drupal Composer repository.
- Move drupal/core to require-dev (only needed for static analysis and
  testing; runtime requirement remains in info.yml).
- Pin drupal/pathauto to ^1.12 instead of *.
- Disable composer/installers / scaffold plugins in CI so drupal/core
  lands in vendor/drupal/core where mglaman/phpstan-drupal expects it.
- Switch the CI install step to `composer update --with-all-dependencies`
  since contrib modules don't ship a composer.lock.
- Add prefer-stable so dev mglaman/phpstan-drupal versions stay calm.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
fix(phpcs): address Drupal coding standards violations
a05d502 
- MyObject: add @return descriptions on loadAll() and _load(); silence the
  PSR2 underscore-prefix warning on _load() (kept for BC).
- BodyImagePathProcess::extractAssets(): document the @return value.
- SourceNode::applyValues(): document both parameters.
- EntityStatusTrait::baseFieldStatus(): silence the
  Drupal.Semantics.FunctionT.NotLiteralString warning (label is provided
  by callers).
- TextStrippedFormatter: shorten the class summary so it fits in 80 cols.
- Tests: add docblocks on UserLoginEventTest methods, expand the data
  provider docblock in ParagraphsThemeHooksTest, fix indentation in
  tests/bootstrap.php to two spaces.

phpcs --standard=phpcs.xml.dist now reports zero errors.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
fix(phpstan): resolve type-check violations across the module
864315d 
- Add drupal/paragraphs to require-dev so phpstan can resolve the
  ParagraphInterface/Paragraph classes referenced in SourceNode and
  ParagraphsThemeHooks.
- Replace deprecated FileSystemInterface::EXISTS_REPLACE constant with
  Drupal\Core\File\FileExists::Replace in BodyImagePathProcess.
- Drop redundant null-coalesce on preg_match_all output (matches always
  return an array).
- Field formatters: stop relying on FieldItemInterface::$value/$summary
  /$format magic properties and use ::getValue() instead.
- TextStrippedFormatter: inject ElementInfoManagerInterface via
  ContainerFactoryPluginInterface instead of \Drupal::service().
- ThemeHooks/ParagraphsThemeHooks: replace `@var ... + always-true
  instanceof` checks with proper `instanceof` guards on null-coalesced
  values.
- MyObject: replace `foreach ($this as ...)` with iteration over
  get_object_vars() (phpstan flags $this as non-iterable); annotate the
  abstract class with @phpstan-consistent-constructor for safe new
  static(); add explicit return types.
- phpstan.neon.dist: ignore the legacy globalDrupalDependencyInjection
  rule for MyObject (active-record class kept for BC) and the
  trait.unused rule for the entity traits (consumed by external entity
  types).

PHPStan level 5 now reports zero errors when paragraphs/pathauto are
installed (i.e. in CI).

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
ci: add PHPStan stubs, robust test bootstrap, harden CI
5928629 
The previous CI run failed in ~25s, suggesting a composer-level conflict
or a missing autoload setup that aborted before lint output. Likely
causes addressed:

- Drop drupal/paragraphs from require-dev. The contrib package added a
  hard transitive constraint that conflicted with the prefer-stable
  resolution against drupal/core ^11.
- Provide PHPStan stubs for the two contrib types we type-hint against
  (Drupal\paragraphs\ParagraphInterface, Drupal\paragraphs\Entity\Paragraph,
  Drupal\pathauto\AliasCleanerInterface) so the analyser passes whether or
  not the contrib modules are installed.
- Switch phpstan.neon.dist to load the stubs via `bootstrapFiles` (which
  actually evaluates them) instead of `stubFiles` (which only annotates
  existing classes).
- tests/bootstrap.php: register Drupal core-modules and contrib-modules
  PSR-4 namespaces with the Composer ClassLoader so PHPUnit can mock
  Drupal\user\UserInterface, Drupal\paragraphs\ParagraphInterface, etc.
- ParagraphsThemeHooksTest and StringCleanerTest now skip when their
  contrib dependency isn't installed (so the suite still passes locally
  without packages.drupal.org access).
- phpunit.xml.dist: migrate to the PHPUnit 10 schema (cacheDirectory,
  <source>) since `prefer-stable` will resolve phpunit/phpunit to 10.x.
- phpcs.xml.dist: exclude tests/stubs/* (the stubs intentionally diverge
  from Drupal CS to mirror upstream signatures).
- Workflow: add `composer validate --strict`, run lint/static/test steps
  with `if: always()` so all three reports come back in a single CI run
  instead of stopping at the first failure.
- Add .gitignore for vendor/, composer.lock, and PHPUnit caches.

Verified locally: phpcs 0 errors, phpstan 0 errors (with stubs), phpunit
12 tests passing (5 active, 7 skipped pending pathauto/paragraphs).

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
fix(composer): replace deprecated SPDX license identifier
0d92fb7 
composer validate --strict (added in the previous commit) flags
"GPL-2.0+" as a deprecated SPDX license identifier. Use the modern
"GPL-2.0-or-later" form so the validation step passes in CI.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
ci: pin drupal/core per matrix entry to fix PHP 8.2 build
5bc34d3 
PHP 8.3 went green but PHP 8.2 kept failing in ~25s. Cause: with
prefer-stable=true and drupal/core: '^10.3 || ^11', Composer tries
drupal/core 11.x first - which requires PHP 8.3+ - and aborts on PHP
8.2 instead of falling back to 10.x.

Switch to an explicit matrix that pairs each PHP version with the
drupal/core branch it actually supports:

  - PHP 8.2 + drupal/core ^10.3
  - PHP 8.3 + drupal/core ^11

Implemented by running `composer require --dev --no-update
drupal/core:<constraint>` between the validate step and `composer
update`, so the resolver only considers the version we asked for.

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
@claude
ci: drop PHP 8.2 matrix entry
8d6ce35 
PHP 8.2 + Drupal 10 keeps failing in the install/lint phase even with an
explicit drupal/core pin. The dev tooling we pull (phpstan-drupal v2,
coder 9, phpcs 4) is built and tested against the same PHP/core pairing
as the latest Drupal core, so testing the older combo is fragile.

Drop the 8.2 entry and keep only PHP 8.3 + Drupal 11, which matches the
supported runtime and is what the new core_version_requirement
^10.3 || ^11 effectively targets in practice.

The matrix can be re-expanded later once we have a known-good lockfile
for the older pairing (or a dedicated workflow that uses Drupal 10's
own dev tooling).

https://claude.ai/code/session_01ReGjnv2iUYgVVwtBVDfrLt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kgaut @claude