fix(merge-tool): Show last know alias in merge revisions

metabrainz · May 25, 2020 · 83bda6b · 83bda6b
1 parent f3c2a70
commit 83bda6b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 15 deletions.
diff --git a/src/client/components/pages/revision.js b/src/client/components/pages/revision.js
@@ -157,14 +157,15 @@ class RevisionPage extends React.Component {
 
 	render() {
 		const {revision, diffs, user} = this.props;
-		const regularDiffs = diffs;
+		let regularDiffs = diffs;
 		let mergeDiffDivs;
 
 		if (revision.isMerge) {
 			/**
 			 * Separate entities between merged and not merged
 			 */
-			const mergeDiffs = _.remove(regularDiffs, diff => diff.entityRevision.isMerge);
+			const mergeDiffs = _.filter(diffs, diff => diff.entityRevision.isMerge);
+			regularDiffs = _.filter(diffs, diff => !diff.entityRevision.isMerge);
 
 			/**
 			 * We sort the merged entities diffs by number of changes.

diff --git a/src/client/stylesheets/style.less b/src/client/stylesheets/style.less
@@ -432,7 +432,7 @@ hr.wide {
 #mergePage {
 	.mergedEntities {
 		position: relative;
-		padding: 0.1em 1.5em;
+		padding: 0.1em 1.5em 1em;
 		margin: 0 -1.5em;
 		background-color: fade(@disabled-bg,15%);
 		&:after,

diff --git a/src/server/helpers/diffFormatters/entity.js b/src/server/helpers/diffFormatters/entity.js
@@ -393,8 +393,15 @@ export function formatEntityDiffs(diffs, entityType, entityFormatter) {
 		formattedDiff.entityRevision = diff.revision && diff.revision.toJSON();
 
 		if (diff.entityAlias) {
-			const aliasJSON = diff.entityAlias.toJSON();
-			formattedDiff.entity.defaultAlias = aliasJSON.aliasSet.defaultAlias;
+			// In the revision route, we fetch an entity's data to show its alias; an ORM model is returned.
+			// For entities without data (deleted or merged), we use getEntityParentAlias instead which returns a JSON object
+			if (typeof diff.entityAlias.toJSON === 'function') {
+				const aliasJSON = diff.entityAlias.toJSON();
+				formattedDiff.entity.defaultAlias = aliasJSON.aliasSet.defaultAlias;
+			}
+			else {
+				formattedDiff.entity.defaultAlias = diff.entityAlias;
+			}
 		}
 
 		if (!diff.changes) {

diff --git a/src/server/routes/revision.js b/src/server/routes/revision.js
@@ -161,7 +161,7 @@ function formatEditionGroupChange(change) {
 	return [];
 }
 
-function diffRevisionsWithParents(entityRevisions) {
+function diffRevisionsWithParents(orm, entityRevisions, entityType) {
 	// entityRevisions - collection of *entityType*_revisions matching id
 	return Promise.all(entityRevisions.map(
 		(revision) =>
@@ -170,15 +170,23 @@ function diffRevisionsWithParents(entityRevisions) {
 					(parent) => Promise.props({
 						changes: revision.diff(parent),
 						entity: revision.related('entity'),
-						entityAlias: revision.related('data').fetch({require: false, withRelated: ['aliasSet.defaultAlias']}),
+						entityAlias: revision.get('dataId') ?
+							revision.related('data').fetch({require: false, withRelated: ['aliasSet.defaultAlias']}) :
+							orm.func.entity.getEntityParentAlias(
+								orm, entityType, revision.get('bbid')
+							),
 						isNew: !parent,
 						revision
 					}),
 					// If calling .parent() is rejected (no parent rev), we still want to go ahead without the parent
 					() => Promise.props({
 						changes: revision.diff(null),
 						entity: revision.related('entity'),
-						entityAlias: revision.related('data').fetch({require: false, withRelated: ['aliasSet.defaultAlias']}),
+						entityAlias: revision.get('dataId') ?
+							revision.related('data').fetch({require: false, withRelated: ['aliasSet.defaultAlias']}) :
+							orm.func.entity.getEntityParentAlias(
+								orm, entityType, revision.get('bbid')
+							),
 						isNew: true,
 						revision
 					})
@@ -193,7 +201,7 @@ router.get('/:id', async (req, res, next) => {
 	} = req.app.locals.orm;
 
 	let revision;
-	function _createRevision(EntityRevisionModel) {
+	function _createRevision(EntityRevisionModel, entityType) {
 		/**
 		 * EntityRevisions can have duplicate ids
 		 * the 'merge' and 'remove' options instructs the ORM to consider that normal instead of merging
@@ -202,7 +210,7 @@ router.get('/:id', async (req, res, next) => {
 		return EntityRevisionModel.forge()
 			.where('id', req.params.id)
 			.fetchAll({merge: false, remove: false, require: false, withRelated: 'entity'})
-			.then(diffRevisionsWithParents)
+			.then((entityRevisions) => diffRevisionsWithParents(req.app.locals.orm, entityRevisions, entityType))
 			.catch(err => { log.error(err); throw err; });
 	}
 	try {
@@ -229,11 +237,11 @@ router.get('/:id', async (req, res, next) => {
 				throw new error.NotFoundError(`Revision #${req.params.id} not found`, req);
 			});
 
-		const authorDiffs = await _createRevision(AuthorRevision);
-		const editionDiffs = await _createRevision(EditionRevision);
-		const editionGroupDiffs = await _createRevision(EditionGroupRevision);
-		const publisherDiffs = await _createRevision(PublisherRevision);
-		const workDiffs = await _createRevision(WorkRevision);
+		const authorDiffs = await _createRevision(AuthorRevision, 'Author');
+		const editionDiffs = await _createRevision(EditionRevision, 'Edition');
+		const editionGroupDiffs = await _createRevision(EditionGroupRevision, 'EditionGroup');
+		const publisherDiffs = await _createRevision(PublisherRevision, 'Publisher');
+		const workDiffs = await _createRevision(WorkRevision, 'Work');
 		const diffs = _.concat(
 			entityFormatter.formatEntityDiffs(
 				authorDiffs,