[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-04-04 Thread gerritbot 
gerritbot added a comment.
Change 423867 abandoned by Addshore:
WIP SCRATCH: Investigate grammatical feature undo failings

https://gerrit.wikimedia.org/r/423867TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Addshore, gerritbotCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-04-04 Thread gerritbot 
gerritbot added a comment.
Change 423867 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/WikibaseLexeme@master] WIP SCRATCH: Investigate grammatical feature undo failings

https://gerrit.wikimedia.org/r/423867TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Addshore, gerritbotCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-23 Thread gerritbot 
gerritbot added a comment.
Change 419450 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Add __clone and copy to FormSet

https://gerrit.wikimedia.org/r/419450TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Addshore, gerritbotCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Versusxo, Majesticalreaper22, Tamgue, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-14 Thread Addshore 
Addshore added a comment.

In T189580#4050690, @WMDE-leszek wrote:
Do you want to add a test "showcasing" the bug @Addshore, so we could have it fixed (with the patch you provided I assume), and shall we call the investigation done then, or do we call it done now?


I think we can leave the investigation now (and mark it as done) and when we come to tackle the actual task, write the test etc and actually merge the fix.TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: AddshoreCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-14 Thread WMDE-leszek 
WMDE-leszek added a comment.
I can confirm that using the patch linked above, it seems that having cloning of FormSet added, undoing and restoring works fine.
The explanation provided above also seems legit to me.

Do you want to add a test "showcasing" the bug @Addshore, so we could have it fixed (with the patch you provided I assume), and shall we call the investigation done then, or do we call it done now?TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Addshore, WMDE-leszekCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-14 Thread Addshore 
Addshore added a comment.
It may also be worth doing a general audit of all other Lexeme DM objects to make sure no other clones that are needed are missing so we don't run into the same issue in other places / with other types of edit.TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: AddshoreCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Lordiis, Cinemantique, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-14 Thread gerritbot 
gerritbot added a comment.
Change 419450 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/WikibaseLexeme@master] WIP DNM Add __clone and copy to FormSet

https://gerrit.wikimedia.org/r/419450TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Addshore, gerritbotCc: gerritbot, Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Lahi, Gq86, Cinemantique, GoranSMilovanovic, QZanden, LawExplorer, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T189580: Investigate: Undoing and restore edit on a form does not work

2018-03-14 Thread Addshore 
Addshore added a comment.
So debugging has lead me to SubmitEntityAction::undo in wikibase, and the following code snippet.

		if ( $patchedContent->equals( $latestContent ) ) {
			$status = Status::newGood();
			$status->warning( 'wikibase-empty-undo' );
		} else {
/// ...
}

$patchedContent->equals( $latestContent ) returns true, so it appears that the undo action will do nothing hence nothing is done.
The 2 content objects here do indeed have the same content inside (both with the OLD data)

$latestRevision correctly has the latest revision ID and text.
$latestRevision->mSlots->slots->main->content{LexemeContent} has the correct blob of JSON, but the Lexeme entity within the content is incorrect showing the previous revision (not the latest)

F15407819: image.png

I suspected this was due to LexemeHandler and the special handling for Forms in LexemeHandler::newEntityContent, as an EntityHolder can contain a specific revision of an Entity, and the special case for Forms will load an entity from an EntityLookup which will not have a specified version / revision. But it doesn't look like this special case code for forms it actually called during the undo submit.

It seems that the DeferredDecodingEntityHolder has one blob, the entity object it has does not match.
The trace of the first moment this occurs when DeferredDecodingEntityHolder::getEntity is called can be seen below... (something to do with redirects, doesn't seem relevant, but still probably an issue because of whatever underlying issue is causing all of this)

DeferredDecodingEntityHolder.php:98, Wikibase\Content\DeferredDecodingEntityHolder->getEntity()
DeferredDecodingEntityHolder.php:132, Wikibase\Content\DeferredDecodingEntityHolder->getEntityId()
DeferredCopyEntityHolder.php:60, Wikibase\Content\DeferredCopyEntityHolder->getEntityId()
EntityContent.php:129, Wikibase\Lexeme\Content\LexemeContent->getEntityId()
EntityContent.php:433, Wikibase\Lexeme\Content\LexemeContent->getRedirectData()
EntityContent.php:612, Wikibase\Lexeme\Content\LexemeContent->getPatchedRedirect()
EntityContent.php:586, Wikibase\Lexeme\Content\LexemeContent->getPatchedCopy()
SubmitEntityAction.php:160, Wikibase\SubmitEntityAction->getPatchContent()
SubmitEntityAction.php:113, Wikibase\SubmitEntityAction->undo()
SubmitEntityAction.php:66, Wikibase\SubmitEntityAction->show()
MediaWiki.php:500, MediaWiki->performAction()

The second trace looks a bit more like it might be the cause...

DeferredDecodingEntityHolder.php:98, Wikibase\Content\DeferredDecodingEntityHolder->getEntity()
DeferredCopyEntityHolder.php:47, Wikibase\Content\DeferredCopyEntityHolder->getEntity()
EntityContent.php:523, Wikibase\Lexeme\Content\LexemeContent->equals()
SubmitEntityAction.php:116, Wikibase\SubmitEntityAction->undo()
SubmitEntityAction.php:66, Wikibase\SubmitEntityAction->show()
MediaWiki.php:500, MediaWiki->performAction()

F15409513: image.png

So how does the entity property of $latestRevision get changed to have the content that we want to undo to rather than actually having the latest content?

It looks like there some cloning logic missing from the Lexeme data model, so some mutable objects such as Form don't get cloned, so maintain their references to the originals.TASK DETAILhttps://phabricator.wikimedia.org/T189580EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: AddshoreCc: Lydia_Pintscher, Aklapper, Addshore, WMDE-leszek, Lahi, Gq86, Cinemantique, GoranSMilovanovic, QZanden, LawExplorer, Wikidata-bugs, aude, Darkdadaah, Mbch331___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs