Follow-up from #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests.

Updated: Comment #0

Problem/Motivation

I have been trying to use the 'new' drupalPost() method for an AJAX request, to find out that I need to update the current content based on its results - drupalPostAjaxForm() will do it, but I do not want to use it.

Proposed resolution

Make the code in drupalPostAjaxForm() that processes the AJAX response into the current content, as a protected method of its own.
This way we'd have a core method to update the current content after a drupalPost() call with AJAX header.
Otherwise each extending class will have to find its own solution in that case.

Remaining tasks

User interface changes

None.

API changes

No changes, just one method added.

#2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake’s picture

Assigned: mondrake » Unassigned
Status: Active » Needs review
FileSize
10.81 KB

Patch

Status: Needs review » Needs work

The last submitted patch, 2087637-drupal_process_ajax_response-1.patch, failed testing.

mondrake’s picture

Status: Needs work » Needs review
FileSize
1.93 KB
10.84 KB

#1 wouldn't have worked anyway :)

effulgentsia’s picture

In general, this seems like a good decoupling, but can you give more detail on your use case for wanting to process an AJAX response outside of drupalPostAjaxForm()? Is there somewhere in core that would need to do that, or is this strictly for contrib's benefit?

mondrake’s picture

I was trying to refactor the ViewUI Preview tests developed for testing pager navigation within the Views UI preview form in #2048309: Views UI Preview - navigation is broken, see comment #2.

