🇺🇸United States @rymcveigh

I tried again today using Address 2.0.1 as the base version of the module.

This workflow worked for me:

• Upgrade from version 1.12.0 to 2.0.1 of the address module: composer require 'drupal/address:^2.0' --with-all-dependencies
• run drush updatedb
• Watch address_update_9201 fail with error: Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.
• Apply the patch from this MR
• Rerun drush updatedb and watch it pass

This workflow failed the first time I tried to do an updatedb but passed the second time I ran it:

• Upgrade from version 1.12.0 to 2.0.1 of the address module: composer require 'drupal/address:^2.0' --with-all-dependencies
• Apply the patch from this MR
• Run drush updatedb and watch it fail with error: Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.
• Rerun drush updatedb and watch it pass.

That made me wonder what would happen if I ran drush updatedb twice using version 2.0.1 of the module without the patch. This did not work. Here is what I got:

• Upgrade from version 1.12.0 to 2.0.1 of the address module: composer require 'drupal/address:^2.0' --with-all-dependencies
• Run drush updatedb and watch it fail with error: Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.
• Rerun drush updatedb and watch it fail with error: Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.

Patch #3 works for me as well.

I tested on:

drupal/paragraphs_browser: 1.1.0
drupal/core: 10.2.3

It would be soooo nice to get this merged in.

I can confirm that the patch from #18 works for me as well.

I tested on:

Drupal 10.2.3
paragraphs_browser 1.1.0

I completely agree that there needs to be a new release of this version.

I ran into this same issue and had to dig for a while to find this solution. Could we update the documentation in either the readme or the module description so that folks have an easier time defining the third-party settings on custom baseFieldDefinitions?

Unfortunately, the patch (in its current state) does not work for my custom entity. Here's what I tried:

I tried adjusting the field definition for my entity prior to running a updb with these changes:

$fields['location'] = BaseFieldDefinition::create('address')
      ->setLabel('Location')
      ->setRevisionable(TRUE)
      ->setCardinality(1)
      ->setDefaultValue([
        'county_code' => 'US',
      ])
      ->setRequired(TRUE)
      ->setSetting('available_countries', ['US'])
      ->setSetting('field_overrides', [
        AddressField::GIVEN_NAME => ['override' => FieldOverride::HIDDEN],
        AddressField::FAMILY_NAME => ['override' => FieldOverride::HIDDEN],
        AddressField::ADDITIONAL_NAME => ['override' => FieldOverride::HIDDEN],
        AddressField::ORGANIZATION => ['override' => FieldOverride::HIDDEN],
        AddressField::ADDRESS_LINE1 => ['override' => FieldOverride::HIDDEN],
        AddressField::ADDRESS_LINE2 => ['override' => FieldOverride::HIDDEN],
        AddressField::ADDRESS_LINE3 => ['override' => FieldOverride::HIDDEN],
        AddressField::LOCALITY => ['override' => FieldOverride::OPTIONAL],
        AddressField::ADMINISTRATIVE_AREA => ['override' => FieldOverride::OPTIONAL],
        AddressField::POSTAL_CODE => ['override' => FieldOverride::HIDDEN],
      ])
      ->setDisplayOptions('form', [
        'type' => 'address_default',
        'weight' => 4,
      ])
      ->setDisplayConfigurable('view', TRUE)
      ->setDisplayConfigurable('form', TRUE);

That resulting in this error: [error] Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.

I then imported a fresh DB and tried running the updb without altering my field definition and got the same error: [error] Cannot add field 'maf_memorial_field_revision.location__address_line3': table doesn't exist.

I am going to try to trigger a field definition update before the address update and see if that helps. If it doesn't help, I will look at the changes in the MR today and see if I can figure out why this may be happening. Thanks for helping everyone.

We have encountered the same issue on a custom entity, a custom block_type and with a commerce order. The initial update hook (9201) will not successfully run because it "Cannot add field 'some_field_name.field_address_address_line3': field already exists."

The MR/Patch does not work for us because the first update hook keeps failing. I agree with @bojanz that "we should try to debug the original update function and fix the problem there before we introduce update functions with workarounds".

Okay, I removed 8.0 from the changes and returned it to needs review.

I'm putting this into needs work because we should remove the ^8 from the ^8 || ^9 || ^10. I can adjust the MR.

rymcveigh → changed the visibility of the branch 3415248-autosave-not-triggering to active.

