Page MenuHomePhabricator

(backend/api) Can not add reference to existing statement when any statement's reference got added/edited in the meantime
Closed, ResolvedPublic

Description

Two users load the same page, user A will finish loading before user B, user B will add a reference to any statement, the reference gets saved. Now user A will send his change of an existing reference or will try to add a new reference and sends the change to the API, but this will fail with the message "Failed to save the change". Only a reload will allow user A to do his edit.

User A can still add a new Statement and then add a new reference for that statement (since for creating that reference, the base rev id received after creation of the new statement was done is being used).


Version: unspecified
Severity: major

Details

Reference
bz44547

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 1:25 AM
bzimport set Reference to bz44547.
bzimport added a subscriber: Unknown Object (MLST).

Actually, user B might have a problem as well after adding/editing a Statement's reference.
After that change, all other Statement's references can not be edited and no new ones can be added for those.

Tried the following example:

  • I have an item with 2 claims: claim A and claim B which both have a reference
  • I change the reference of claim A
  • I try to change the reference of claim B (without reloading the page): this fails

What happens:

  • the request fails, because EditEntity::fixEditConflict() returns false in line 481.
  • of course the problem lies deeper, so I tracked it down a bit more
  • in line 468 of EditEntity::fixEditConflict() $patchedCurrent seems to contain wrong data: Claim B still has the old reference although it should have been patched with the Diff between the baseContent and the newContent (which still is correct - line 458).
  • consequently this leads to the case where the currentContent and the patched currentContent are the same and diffing them results in an empty diff which then leads to the patch collapsing.
  • going even further, in line 855 of Entity::patch() both parameters (array of claims of current content & the claimsDiff) seem to be correct, so I assume the problem might be somewhere deeper in the MapPatcher of the Diff extension

It runs into line 130 in MapPatcher::getPatchedMap(): "Cannot do a non-add operation with an element not present in a map."

Restricted Application added a subscriber: StudiesWorld. · View Herald Transcript