Issue #2040021 by Wim Leers, tim.plunkett: Fixed In-place editing of nodes...

Issue #2040021 by Wim Leers, tim.plunkett: Fixed In-place editing of nodes does not create new revisions (when the content type is configured to create new revisions by default).

core/modules/edit/edit.module

@@ -175,13 +175,3 @@ function edit_preprocess_field(&$variables) {
 function edit_entity_view_alter(&$build, EntityInterface $entity, EntityDisplay $display) {
   $build['#attributes']['data-edit-entity'] = $entity->entityType() . '/' . $entity->id();
 }
-
-/**
- * Form constructor for the field editing form.
- *
- * @ingroup forms
- */
-function edit_field_form(array $form, array &$form_state, EntityInterface $entity, $field_name, TempStoreFactory $temp_store_factory) {
-  $form_handler = new EditFieldForm();
-  return $form_handler->build($form, $form_state, $entity, $field_name, $temp_store_factory);
-}

core/modules/edit/lib/Drupal/edit/EditController.php

@@ -2,11 +2,12 @@
 
 /**
  * @file
- * Contains of \Drupal\edit\EditController.
+ * Contains \Drupal\edit\EditController.
  */
 
 namespace Drupal\edit;
 
+use Symfony\Component\DependencyInjection\ContainerAware;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -23,12 +24,13 @@
 use Drupal\edit\Ajax\FieldFormValidationErrorsCommand;
 use Drupal\edit\Ajax\EntitySavedCommand;
 use Drupal\edit\Ajax\MetadataCommand;
+use Drupal\edit\Form\EditFieldForm;
 use Drupal\user\TempStoreFactory;
 
 /**
  * Returns responses for Edit module routes.
  */
-class EditController implements ContainerInjectionInterface {
+class EditController extends ContainerAware implements ContainerInjectionInterface {
 
   /**
    * The TempStore factory.
@@ -205,9 +207,10 @@ public function fieldForm(EntityInterface $entity, $field_name, $langcode, $view
     $form_state = array(
       'langcode' => $langcode,
       'no_redirect' => TRUE,
-      'build_info' => array('args' => array($entity, $field_name, $this->tempStoreFactory)),
+      'build_info' => array('args' => array($entity, $field_name)),
     );
-    $form = drupal_build_form('edit_field_form', $form_state);
+    $form_id = _drupal_form_id(EditFieldForm::create($this->container), $form_state);
+    $form = drupal_build_form($form_id, $form_state);
 
     if (!empty($form_state['executed'])) {
       // The form submission saved the entity in tempstore. Return the

core/modules/edit/lib/Drupal/edit/Form/EditFieldForm.php

@@ -7,13 +7,18 @@
 
 namespace Drupal\edit\Form;
 
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\EntityStorageControllerInterface;
+use Drupal\Core\Extension\ModuleHandlerInterface;
+use Drupal\Core\Form\FormInterface;
 use Drupal\user\TempStoreFactory;
 
 /**
  * Builds and process a form for editing a single entity field.
  */
-class EditFieldForm {
+class EditFieldForm implements FormInterface, ContainerInjectionInterface {
 
   /**
    * Stores the tempstore factory.
@@ -23,13 +28,62 @@ class EditFieldForm {
   protected $tempStoreFactory;
 
   /**
+   * The module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
+
+  /**
+   * The node type storage.
+   *
+   * @var \Drupal\Core\Entity\EntityStorageControllerInterface
+   */
+  protected $nodeTypeStorage;
+
+  /**
+   * Constructs a new EditFieldForm.
+   *
+   * @param \Drupal\user\TempStoreFactory $temp_store_factory
+   *   The tempstore factory.
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   *   The module handler.
+   * @param \Drupal\Core\Entity\EntityStorageControllerInterface $node_type_storage
+   *   The node type storage.
+   */
+  public function __construct(TempStoreFactory $temp_store_factory, ModuleHandlerInterface $module_handler, EntityStorageControllerInterface $node_type_storage) {
+    $this->moduleHandler = $module_handler;
+    $this->nodeTypeStorage = $node_type_storage;
+    $this->tempStoreFactory = $temp_store_factory;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('user.tempstore'),
+      $container->get('module_handler'),
+      $container->get('plugin.manager.entity')->getStorageController('node_type')
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFormID() {
+    return 'edit_field_form';
+  }
+
+  /**
+   * {@inheritdoc}
+   *
    * Builds a form for a single entity field.
    */
-  public function build(array $form, array &$form_state, EntityInterface $entity, $field_name, TempStoreFactory $temp_store_factory) {
+  public function buildForm(array $form, array &$form_state, EntityInterface $entity = NULL, $field_name = NULL) {
     if (!isset($form_state['entity'])) {
       $this->init($form_state, $entity, $field_name);
     }
-    $this->tempStoreFactory = $temp_store_factory;
 
     // Add the field form.
     field_attach_form($form_state['entity'], $form, $form_state, $form_state['langcode'], array('field_name' =>  $form_state['field_name']));
@@ -42,10 +96,6 @@ public function build(array $form, array &$form_state, EntityInterface $entity,
       '#attributes' => array('class' => array('edit-form-submit')),
     );
 
-    // Add validation and submission handlers.
-    $form['#validate'][] = array($this, 'validate');
-    $form['#submit'][] = array($this, 'submit');
-
     // Simplify it for optimal in-place use.
     $this->simplify($form, $form_state);
 
@@ -59,7 +109,9 @@ protected function init(array &$form_state, EntityInterface $entity, $field_name
     // @todo Rather than special-casing $node->revision, invoke prepareEdit()
     //   once http://drupal.org/node/1863258 lands.
     if ($entity->entityType() == 'node') {
-      $entity->setNewRevision(in_array('revision', variable_get('node_options_' . $entity->bundle(), array())));
+      $node_type_settings = $this->nodeTypeStorage->load($entity->bundle())->getModuleSettings('node');
+      $options = (isset($node_type_settings['options'])) ? $node_type_settings['options'] : array();
+      $entity->setNewRevision(in_array('revision', $options));
       $entity->log = NULL;
     }
 
@@ -76,23 +128,25 @@ protected function init(array &$form_state, EntityInterface $entity, $field_name
       'bundle' => $entity->bundle(),
       'form_mode' => 'default',
     );
-    \Drupal::moduleHandler()->alter('entity_form_display', $form_display, $form_display_context);
+    $this->moduleHandler->alter('entity_form_display', $form_display, $form_display_context);
 
     $form_state['form_display'] = $form_display;
   }
 
   /**
-   * Validates the form.
+   * {@inheritdoc}
    */
-  public function validate(array $form, array &$form_state) {
+  public function validateForm(array &$form, array &$form_state) {
     $entity = $this->buildEntity($form, $form_state);
     field_attach_form_validate($entity, $form, $form_state, array('field_name' =>  $form_state['field_name']));
   }
 
   /**
+   * {@inheritdoc}
+   *
    * Saves the entity with updated values for the edited field.
    */
-  public function submit(array $form, array &$form_state) {
+  public function submitForm(array &$form, array &$form_state) {
     $form_state['entity'] = $this->buildEntity($form, $form_state);
 
     // Store entity in tempstore with its UUID as tempstore key.

core/modules/edit/lib/Drupal/edit/Tests/EditLoadingTest.php

@@ -58,7 +58,8 @@ function setUp() {
           'value' => '<p>How are you?</p>',
           'format' => 'filtered_html',
         )
-      )
+      ),
+      'log' => $this->randomString(),
     ));
 
     // Create 2 users, the only difference being the ability to use in-place
@@ -112,7 +113,7 @@ function testUserWithoutPermission() {
     $edit['body[0][value]'] = '<p>Malicious content.</p>';
     $edit['body[0][format]'] = 'filtered_html';
     $edit['op'] = t('Save');
-    $response = $this->drupalPost('edit/form/' . 'node/1/body/und/full', 'application/vnd.drupal-ajax', $post);
+    $response = $this->drupalPost('edit/form/' . 'node/1/body/und/full', 'application/vnd.drupal-ajax', $edit);
     // @todo Uncomment the below once https://drupal.org/node/2063303 is fixed.
     // $this->assertIdentical('[]', $response);
     $this->assertResponse(403);
@@ -142,6 +143,11 @@ function testUserWithPermission() {
     $this->assertRaw('data-edit-entity="node/1"');
     $this->assertRaw('data-edit-id="node/1/body/und/full"');
 
+    // There should be only one revision so far.
+    $revisions = node_revision_list(node_load(1));
+    $this->assertIdentical(1, count($revisions), 'The node has only one revision.');
+    $original_log = $revisions[1]->log;
+
     // Retrieving the metadata should result in a 200 JSON response.
     $htmlPageDrupalSettings = $this->drupalSettings;
     $post = array('fields[0]' => 'node/1/body/und/full');
@@ -176,7 +182,7 @@ function testUserWithPermission() {
 
     // Retrieving the form for this field should result in a 200 response,
     // containing only an editFieldForm command.
-    $post = array('nocssjs' => 'true') + $this->getAjaxPageStatePostData();
+    $post = array('nocssjs' => 'true', 'reset' => 'true') + $this->getAjaxPageStatePostData();
     $response = $this->drupalPost('edit/form/' . 'node/1/body/und/full', 'application/vnd.drupal-ajax', $post);
     $this->assertResponse(200);
     $ajax_commands = drupal_json_decode($response);
@@ -191,16 +197,18 @@ function testUserWithPermission() {
     $this->assertTrue($form_tokens_found, 'Form tokens found in output.');
 
     if ($form_tokens_found) {
-      $post = array(
-        'form_id' => 'edit_field_form',
-        'form_token' => $token_match[1],
-        'form_build_id' => $build_id_match[1],
+      $edit = array(
         'body[0][summary]' => '',
         'body[0][value]' => '<p>Fine thanks.</p>',
         'body[0][format]' => 'filtered_html',
         'op' => t('Save'),
       );
-      $post += $this->getAjaxPageStatePostData();
+      $post = array(
+        'form_id' => 'edit_field_form',
+        'form_token' => $token_match[1],
+        'form_build_id' => $build_id_match[1],
+      );
+      $post += $edit + $this->getAjaxPageStatePostData();
 
       // Submit field form and check response. This should store the
       // updated entity in TempStore on the server.
@@ -228,6 +236,58 @@ function testUserWithPermission() {
       // Ensure the text on the original node did change.
       $this->drupalGet('node/1');
       $this->assertText('Fine thanks.');
+
+      // Ensure no new revision was created and the log message is unchanged.
+      $revisions = node_revision_list(node_load(1));
+      $this->assertIdentical(1, count($revisions), 'The node has only one revision.');
+      $this->assertIdentical($original_log, $revisions[1]->log, 'The revision log message is unchanged.');
+
+      // Now configure this node type to create new revisions automatically,
+      // then again retrieve the field form, fill it, submit it (so it ends up
+      // in TempStore) and then save the entity. Now there should be two
+      // revisions.
+      $this->container->get('config.factory')->get('node.type.article')->set('settings.node.options', array('status', 'revision'))->save();
+
+      // Retrieve field form.
+      $post = array('nocssjs' => 'true', 'reset' => 'true');
+      $response = $this->drupalPost('edit/form/' . 'node/1/body/und/full', 'application/vnd.drupal-ajax', $post);
+      $this->assertResponse(200);
+      $ajax_commands = drupal_json_decode($response);
+      $this->assertIdentical(1, count($ajax_commands), 'The field form HTTP request results in one AJAX command.');
+      $this->assertIdentical('editFieldForm', $ajax_commands[0]['command'], 'The first AJAX command is an editFieldForm command.');
+      $this->assertIdentical('<form ', Unicode::substr($ajax_commands[0]['data'], 0, 6), 'The editFieldForm command contains a form.');
+
+      // Submit field form.
+      preg_match('/\sname="form_token" value="([^"]+)"/', $ajax_commands[0]['data'], $token_match);
+      preg_match('/\sname="form_build_id" value="([^"]+)"/', $ajax_commands[0]['data'], $build_id_match);
+      $edit['body[0][value]'] = '<p>kthxbye</p>';
+      $post = array(
+        'form_id' => 'edit_field_form',
+        'form_token' => $token_match[1],
+        'form_build_id' => $build_id_match[1],
+      );
+      $post += $edit + $this->getAjaxPageStatePostData();
+      $response = $this->drupalPost('edit/form/' . 'node/1/body/und/full', 'application/vnd.drupal-ajax', $post);
+      // @todo Uncomment the below once https://drupal.org/node/2063303 is fixed.
+      // $this->assertIdentical('[]', $response);
+      $this->assertResponse(200);
+      $ajax_commands = drupal_json_decode($response);
+      $this->assertIdentical(1, count($ajax_commands), 'The field form HTTP request results in one AJAX command.');
+      $this->assertIdentical('editFieldFormSaved', $ajax_commands[0]['command'], 'The first AJAX command is an editFieldFormSaved command.');
+      $this->assertTrue(strpos($ajax_commands[0]['data'], 'kthxbye'), 'Form value saved and printed back.');
+
+      // Save the entity.
+      $post = array('nocssjs' => 'true');
+      $response = $this->drupalPost('edit/entity/' . 'node/1', 'application/json', $post);
+      // @todo Uncomment the below once https://drupal.org/node/2063303 is fixed.
+      // $this->assertIdentical('[]', $response);
+      $this->assertResponse(200);
+
+      // Test that a revision was created with the correct log message.
+      $revisions = node_revision_list(node_load(1));
+      $this->assertIdentical(2, count($revisions), 'The node has two revisions.');
+      $this->assertIdentical($original_log, $revisions[1]->log, 'The first revision log message is unchanged.');
+      $this->assertIdentical('Updated the <em class="placeholder">Body</em> field through in-place editing.', $revisions[2]->log, 'The second revision log message was correctly generated by Edit module.');
     }
   }
 

core/modules/node/lib/Drupal/node/Tests/NodeFormButtonsTest.php

@@ -108,7 +108,6 @@ function testNodeFormButtons() {
     // Set article content type default to unpublished. This will change the
     // the initial order of buttons and/or status of the node when creating
     // a node.
-    variable_set('node_options_article', array('promote'));
     \Drupal::config('node.type.article')->set('settings.node.options.status', 0)->save();
 
     // Verify the buttons on a node add form for an administrator.