rymcveigh → changed the visibility of the branch 3415248-autosave-not-triggering to hidden.

These changes look good to me, though I wonder if we should still support drupal version 8.x?

@greggles, nice catch. I wasn't sure if that needed to be addressed because using an unnamed function in a .once is one of the alternative methods defined on this page https://www.drupal.org/node/3158256 → .

I addressed it using a different recommended method that names the function.

I think this could be like the webform draft purge configuration, where the admin defines a set number of days to purge drafts or completed webforms from the system. It does this when cron is run on the system. Would that make sense, or do you have other thoughts?

The byproduct of doing this will be that if a user visits a form whose logs have been removed, they will not see the pages that have errors or pages they have seen in the past in the navigation bar.

rymcveigh → made their first commit to this issue’s fork.

rymcveigh → created an issue.

I reviewed these changes, and they look good to me. Thanks @pixelwhip!

The PHPStan, eslint, and test fails are not related to the changes made in this MR.

Thank you for documenting this. It helped us resolve the same issue with a custom module.

I recommend getting this patch in. This fix works for us as well.

I created a branch for these changes. You will see a link to it above.

rymcveigh → made their first commit to this issue’s fork.

I haven't run into this issue, nor have I heard of anyone else running into it. Can you share your webform config with me to see if I can reproduce it on a vanilla install? Also, it'd be good to know what version of Drupal you are running and what version of PHP is installed on your server.

The webform config can be found at /admin/structure/webform/manage/webform_machine_name/export or admin->structure->webform->export. You should be able to copy the YML and share it here without sharing any sensitive information.

You can see the versions of PHP and Drupal you are using by going to /admin/reports/status or admin->reports->status report.

I reviewed these changes, and they look good to me. The failing unit tests are the same as the failing unit tests on the current release of this module. Do we need to add unit tests for inline errors as part of these adjustments? Or are there tests already in place?

Is there any way to turn that off until they've made at least one selection in the form? Right now it seems to be saving a draft before they've even interacted with anything.

Unfortunately we need to have a draft to save to before a selection is made. We explored this in issues #3145769 and #3103887

rymcveigh → created an issue.

To get the listener to work on the select created by the image picker jQuery library, we needed to add a change event listener to selects. I added the listener to the attached merge request. I've attached the config yaml for the test webform I created when working on this issue.

This looks to be a bug. I created a test webform on a vanilla Drupal 10 website. The autosave functionality worked except for the image select element. I suspect it is because the JS listener is not triggering when the image is selected. I altered the category of this issue to be a bug.

Just noting that it looks like this issue (Pathauto Condition UI is not D10 ready) 💬 Pathauto Condition UI is not D10 ready Fixed was/is related to the changes needed for this release.

This seems to be a regression that was introduced in this merge request. I wonder if we should focus our efforts on figuring out what caused the regression in those changes?

I've created a new MR with the other MR's changes and discovered that users can bypass validation if they navigate to the preview page (on the Wizard example form provided by the webform_examples module) and press submit.

Here's the config for the test form:

uuid: 7b7c5e9d-8759-43a6-be0f-167dcba96aa9
langcode: en
status: open
dependencies:
  module:
    - webformnavigation
  enforced:
    module:
      - webform_examples
third_party_settings:
  webformnavigation:
    forward_navigation: true
    prevent_next_validation: 0
    additional_error_message: ''
_core:
  default_config_hash: pL2poJ5SN2KH-toyxJFZUhL1LfJU_jIxHdaH2nXvMng
weight: 0
open: null
close: null
uid: null
template: false
archive: false
id: example_wizard
title: 'Example: Wizard'
description: "<p>Example of a multiple step 'wizard' webform.</p>"
categories:
  - Example
elements: |
  '#attributes':
    data-current-page: '[webform_submission:current-page]'
  information:
    '#title': 'Your Information'
    '#type': webform_wizard_page
    '#open': true
    first_name:
      '#title': 'First Name'
      '#type': textfield
      '#required': true
    last_name:
      '#title': 'Last Name'
      '#type': textfield
      '#required': true
    gender:
      '#type': webform_radios_other
      '#title': Gender
      '#options': gender
      '#required': true
  contact:
    '#title': 'Contact Information'
    '#type': webform_wizard_page
    '#open': true
    email:
      '#title': Email
      '#type': email
      '#required': true
    phone:
      '#title': Phone
      '#type': tel
      '#required': true
    contact_via_phone:
      '#type': radios
      '#title': 'Can we contact you via phone?'
      '#options': yes_no
  feedback:
    '#title': 'Your Feedback'
    '#type': webform_wizard_page
    '#open': true
    comments:
      '#type': textarea
