Updated: Comment #3

(Split off from #1989974: [meta] Clean up/improve consistency of Edit because it deserves its own issue. There, it just said "Make sure Edit works at all when a same entity exists multiple times on the same page (possibly in a different view mode). Also: all instances of the same entity on the same page should be updated when one of them is modified.".

Problem/Motivation

A Drupal site contains one article node: node 1. You go to the full node page for this node and on this page, you see the node 1 in full view mode, but there's also a block on the side containing node 1 in teaser view mode, as well as another block with node 1 in full view mode.

(So, the same node, 3 instances, two of which have the same view mode, and consequently the exact same HTML representation.)

When you edit either, the others don't update.

Proposed resolution

Just make it work.

The tricky case is when the view mode is different: then you need to go to the server to retrieve the entire rerendered node, because it might be rendered entirely differently now.

See the explanation and patch in #3.

Remaining tasks

None.

User interface changes

None.

API changes

None.

#1979784: Factor Create.js and VIE.js out of the Edit module
#1989974: [meta] Clean up/improve consistency of Edit

Comments

wim leers’s picture

The fundaments for this are already present. I also already had a basic (and for some reason inverse) patch for this since a long time. Attached, because it's a good starting point. It explains the challenges.

wim leers’s picture

Issue tags: -sprint

We're not fixing this in the current sprint.

wim leers’s picture

Title: When an entity is in-place edited (i.e. saved), other instances of that entity on the same page are not updated » When an entity is in-place edited (i.e. saved), other instances of that entity on the same page are not updated (no propagation)
Assigned: Unassigned » wim leers
Issue summary: View changes
Status: Active » Postponed
Issue tags: +JavaScript, +sprint
StatusFileSize
new20.81 KB

While working on this, I encountered two blockers:

  1. #2139921: Contextual links can't handle multiple occurrences of the same contextual links in contextual.modul
  2. #2141055: When multiple instances of the same entity on one page, only the first can be edited in edit.module, which makes this patch a whole lot simpler

This patch solves the problem through the following changes:

  1. When starting to edit a field, all other instances of the same "logical field" are checked to see if they have a different view mode. If they do have a different view mode, they are remembered as one of pieces of data that should be POSTed when changes are made to that field.
  2. EditController::fieldForm() is slightly refactored: the rerendering of a field now happens in a protected renderField() helper method and instead of just rendering the updated field instance's view mode, it also renders the other view modes that were received.
  3. FieldFormSavedCommand was updated to not only return the rerendered HTML of the updated field instance, but also the rerendered HTML for the other view modes that were received.
  4. Each Drupal.edit.FieldModel now has a logicalFieldID attribute that is identical to the fieldID attribute, minus the view mode. So, for example, fieldID = node/1/body/en/full, then logicalFieldID = node/1/body/en. The difference should be clear: this allows us to find all instances of the same field, no matter in which view mode they're rendered on the page.
  5. Whenever a field is rerendered (because changes were POSTed), Drupal.edit.FieldModel.attributes.html is changed, and a new propagateUpdatedField() method listens for that. When that method is called, it finds all other instances on the page of the same "logical field" (cfr. logicalFieldID) and rerenders them, too. If other instances use the same view mode, then it's easy: just use the same HTML as the one used to rerender the edited field instance. In the other case, step 1 guarantees that the HTML for other view modes is also available.
  6. The test coverage was updated to test for both the case of "no other view modes" and "other view modes". The latter is tested in combination with the custom render pipeline functionality, to ensure that even complex cases work fine, such as "the same entity exists twice on the page: once rendered by Views, once rendered by Entity/Field API".





Marking as postponed because this is blocked on #2139921 and #2141055!

wim leers’s picture

Issue summary: View changes
wim leers’s picture

Status: Postponed » Needs review
Issue tags: -sprint
StatusFileSize
new20.81 KB
new1.51 KB

Now unblocked!

Attached is a straight reroll to apply cleanly against HEAD, and a .zip file containing two YML files to set up a block View for testing this feature. Just import these YML files and make sure you have at least one node, then on the front page, edit the node body or title, and BOOM, the other one updates as well! :)

wim leers’s picture

Issue tags: +sprint

The last submitted patch, 5: edit_change_propagation-2075185-5.patch, failed testing.

jessebeach’s picture

jessebeach’s picture

Status: Needs review » Needs work

I'm getting an error when editing the body of an article node on the full view, with the imported views in the sidebar.

Uncaught AjaxError: 
An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /edit/form/node/1/body/en/full
StatusText: OK
ResponseText:  

The AJAX return object status is 'parsererror'.

PHP stack trace:

[11-Dec-2013 18:44:36 UTC] PHP Fatal error:  Using $this when not in object context in /Users/jbeach/code/drupal/core/d8/core/modules/edit/lib/Drupal/edit/EditController.php on line 239
[11-Dec-2013 18:44:36 UTC] PHP Stack trace:
[11-Dec-2013 18:44:36 UTC] PHP   1. {main}() /Users/jbeach/code/drupal/core/d8/index.php:0
[11-Dec-2013 18:44:36 UTC] PHP   2. drupal_handle_request($test_only = *uninitialized*) /Users/jbeach/code/drupal/core/d8/index.php:15
[11-Dec-2013 18:44:36 UTC] PHP   3. Drupal\Core\DrupalKernel->handle($request = *uninitialized*, $type = *uninitialized*, $catch = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/includes/bootstrap.inc:1886
[11-Dec-2013 18:44:36 UTC] PHP   4. Drupal\Core\HttpKernel->handle($request = *uninitialized*, $type = *uninitialized*, $catch = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/DrupalKernel.php:282
[11-Dec-2013 18:44:36 UTC] PHP   5. Symfony\Component\HttpKernel\HttpKernel->handle($request = *uninitialized*, $type = *uninitialized*, $catch = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/HttpKernel.php:52
[11-Dec-2013 18:44:36 UTC] PHP   6. Symfony\Component\HttpKernel\HttpKernel->handleRaw($request = *uninitialized*, $type = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:61
[11-Dec-2013 18:44:36 UTC] PHP   7. call_user_func_array(*uninitialized*, *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:117
[11-Dec-2013 18:44:36 UTC] PHP   8. Drupal\Core\Controller\AjaxController->content($request = *uninitialized*, $_content = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:117
[11-Dec-2013 18:44:36 UTC] PHP   9. Drupal\Core\HttpKernel->forward($controller = *uninitialized*, $attributes = *uninitialized*, $query = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/Controller/AjaxController.php:55
[11-Dec-2013 18:44:36 UTC] PHP  10. Drupal\Core\HttpKernel->handle($request = *uninitialized*, $type = *uninitialized*, $catch = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/HttpKernel.php:81
[11-Dec-2013 18:44:36 UTC] PHP  11. Symfony\Component\HttpKernel\HttpKernel->handle($request = *uninitialized*, $type = *uninitialized*, $catch = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/HttpKernel.php:52
[11-Dec-2013 18:44:36 UTC] PHP  12. Symfony\Component\HttpKernel\HttpKernel->handleRaw($request = *uninitialized*, $type = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:61
[11-Dec-2013 18:44:36 UTC] PHP  13. call_user_func_array(*uninitialized*, *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:117
[11-Dec-2013 18:44:36 UTC] PHP  14. Drupal\edit\EditController->fieldForm($entity = *uninitialized*, $field_name = *uninitialized*, $langcode = *uninitialized*, $view_mode_id = *uninitialized*, $request = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php:117
[11-Dec-2013 18:44:36 UTC] PHP  15. Drupal\edit\{closure}($view_mode_id = *uninitialized*) /Users/jbeach/code/drupal/core/d8/core/modules/edit/lib/Drupal/edit/EditController.php:243

The last submitted patch, 5: edit_change_propagation-2075185-5.patch, failed testing.

wim leers’s picture

Status: Needs work » Needs review
Related issues: +#2152073: Bump Drupal core's PHP requirement to 5.4.2
StatusFileSize
new21.47 KB
new2.28 KB

Root cause determined. I'm working on PHP 5.4. Drupal 8 will soon require PHP 5.4: #2152073: Bump Drupal core's PHP requirement to 5.4.2. But, alas, for now we're stuck on 5.3. The problems you're encountering are also the reason testbot failed (testbot's still on 5.3).

To fix this, I had to introduce a cludgy work-around. Added two @todo's to make sure it's removed in #2152073. In that issue, the interdiff for this comment/reroll should be inversed to restore the clean PHP 5.4 code.

jessebeach’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new92.37 KB

So it appears that for some Views output, the content will not update, because the necessary markup is not present, specifically the data-edit-field-id attribute. So, in the case that this attribute exists on the field, the content updates on save, as show in this image:

To address the code unraveling, I opened the following issue and linked it to #2152073 (Bump Drupal core's PHP requirement to 5.4.2).

#2156793: Unravel a PHP 5.4 workaround for Edit module once Drupal requires PHP 5.4

So, this issue resolves the multiple node instance update problem without regressions.

jessebeach’s picture

webchick’s picture

  1. +++ b/core/modules/edit/js/models/FieldModel.js
    @@ -61,6 +70,9 @@ Drupal.edit.FieldModel = Backbone.Model.extend({
    +
    +    //
    +    this.set('logicalFieldID', this.get('fieldID').split('/').slice(0, 4).join('/'))
       },
    

    Unfinished thought? :)

  2. +++ b/core/modules/edit/js/models/FieldModel.js
    @@ -112,6 +124,49 @@ Drupal.edit.FieldModel = Backbone.Model.extend({
    +    // Second, figure out if there are .
    

    Another one? :)

Not moving down from RTBC but I don't have mental capacity to review this just yet so if it happens to get fixed in the next day or so then yay. :)

wim leers’s picture

StatusFileSize
new22.44 KB
new2.32 KB

#12:

  1. That's rendered by the Views render pipeline, not by the Entity/Field API render pipeline. They're not supported. You must choose "Content", not "Fields" when configuring Views to show something. That will cause it to use the Entity API render pipeline.
    The demo views I attached in #5 take care of that for you :)
    Supporting Views' own render pipeline is something for D8 contrib: https://drupal.org/project/edit_views.
  2. Thank you for already putting that into its own issue!

#14: Thanks, fixed! I also fixed two JSHint errors (two missing semi-colons). I tested manually to ensure everything still works.
If you test this manually before committing: please follow the testing instructions in #5, they'll make testing a breeze :)

dries’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Small but important clean-up. Committed to 8.x.

wim leers’s picture

Issue tags: -sprint

Now also backported to the Drupal 7 contrib module: http://drupalcode.org/project/edit.git/commit/b199397bde82a15cec6c494ef0....

Status: Fixed » Closed (fixed)

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