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

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.

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.

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

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