css: ''
javascript: ''
settings:
  ajax: true
  ajax_scroll_top: form
  ajax_progress_type: fullscreen
  ajax_effect: ''
  ajax_speed: null
  page: true
  page_submit_path: ''
  page_confirm_path: ''
  page_theme_name: ''
  form_title: source_entity_webform
  form_submit_once: false
  form_open_message: ''
  form_close_message: ''
  form_exception_message: ''
  form_previous_submissions: true
  form_confidential: false
  form_confidential_message: ''
  form_disable_remote_addr: false
  form_convert_anonymous: false
  form_prepopulate: false
  form_prepopulate_source_entity: false
  form_prepopulate_source_entity_required: false
  form_prepopulate_source_entity_type: ''
  form_unsaved: true
  form_disable_back: false
  form_submit_back: false
  form_disable_autocomplete: false
  form_novalidate: false
  form_disable_inline_errors: false
  form_required: false
  form_autofocus: false
  form_details_toggle: false
  form_reset: false
  form_access_denied: default
  form_access_denied_title: ''
  form_access_denied_message: ''
  form_access_denied_attributes: {  }
  form_file_limit: ''
  form_attributes: {  }
  form_method: ''
  form_action: ''
  share: false
  share_node: false
  share_theme_name: ''
  share_title: true
  share_page_body_attributes: {  }
  submission_label: ''
  submission_exception_message: ''
  submission_locked_message: ''
  submission_log: false
  submission_excluded_elements: {  }
  submission_exclude_empty: false
  submission_exclude_empty_checkbox: false
  submission_views: {  }
  submission_views_replace: {  }
  submission_user_columns: {  }
  submission_user_duplicate: false
  submission_access_denied: default
  submission_access_denied_title: ''
  submission_access_denied_message: ''
  submission_access_denied_attributes: {  }
  previous_submission_message: ''
  previous_submissions_message: ''
  autofill: false
  autofill_message: ''
  autofill_excluded_elements: {  }
  wizard_progress_bar: true
  wizard_progress_pages: false
  wizard_progress_percentage: false
  wizard_progress_link: true
  wizard_progress_states: false
  wizard_start_label: ''
  wizard_preview_link: true
  wizard_confirmation: true
  wizard_confirmation_label: ''
  wizard_auto_forward: true
  wizard_auto_forward_hide_next_button: false
  wizard_keyboard: true
  wizard_track: ''
  wizard_prev_button_label: ''
  wizard_next_button_label: ''
  wizard_toggle: false
  wizard_toggle_show_label: ''
  wizard_toggle_hide_label: ''
  wizard_page_type: container
  wizard_page_title_tag: h2
  preview: 2
  preview_label: ''
  preview_title: ''
  preview_message: ''
  preview_attributes: {  }
  preview_excluded_elements: {  }
  preview_exclude_empty: true
  preview_exclude_empty_checkbox: false
  draft: all
  draft_multiple: false
  draft_auto_save: true
  draft_saved_message: ''
  draft_loaded_message: ''
  draft_pending_single_message: ''
  draft_pending_multiple_message: ''
  confirmation_type: page
  confirmation_url: ''
  confirmation_title: ''
  confirmation_message: ''
  confirmation_attributes: {  }
  confirmation_back: true
  confirmation_back_label: ''
  confirmation_back_attributes: {  }
  confirmation_exclude_query: false
  confirmation_exclude_token: false
  confirmation_update: false
  limit_total: null
  limit_total_interval: null
  limit_total_message: ''
  limit_total_unique: false
  limit_user: null
  limit_user_interval: null
  limit_user_message: ''
  limit_user_unique: false
  entity_limit_total: null
  entity_limit_total_interval: null
  entity_limit_user: null
  entity_limit_user_interval: null
  purge: draft
  purge_days: 365
  results_disabled: false
  results_disabled_ignore: false
  results_customize: false
  token_view: false
  token_update: false
  token_delete: false
  serial_disabled: false
