Updated: Comment #0

Problem/Motivation

Discovered via #2075185: When an entity is in-place edited (i.e. saved), other instances of that entity on the same page are not updated (no propagation).

When the same entity appears more than once on the same page, only the first entity gets the "Quick edit" contextual link. Consequently, only the first entity can be in-place edited.

Steps to reproduce:

  1. Create an article node.
  2. Expose the frontpage as a block in the sidebar.
  3. Go to the frontpage. The same node now appears twice: once in the main content area, once in the sidebar. You'll see that only the one in the main content area gets a "Quick edit" contextual link.

Proposed resolution

Fix this.

Remaining tasks

User interface changes

None.

API changes

None.

Files: 

Comments

Wim Leers’s picture

Status: Active » Needs review
FileSize
15.77 KB
PASSED: [[SimpleTest]]: [MySQL] 59,097 pass(es). View

This patch solves the problem through the following changes:

  • Each instance of an entity now has an entityInstanceID: an incrementing number starting at 0.
  • Each Drupal.edit.EntityModel now has a entityID and a entityInstanceID attribute in addition to the pre-existing id attribute. The entityID attribute now contains what the id attribute contained before (something like node/1). The entityInstanceID speaks for itself. The id attribute now provides a "globally unique" identifier for the entity: <entityID>:[<entityInstanceID>], e.g. node/1[0].
  • Similarly, each Drupal.edit.FieldModel now has a fieldID attribute that contains what the id attribute contained before (something like node/1/body/en/full). The id attribute now provides a "globally unique" identifier for the field: <fieldID>:[<entityInstanceID>], e.g. node/1/body/en/full[0].
  • Where appropriate, existing code has been updated accordingly to e.g. find all fields for a specific instance of an entity instead of just all fields for a "logical entity".
Wim Leers’s picture

Can be reviewed, but should NOT be committed until #2139921: Contextual links can't handle multiple occurrences of the same contextual links is committed.

Wim Leers’s picture

jessebeach’s picture

Status: Needs review » Needs work

I really like the changes of id to fieldID and entityID; it makes the code easier to read. We can always remap the ID attribute of the models later if ever we go full REST: http://backbonejs.org/#Model-idAttribute

+++ b/core/modules/edit/js/views/EditorView.js
@@ -172,7 +172,7 @@ Drupal.edit.EditorView = Backbone.View.extend({
-    var backstageId = 'edit_backstage-' + this.fieldModel.id.replace(/[\/\_\s]/g, '-');
+    var backstageId = 'edit_backstage-' + this.fieldModel.id.replace(/[\/\[\]\_\s]/g, '-');

For consistency, this should be this.fieldModel.get('fieldID').replace()

That's my only needs-change feedback. Otherwise it looks great and functions.

Wim Leers’s picture

Status: Needs work » Needs review

Actually, that was intentional. It ensures that the DOM ID attributes that are generated are unique across the entire page: they're entity instance-specific.

i.e.:

  • fieldID would be "node/1/title/en/teaser"
  • id would be "node/1/title/en/teaser[0]" (note the entity instance identifier at the end
  • Due to the entity instance ID at the end, the regexp also had to be updated to replace [ and ] with a dash.
jessebeach’s picture

Status: Needs review » Reviewed & tested by the community

Ok, that was my only concern. Thanks for the explanation.

I've gone through further manual testing and there are no behavioral regressions.

It solves the issue of being able to launch quick edit on instances of the same entity on the same page.

chx’s picture

This is the kind of attention to detail that makes the Spark team outstanding. Very nice.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Wim Leers’s picture

Issue tags: -sprint
Wim Leers’s picture

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

Status: Fixed » Closed (fixed)

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

sittard’s picture

This bug or similar appears to be present in Drupal 8.1.8.

Steps to reproduce:

  1. Create a number of Basic page nodes for testing
  2. Create a view as a block with the format content rendered as a teaser. Set the pager to display all items
  3. Place the views block in the sidebar region with visibility set so that it appears on a Basic page node
  4. Go to any Basic page node. It is not possible to access the quick edit menu for the node being viewed or the same node in the sidebar region.

Attached are some screen shots to help highlight the issue.

sittard’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Closed (fixed) » Active
sittard’s picture

Version: 8.1.x-dev » 8.3.x-dev

Just ran a test on 8.3.x-dev and the quick edit worked as expected on the first page load. But failed on subsequent page loads, cleared browser cache and flushed full cache but contextual links still don't appear.

Also noticed in this post (https://www.drupal.org/node/2650910) discussion around non-cache and cached-cases. Perhaps it's related.

  • Dries committed 1069bef on 8.4.x
    Issue #2141055 by Wim Leers: When multiple instances of the same entity...

  • Dries committed 1069bef on 8.4.x
    Issue #2141055 by Wim Leers: When multiple instances of the same entity...

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jamaral86’s picture

Issue tags: +FLDC17

Im going to triage it

droplet’s picture

Issue tags: +Needs JS testing

That's great if anyone can write a test to prove it still a problem.