There, when we click on link, ajax.js sends a POST request that is not (fully) processed via the form controller, as there is no form submission and hence no 'triggering element' from a form. ajax.js also sends an additional 'js' => 'true' post key which I used in the ViewPreviewFormController to detect that the preview needs to be refreshed, in place of checking for the unavailable triggering element. (see discussion in #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests, comments #5 to #13).

However, the response is used by ajax.js to refresh the page content.

For the test I used the (now) drupalPostAjaxForm method, however this brings up a lot of form related info and I could not mimic the actual ajax.js behavior. So I turned to the 'new' drupalPost, which works fine - but falls short of updating the current content.

So options

  1. we leave thing as they are
  2. we let extending classes do their content update if needed
  3. we have this in place and classes that need to update content can invoke the new method
  4. other?

As an example, specifically for this View Preview test, this would be before and after this patch

Before

 206   /**
 207    * Mimic clicking on a preview link.
 208    *
 209    * @param string $url
 210    *   The url to navigate to.
 211    * @param int $row_count
 212    *   The expected number of rows in the preview.
 213    */
 214   protected function clickPreviewLinkAJAX($url, $row_count) {
 215     $ajax_settings = array(
 216       'url' => $url,
 217       'wrapper' => 'views-preview-wrapper',
 218       'method' => 'replaceWith',
 219     );
 220     $result = $this->drupalPostAjaxForm(NULL, array(), NULL, NULL, array(), array(), NULL, $ajax_settings);
 221     $this->assertPreviewAJAX($result, $row_count);
 222   }

After

  /**
   * Mimic clicking on a preview link.
   *
   * @param string $url
   *   The url to navigate to.
   * @param int $row_count
   *   The expected number of rows in the preview.
   */
  protected function clickPreviewLinkAJAX($url, $row_count) {
    $content = $this->content;
    $ajax_settings = array(
      'wrapper' => 'views-preview-wrapper',
      'method' => 'replaceWith',
    );
    $url = $this->getAbsoluteUrl($url);
    $post = array('js' => 'true') + $this->getAjaxPageStatePostData();
    $result = drupal_json_decode($this->drupalPost($url, 'application/vnd.drupal-ajax', $post));
    if (!empty($result)) {
      $this->drupalProcessAjaxResponse($content, $result, $ajax_settings);
    }
    $this->assertPreviewAJAX($result, $row_count);
  }

The 'after' though is really aligned to the actual 'post' keys that ajax.js uses for processing a link click (not a form button).

Status: Needs review » Needs work

The last submitted patch, 2087637-drupal_process_ajax_response-3.patch, failed testing.

mondrake’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2087637-drupal_process_ajax_response-3.patch, failed testing.

mondrake’s picture

Status: Needs work » Needs review
FileSize
1.39 KB
10.17 KB

$drupal_settings has to be pre-saved like $content

effulgentsia’s picture

Status: Needs review » Needs work

Thanks for the use case in #5; totally makes sense.

  1. +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
    @@ -1590,95 +1589,123 @@ protected function drupalPostAjaxForm($path, $edit, $triggering_element, $ajax_p
    +   * JSON data, and an array of commands, to update $this->content using
    

    The JSON data *is* the array of commands, so remove the word "and".

  2. +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
    @@ -1590,95 +1589,123 @@ protected function drupalPostAjaxForm($path, $edit, $triggering_element, $ajax_p
    +   *   An array of Ajax settings which will be used to process the response.
    

    Looks like we're still capitalizing AJAX elsewhere in this docblock, so let's do that here too, for consistency.

  3. +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
    @@ -1590,95 +1589,123 @@ protected function drupalPostAjaxForm($path, $edit, $triggering_element, $ajax_p
    +   *   (optional) An array of settings to update the value of drupalSettings
    

    I don't think $drupal_settings should be optional. There's always a non-empty drupalSettings on the original page, so calling this without passing that will result in incorrect test emulation.

mondrake’s picture

Status: Needs work » Needs review
FileSize
1.99 KB
10.15 KB

Thank you @effulgentsia. Here we go.

Shall I also add the refactoring of Drupal\views_ui\Tests\PreviewTest::clickPreviewLinkAJAX() as per #5 here, or leave that for a separate issue?

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

Great. Thanks. I think the Views test refactor can be a separate issue.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

mondrake’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
10.16 KB
mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC

alexpott’s picture

Title: Add a drupalProcessAjaxResponse() method » Change notice: Add a drupalProcessAjaxResponse() method
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed cfc1584 and pushed to 8.x. Thanks!

Lets create a change notice to let people know.

mondrake’s picture

There's no usage of this method yet, apart from the obvious split in drupalPostAjaxForm. #2100323: Improve views UI preview tests will introduce an example which I suggest to refer to in the change record.

mondrake’s picture

Status: Active » Needs review


Proposal for review

Added a drupalProcessAjaxResponse() method to WebTestBase

Summary:

  • In Drupal7, the single drupalPostAJAX() method manages both sending an AJAX request, and updating the current HTML content upon return.
  • In Drupal 8, we now have the option to post AJAX requests that aren't processed by a form controller, through a new drupalPost() method. This method however is NOT updating the current HTML content.
  • A new drupalProcessAjaxResponse() method has been introduced to process AJAX response in the current content. This method is used by drupalPostAjaxForm() itself, but can also be used when it is necessary to update page content after a non-form initiated AJAX request.

Code example - from the views preview UI test:

  /**
   * Mimic clicking on a preview link.
   *
   * @param string $url
   *   The url to navigate to.
   * @param int $row_count
   *   The expected number of rows in the preview.
   */
  protected function clickPreviewLinkAJAX($url, $row_count) {
    $content = $this->content;
    $drupal_settings = $this->drupalSettings;
    $ajax_settings = array(
      'wrapper' => 'views-preview-wrapper',
      'method' => 'replaceWith',
    );
    $url = $this->getAbsoluteUrl($url);
    $post = array('js' => 'true') + $this->getAjaxPageStatePostData();
    $result = drupal_json_decode($this->drupalPost($url, 'application/vnd.drupal-ajax', $post));
    if (!empty($result)) {
      $this->drupalProcessAjaxResponse($content, $result, $ajax_settings, $drupal_settings);
    }
    $this->assertPreviewAJAX($result, $row_count);
  }

Note: content and drupalSettings need to be stored away before invoking drupalPost(), and then passed back to drupalProcessAjaxResponse(). This because drupalPost() replaces current content completely with the content returned by the AJAX controller, which is a sequence of AJAX commands (not HTML).

xjm’s picture

Issue summary: View changes
Issue tags: +Missing change record, +API addition

Tagging "Missing change record", as completing the change record updates here blocks a beta: #2083415: [META] Write up all outstanding change notices before release

The patch for this issue was committed on September 28, 2013, meaning that the change record for this issue has been outstanding for more than four months. The proposed draft looks pretty good to me; I'd just make it clearer in the bullet points at the top that we're talking about simpletests. E.g.:

In Drupal7, the single WebTestBase::drupalPostAJAX() method manages both sending an AJAX request during an automated test, and updating the current HTML content upon return.

With that change, I'd say go ahead and post the proposed change record, and restore this issue's status/priority/title/tags/etc. :)

mondrake’s picture

Title: Change notice: Add a drupalProcessAjaxResponse() method » Add a drupalProcessAjaxResponse() method
Priority: Major » Normal
Status: Needs review » Fixed
Issue tags: -Needs change record, -Missing change record

Added change record: https://drupal.org/node/2185955

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.