access:
  create:
    roles:
      - anonymous
      - authenticated
    users: {  }
    permissions: {  }
  view_any:
    roles: {  }
    users: {  }
    permissions: {  }
  update_any:
    roles: {  }
    users: {  }
    permissions: {  }
  delete_any:
    roles: {  }
    users: {  }
    permissions: {  }
  purge_any:
    roles: {  }
    users: {  }
    permissions: {  }
  view_own:
    roles: {  }
    users: {  }
    permissions: {  }
  update_own:
    roles: {  }
    users: {  }
    permissions: {  }
  delete_own:
    roles: {  }
    users: {  }
    permissions: {  }
  administer:
    roles: {  }
    users: {  }
    permissions: {  }
  test:
    roles: {  }
    users: {  }
    permissions: {  }
  configuration:
    roles: {  }
    users: {  }
    permissions: {  }
handlers:
  webform_navigation:
    id: webform_navigation
    handler_id: webform_navigation
    label: 'Webform Navigation'
    notes: ''
    status: true
    conditions: {  }
    weight: 0
    settings:
      debug: false
variants: {  }

We should wait till Webform 6.2 is out of beta before adding this adjustment.

One error I got when running PHPCS is Use null coalesce operator instead of ternary operator. I'm not sure if we want to use a ternary or not but I adjusted the code so that PHPCS is happy.

I created an MR that applies the patch and I ran PHP code sniffer and PHPStan to ensure the codebase is ready for Drupal10.

rymcveigh → made their first commit to this issue’s fork.

rymcveigh → created an issue.

It would be really nice if we could get this (now merged) change released to the most recent stable version of the module.

rymcveigh → created an issue.

This issue fork worked for me as well.

I'm marking this as a duplicate because it is resolved in this module's 8.x-4.x branch. It checks to see if the invokeAllWith method exists and then calls it. But, to support Drupal 8, they must leave the getImplementations function in place.

if (method_exists($this->moduleHandler, 'invokeAllWith')) {
      $this->moduleHandler->invokeAllWith('mail', function (callable $hook, string $module) use (&$list) {
        $list[$module] = $this->moduleHandler->getName($module);
      });
    }
    else {
      foreach ($this->moduleHandler->getImplementations('mail') as $module) {
        $list[$module] = $this->moduleHandler->getName($module);
      }
    }

rymcveigh → made their first commit to this issue’s fork.

This change worked for me. I think it is GTG.

This patch works for us as well. It looks GTG to me.

I'm curious if anyone has attempted to use MYSQL partitioning on the webform_submission_data table to handle this issue. If you have, I would appreciate hearing about your experience with Drupal's handling of partitioning and the resulting performance improvement.

Thanks for the review @lkacenja!

Currently, the module overrides the wizard_track webform setting and disregards the step URL parameter. This is a great suggestion, though. I changed this issue to a feature request so folks can work on it.

I want to make sure I'm doing this accurately. To reproduce this issue I:

1. Install the module on a vanilla Drupal site
2. Enable autosave on the base contact webform
3. Set the autosave trigger to 5000 milliseconds
4. Fill out the contact form but stop typing as the Ajax call is happening
5. Then autosave triggers again after another 5 seconds (as I'd expect)

Autosave triggers twice for me. There's the initial save (when I stop typing) then there's a second save. If I submit the form before the second save the form is saved with the correct values as well.

Can you please help me reproduce the issue on a vanilla site? What are the steps I can take to reproduce this?

I added some feedback to the merge request. One major thing I see with these changes is a Drupal\Core\Entity\EntityStorageException when I try to save a webform programmatically (see comment on the MR for an example). I believe that is related to the changes in the handler's postSave function.

I was able to resolve this issue on our site by enabling the stable9 theme (which is a hidden theme in the latest version of core) via drush drush theme:enable stable9. I wonder if there is a way to add the stable9 theme as a dependency that will force the stable9 theme to enable when a user upgrades to the latest stable version of the Bulma theme?

It looks like the stable9 base theme dependency was added in issue #3162911 ✨ Use stable9 as base theme Fixed

I wonder why the stable9 base theme was not added as a composer dependency in this work. It isn't a core theme in Drupal 9.5.9 and I'm trying to figure out how to install this new dependency.

This is related to issue #3361624 🐛 The base theme stable9 is not enabled when updating the theme on Drupal 9.x Active

I see the same issue. There appears to be a dependency on the stable9 base theme, that is a contributed theme. But, it is not defined in the composer file for this theme. So, when you install the theme using composer, you will start to see the Base theme stable9 has not been installed. (Drupal\Core\Theme\MissingThemeDependencyException) exception in watchdog.

I am marking this issue as "Needs Work" because there are merge conflicts in the merge request.

I tested these changes locally and they look great. I used the following to test:

• Ran PHPCS
• Created a wizard webform on a vanilla Drupal 10 install
• Ran a series of Behat tests with the changes on an existing site using these changes.

These changes look good to me.

I have been able to install this module on a vanilla Drupal 10 site and the primary automated tests for the module run on Drupal 10. Based on the output from composer you shared it looks like the version of the webform_submission_log you are using is locked to a specific version. That module is supplied by the webform module. What version of webform do you have installed? Are you using the latest version?

This MR seems to help but I am getting a new error when I attempt to upgrade from rc3 to the 3320565-upgrade-from-rc3 branch. The error I'm seeing is Exception: Webform submission Ajax form should never be cancelled. Only ::reset should be called. in Drupal\webform\WebformSubmissionForm->cancelAjaxForm() (line 3164 of /var/www/html/docroot/modules/contrib/webform/src/WebformSubmissionForm.php).

Here are the steps I took:

1. Create a fresh site using Drupal version 9.5.7 with MySQL 8
2. Install version rc3 of the webform navigation module
3. Import the webform yml posted in comment #12
4. Create some submissions of that webform
5. replace the webformnavigation module with the 3320565-upgrade-from-rc3 branch
6. clear cache and run a drush updb
7. edit an existing submission (the endpoint will return a 500 and you'll see the error in watchdog when you try to submit the form)
8. Try to create a new submission
9. Get a 500 when I try to submit the form on the final page and see the error in watchdog.

It looks like the MR attached to this issue has been RTBTC. Is there a plan to merge this into the codebase sometime soon?

I created an MR for this issue and was able to move the required logic into the preprocess much like how it's done in the form_element_label.preprocess.inc file.

I'm going to create an MR for this issue and update the codebase to use the current USWDS classes for fieldsets.

rymcveigh → made their first commit to this issue’s fork.

I also updated the project page for this module to reflect the Project page template → .

I adjusted tests for Drupal 10 in commit eab24853

I updated the README in commit 22fa75c4.

I adjusted the doMail function to be protected in commit 10ca0226.

I wonder if we should adjust the readme and module description to meet Drupal standards as part of this work?

I created a merge request with some security-based adjustments.

I moved your changes into an MR and based it off of the 8.x-2.x branch so it should be mergeable. I'm confused why we need the extra catch with the log. It looks like most (if not all) of the exceptions are logged in the original ::mail() function. Is there an exception that isn't covered? If so, would it be better to adjust the original function rather than add a new function to the code base?

I'm marking this as needing work and creating a new merge request because the current patch can not be applied to the 8.x-2.x branch.

rymcveigh → made their first commit to this issue’s fork.

I created an MR that is built on the 8.x-2.x branch. It looks like some of the changes in your patch had been addressed in a prior merge so the number of changes needed has decreased since the creation of that patch.

The patch seems to be missing a couple $stats_data uses. I see it being used on lines 233, 234, 239, 295, and 347. Your patch seems to be missing the if empty check on line 234. It seems like we will want to make sure the function returns an empty array if $statsdata is empty.

The code changes look good to me. I wonder if there is a good way to reproduce the issue though. Are there any tests in the code base that triggers this error?

These changes seem great. I just had one comment on the MR. Are you okay if we use protected TimeInterface $time; for consistency throughout the module's codebase?

Webforms should autosave for anonymous users. The questionable bit would be the optimistic locking. I'm pretty sure that will fail because it is reliant on UIDs. As far as an Email handler, I'm not sure you'll want to get an email every time an input changes (when the autosave is triggered) but you could trigger an email anytime a draft is created. That means that you would get emails for every submission. You would just need to create a new handler that sends the email even if the submission has not been completed.

rymcveigh → created an issue.

rymcveigh → created an issue. See original summary → .

I am willing to make a small bet this regression is being triggered by #3335861: [Drupal 9.2.x] Replace jquery.removeOnce with new once API.

It actually happens on the current 6.2@beta release of webform as well as the 6.2.x branch. So I do not think they are related.

I am able to reproduce this issue using Drupal 10 with PHP 8.1 using the composer installed 6.2@beta version on the module (that doesn't have the removeOnce adjustment). But, the removeOnce function doesn't work on D10 anyway so... I gues it could be a regression... 🤷

In any case, the current version of dev works okay on Drupal 9.5. This bug seems to be related to a change in core.