Rewrite views_ui_add_ajax_trigger() to not rely on /system/ajax.

I guess we need to replace the entire form with the ajax callback
OR
start with no default and force people to always select an entity type first?

Raising to critical per #2502785-10: Remove support for $form_state->setCached() for GET requests.

One possible approach would be to prefix the entire form, or rather put it into a wizard plugin specific form structure, like 'wizard__' . 'node'
The additional step then would require to rewrite a bunch of other stuff ... or actually quite a lot.

This interdiff just shows what this could like ...

+1 for the idea.

So yeah what we kind of discussed has been to change the UI into a two step process, similar to the field UI.

+++ b/core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php
@@ -518,6 +518,13 @@ public static function getSelected(FormStateInterface $form_state, $parents, $de
+        // If the user-submitted value has changed, remove all other options in
+        // that set of values.

I think we should explore this option first before moving to two step from. If we'll be able to fix it like this then we don't have to deal with UX issues. Thoughts??

I think we should explore this option first before moving to two step from.

I totally agree ...

1. +++ b/core/lib/Drupal/Core/Form/FormBuilder.php
   @@ -236,10 +236,13 @@ public function buildForm($form_id, FormStateInterface &$form_state) {
   +      // - input: Any user submitted data is expected to survive within the same
   +      //   page request.
          if ($check_cache) {
            $cache_form_state = $form_state->getCacheableArray();
            $cache_form_state['always_process'] = $form_state->getAlwaysProcess();
            $cache_form_state['temporary'] = $form_state->getTemporary();
   +        $cache_form_state['input'] = $form_state->getUserInput();
   

   I don't know much about fapi, but this particular change feels really dangerous

2. +++ b/core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php
   @@ -480,16 +480,16 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -  public static function getSelected(FormStateInterface $form_state, $parents, $default_value, $element) {
   +  public static function getSelected(FormStateInterface $form_state, array $parents, $default_value, array $element) {
   

   +1 for moving the additional logic into this method. This indeed is the natural place for it

Nice!
Tim do you mind explaining what is going on in the first hunk, see comment #15

Re #18, that first hunk is trying to make it so that the form builder function (e.g., ViewAddForm::form() via its call to WizardPluginBase::getSelected()) can modify $form_state['input']. Which is potentially problematic if the form is cached, because in such a situation, the form builder wouldn't run. Even if we remove the case of caching on the initial GET request, we should consider what would happen if some other module made the form cacheable after a validation error (e.g., Mollom does so that if you answered the captcha correctly, but got a validation error for some other field, you don't need to re-answer the captcha again). In such a case, when you submit after correcting your validation problems, the form is retrieved from cache, so the form builder function is skipped, so its changes to $form_state['input'] don't happen, which means $form_state['input'] is different as an unintended side-effect of enabling Mollom (or any other module that turns on form caching after a validation error).

Does this work?

1. +++ b/core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php
   @@ -504,7 +504,7 @@ public static function getSelected(FormStateInterface $form_state, $parents, $de
   -    $user_input = &$form_state->getUserInput();
   +    $user_input = $form_state->isRebuilding() ? $form_state->getUserInput() : [];
   

   On the first AJAX submission, when the form is uncached, this lets the form get built the first time the same way it did for the GET request.

2. +++ b/core/modules/views_ui/admin.inc
   @@ -119,7 +119,7 @@ function views_ui_add_ajax_trigger(&$wrapping_element, $trigger_key, $refresh_pa
   -    'duplicate_wrapper' => !empty($seen_ids[$triggering_element['#ajax']['wrapper']]),
   +    //'duplicate_wrapper' => !empty($seen_ids[$triggering_element['#ajax']['wrapper']]),
   

   Without this, the first AJAX submission results in views_ui_ajax_update_form() returning $form['displays'] but without that element having a #prefix with the wrapper ID. I think because that first submission builds the form twice, so the 2nd build looks to be a wrapper duplication but isn't. Then, AJAX submissions 2-N end up doing a replaceWith() with the correct content returned from the server but don't have a wrapper element in the DOM to do it on, so the front-end doesn't get updated to match what's on the back-end.

1. +++ b/core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php
   @@ -496,15 +496,18 @@ public static function getSelected(FormStateInterface $form_state, $parents, $de
   -    $user_input = &$form_state->getUserInput();
   ...
   +    $user_input = $form_state->isRebuilding() ? $form_state->getUserInput() : [];
   

   I'm a bit confused that we no longer need a reference, but I see, we just get values, so there is no need for a reference.

2. +++ b/core/modules/views_ui/admin.inc
   @@ -117,9 +113,6 @@ function views_ui_add_ajax_trigger(&$wrapping_element, $trigger_key, $refresh_pa
   -    // Keep track of duplicate wrappers so we don't add the same wrapper to the
   -    // page more than once.
   -    'duplicate_wrapper' => !empty($seen_ids[$triggering_element['#ajax']['wrapper']]),
   

   Yeah I don't see an issue with doing it.

3. +++ b/core/modules/views_ui/admin.inc
   @@ -166,29 +159,25 @@ function views_ui_add_limited_validation($element, FormStateInterface $form_stat
   +  // This was earlier stored in a property on the element.
   

   Does that documentation add any value on top of version control, I don't really think so

+++ b/core/modules/views/tests/src/Unit/WizardPluginBaseTest.php
@@ -0,0 +1,112 @@
+    $data['invalid_type'] = [
...
+    $data['no_options'] = [
...
+    // A valid form element with no user input.
+    $data['no_user_input'] = [

Awesome!

RTBC + 1, Great work! - Fantastic simple solution, can we document that approach somewhere, please?

Committed 40004e3 and pushed to 8.0.x. Thanks!

@Fabianx I talk about documenting the approach in IRC with @tim.plunkett. He said

i could, but i doubt other non-merlinofchaos implementations had FAPI workarounds of that magnitude...
we already say "you probably shouldn't use getUserInput()"

I'm inclined to agree.