Tracking issue for linting, coding-standards, CI, and test coverage across 2.x and 3.x. Bug-fix issues targeting 2.x should stay scoped to the fix plus test coverage – see the discussion on #3541709, where a fix MR bundled in phpcs/cspell fixes and Drupal 10 CI-lane tuning alongside the actual bug fix, which made the real change harder to review. Drupal 10 is nearing EOL and 3.x is already the deliberate major-refactor branch, so linting/coding-standards churn belongs there, not on 2.x.
3.x linting and CI (done, MR !16)
- Added
.gitlab-ci.yml– the project previously had none, so no pipeline ran on any merge request. Scoped for 3.x only: no Drupal 10 lane, so none of the PHPUnit 9.6/attribute-compatibility handling or phpstan version-skipping that the Drupal 10 lane needs on 2.x applies here. - Fixed phpcs violations from the GitLab template's drupal-contrib-project ruleset (indentation and brace style left over from the Rector-generated
SaveEditHooksclass, missing full stops on doc comments, comment lines over 80 characters). - Fixed cspell violations and declared maintainer/contributor names and project-specific terms in
_CSPELL_WORDS. - Converted
SaveEditHooks's\Drupal::static calls to constructor-injected services (ConfigFactoryInterface,AccountProxyInterface), per the DrupalPractice dependency-injection sniff.
2.x test coverage (in progress, MR !17)
This module had no automated test coverage at all. Tests land on 2.x first – it's the confirmed-working, widely-deployed branch, so a test against it establishes ground truth for known-good behavior – then get ported forward to 3.x once confirmed passing, which doubles as a real regression check on the Rector-generated hooks-in-classes conversion. .gitlab-ci.yml was added to 2.x as well so these tests actually run in CI, with phpcs and phpstan explicitly disabled there (deferred to 3.x per the linting split above, so 2.x's pre-existing style debt doesn't turn every pipeline red for unrelated reasons).
Coverage so far, across three kernel test classes:
- SaveEditFormAlterTest – the button's presence/absence gating (permission and per-content-type config), its properties (text, weight, name, no stray
#button_type), submit-handler inheritance without leaking onto the default Save button, Gin primary-action styling, and the default-Save-button text override and its suppression underhide_default_save/hide_default_preview/hide_default_delete. - SaveEditSettingsFormTest – the settings form builds without error on a never-saved install.
- SaveEditSettingsFormAccessTest – the settings route requires the module's own permission, not general admin access.
Regression tests for specific historical bugs from the closed-issue queue, folded into the above:
- #3381925 (redirect handler ordering vs. Content Moderation) – covered by asserting the redirect handler is always last in the submit chain.
- #3381269 (Save and Save & Edit buttons sharing one
#attributesobject) – covered directly. - #3110054 and #3085773 (both:
node_typesunset entirely on a fresh install breakingin_array()/array_values()) – covered directly. - #2981648 (settings form checkbox default-value error on first use) – covered directly.
- #2940629 (settings route had the wrong permission requirement) – covered directly.
Flagged but not covered: #3026200 (Inline Entity Form submit-handler ordering). The real bug was an interaction with a third-party contrib module; a synthetic stand-in wouldn't actually prove the IEF scenario, so this is left as a known gap rather than a false sense of coverage.
Next: confirm MR !17's pipeline passes on 2.x, then port the tests forward to 3.x (adjusting for the dropped Drupal 10/pre-11.2 NodeForm handling and the class-based hooks refactor where the two branches genuinely differ).
Issue fork save_edit-3619151
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:
Comments
Comment #2
caesius commentedComment #3
caesius commentedComment #5
caesius commentedComment #6
caesius commentedComment #7
caesius commentedComment #9
caesius commentedComment #12
caesius commented