Page MenuHomePhabricator

Build EntityDiffViewFactory for different entity types and use it in EntityDiffVisualizer
Closed, ResolvedPublic

Description

Proposed solution for T160615: Investigate why changes to Lexeme entity are not shown on the diff page

Have the EntityDiffViewFactory interface that returns instances of DiffView, get it implemented in different entity types using entity type definition class. And use it in EntityDiffVisualizer

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 16 2017, 4:36 PM

I agree with the general idea, but I would structure the solution a bit differently:

  • turn EntityDiffVisualizer into an interface.
  • create a BasicEntityDiffVisualizer implementation to share code between the implementation for different entity types - or maybe better, an EntityDiffVisualizerHelper (or EntityDiffComponentVisualizer), so we can use composition instead of subclassing.
  • create a DispatchingEntityDiffVisualizer, using callbacks from EntityTypeDefinitions.
WMDE-leszek triaged this task as Normal priority.Mar 21 2017, 11:28 AM
Restricted Application added projects: Wikidata, User-Ladsgroup. · View Herald TranscriptMar 22 2017, 1:45 PM
Ladsgroup moved this task from Incoming to In progress on the User-Ladsgroup board.

I started it by doing something very simple: Renaming EntityDiffVisualizer to BasicEntityDiffVisualizer. once this is merged (or in a followup) I make a patch for adding EntityDiffVisualizer as an interface. In a follow-up patch, I make the DispatchingEntityDiffVisualizer then ItemEntityDiffVisualizer.

Change 344146 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Rename EntityDiffVisualizer to BasicEntityDiffVisualizer

https://gerrit.wikimedia.org/r/344146

Made https://gerrit.wikimedia.org/r/344175 now to add the interface and use it. Dispatching will be in another patch.

Ladsgroup moved this task from Doing to Review on the Wikidata-Former-Sprint-Board board.

Change 344175 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Introduce EntityDiffVisualizer interface and use it in BasicEntityDiffVisualizer

https://gerrit.wikimedia.org/r/344175

Given that gerritbot still doesn't want to add comment when I make a patch:

Change 344943 merged by Daniel Kinzler:
[mediawiki/extensions/Wikibase@master] Add $entityType attribute to EntityContentDiff

https://gerrit.wikimedia.org/r/344943

For the record there is a very basic patch: https://gerrit.wikimedia.org/r/#/c/345091/

Change 382739 had a related patch set uploaded (by Ladsgroup; owner: Amir Sarabadani):
[mediawiki/extensions/Wikibase@master] Decouple DiffView and RequestContext

https://gerrit.wikimedia.org/r/382739

Change 382739 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Decouple DiffView and RequestContext

https://gerrit.wikimedia.org/r/382739

Change 384051 had a related patch set uploaded (by Ladsgroup; owner: Amir Sarabadani):
[mediawiki/extensions/Wikibase@master] Introduce 'entity-diff-visualizer-callback' in entity type definitions

https://gerrit.wikimedia.org/r/384051

Change 384051 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Introduce 'entity-diff-visualizer-callback' in entity type definitions

https://gerrit.wikimedia.org/r/384051

Change 345091 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Introduce DispatchingEntityDiffVisualizer and EntityDiffVisualizerFactory

https://gerrit.wikimedia.org/r/345091

Ladsgroup moved this task from In progress to Done on the User-Ladsgroup board.Nov 2 2017, 4:44 PM
Ladsgroup closed this task as Resolved.Nov 23 2017, 1:38 PM
Ladsgroup removed a project: Patch-For-Review.