Problem/Motivation

The existing tests cover the settings form, config schema, update 8002 and a page load smoke test. The behavior users rely on has little or no coverage: forward navigation through the progress bar, errors shown again when a page is revisited, the progress bar classes, prevent_next_validation, and the error and page visit logging. Without these, changes such as #3632737 are hard to review, and sites that depend on the markup (for example the has-errors and is-complete classes) can break without notice.

Proposed resolution

Add tests in this order, each as its own MR or alongside the issue that changes that area.

Functional

  • Forward navigation: with forward_navigation on, every page link in the progress bar can be used, moving ahead skips validation, and returning to a page shows its logged errors (WebformNavigationHandler::alterForm(), submitForm()).
  • Progress bar and tracker classes: has-errors, is-complete and the current page class from webformnavigation_preprocess_webform_progress_tracker() and webformnavigation_preprocess_webform_progress_bar().
  • prevent_next_validation: Next with an empty required field moves on when enabled, and stays on the page when disabled.
  • Final submit: started in #3632737 (ValidateAllPagesTest).

Kernel

  • WebformNavigationHelper:
    • logPageVisit() does not log a page twice.
    • getErrors() returns the errors for one page or all pages.
    • logPageErrors() maps nested element errors (parent][child) to the element's page.
    • Errors for a submission that is not saved yet are stashed, then logged by postSave() through logStashedPageErrors().
  • webformnavigation_webform_presave(): enabling forward navigation sets draft, purge, purge_days and wizard_progress_link. Record the current behavior before it changes.
  • Log cleanup: deleting a submission removes its webformnavigation_log rows (postDelete()).

Remaining tasks

  • Write the tests above.
  • Decide what to do with the additional_error_message setting. It is saved and has a schema, but nothing reads it, so it never appears in the final submit error. Either use it (with a test) or deprecate it, in a separate issue.

User interface changes

None.

API changes

None.

Data model changes

None.

Command icon 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

rymcveigh created an issue. See original summary.

rymcveigh’s picture

Issue summary: View changes

rymcveigh’s picture

Status: Active » Needs review

This issue is ready for review.

shubhamgoel’s picture

Hi @rymcveigh
I read the whole MR diff. The MR adds tests and CI config, plus a typo fix in two templates, and I found no blocking problems.

Suggestions:
- Core version mismatch: the CI comment says Drupal ^10.3 || ^11.0, but webformnavigation.info.yml declares ^9 || ^10 || ^11. The test code uses PHP 8 features (a heredoc closing marker with indentation, static data providers), and Webform 6.x requires Drupal 10 or later. Consider narrowing core_version_requirement in a follow-up. The MR doesn't touch it.
- Package constraint: composer.json allows drupal/webform ^5.16 || ^6.0. The CI comment only says ^10.3 || ^11.0, so you should confirm that 5.x is really tested, or drop it.

rymcveigh’s picture

Status: Needs review » Reviewed & tested by the community
rymcveigh’s picture

Thanks for the review @shubhamgoel.

Agreed on both points: the declared support was wider than what is tested. CI installs the current Webform, 6.3.x, which requires `^10.3 || ^11.0`, so the tests run on Drupal 10 and 11. We never tested Webform 5.x or Drupal 9.

A small correction on the PHP point: the tests don't need PHP 8. The heredoc closing marker isn't indented, and the trailing comma after it and static data providers work on PHP 7.3+. The real reason to drop Drupal 9 is that it has been end-of-life since November 2023, and the current Webform no longer supports it.

Rather than a follow-up, I've made the change in this MR (and the same in webformautosave !32):

  • `core_version_requirement: ^10.3 || ^11`
  • `composer.json`: `drupal/core: ^10.3 || ^11` and `drupal/webform: ^6.2`

This will go out as a new minor release, with the dropped support in the release notes. Composer keeps Drupal 9 sites on the last release that supports them.

rymcveigh’s picture

Version: 2.0.x-dev » 2.1.x-dev

  • rymcveigh committed aa00bee7 on 2.1.x
    task: #3632767 Add test coverage for forward navigation, progress bar...
rymcveigh’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

shubhamgoel’s picture

Hi @rymcveigh,

Can you please review the credit for this issue?

rymcveigh’s picture

Sorry @shubhamgoel,

I thought you had been listed, but I didn't realize I needed to check your name. It should be resolved now.