FIX(Revision): "New" badge appears in entity deletion revision

[akashgp09]

Problem

This PR Fixes: BB-523

Solution

Refactored the isNew property logic in server/routes/revision.js by taking advantage of dataId( which is null for a deleted entity)

Before

After

Areas of Impact

server/routes/revision.js

[coveralls]

Coverage decreased (-0.04%) to 60.856% when pulling 941c3a3 on akashgp09:revision-diff into 8a3c5d8 on bookbrainz:master.

[MonkeyDo]

Great, that's a good improvement!

I think it would be great to take that opportunity to mark deleted entities with a Deleted badge with a red background, now that you've got that piece of info in your logic.
I guess the cleanest would be to pass a new isDeletion property.

What do you think?

[akashgp09]

Added the deleted badge to mark deleted entitites. Let me know if any changes are required :)

[MonkeyDo]

Yes, that's nice, thanks for adding that :)

[MonkeyDo]

OK, this is starting to look really nice!
I went and deployed this PR to test.bookbraine.org so that we can do some testing on a common space.
It's working pretty well and showing the right badges where expected most of the time, but I did find a couple of issues:

If the entity has been merged into another entity, it will display the "deleted" badge, because for merges we also remove the dataId of any merged entity (apart from the one we are merging into).
I think it should be relatively easy to make another "merged" badge (and perhaps that will entirely replace the current way we display merged entities in a revision. That's for another PR)
An example of that: https://test.bookbrainz.org/revision/29798

In another case, I deleted an entity that had some previous revisions, and it appears with the "new" badge instead of "deleted". Not sure what is happening here:
https://test.bookbrainz.org/revision/29849
I deleted the entity 6ef597ce-9f15-4c91-ae8d-83c18a0c6797 and nothing else in that revision and the database seems to reflect that correctly:

[akashgp09]

I think it should be relatively easy to make another "merged" badge (and perhaps that will entirely replace the current way we display merged entities in a revision. That's for another PR)

Ok sure will open a PR for mergedbadge to mark merged entities once this PR gets merged.

In another case, I deleted an entity that had some previous revisions, and it appears with the "new" badge instead of "deleted". Not sure what is happening here:
https://test.bookbrainz.org/revision/29849
I deleted the entity 6ef597ce-9f15-4c91-ae8d-83c18a0c6797 and nothing else in that revision and the database seems to reflect that correctly.

Yeah thanks for reporting it. I have made required changes and it seems to work fine now

[MonkeyDo]

All working fine now, thank you ! :)