search

guardian / media-atom-maker

A tool for creating media atoms, including a UI specifically for managing video
https://video.gutools.co.uk/videos
4 stars 3 forks source link

Drop unmaintained `diff` library for logging `MediaAtom` changes #1141

Closed rtyley closed 8 months ago

rtyley commented 8 months ago

This removes the unmaintained bizzabo/diff library ("ai.x" %% "diff") from media-atom-maker, as a prelude to the Scala 2.13 upgrade in https://github.com/guardian/media-atom-maker/pull/1140 (the diff library is only available for Scala 2.11 & 2.12).

The library is only used for the media-atom-maker audit log, logging a diff when certain fields on MediaAtom entities change. The diff text only goes to the ELK logs in the free-format message & description fields:

image

Precise format of the Audit Log diff is not crucial

Originally, there was some functionality in the media-atom-maker UI involving an audit button, but this was removed with https://github.com/guardian/media-atom-maker/pull/759 in February 2018, and the ELK became the place this information was sent to (the only place) with https://github.com/guardian/media-atom-maker/pull/767. See also https://github.com/guardian/media-atom-maker/pull/868#issuecomment-480065134 for a bit of additional context.

All this means that the exact format of the diff logged to the ELK is not that important - right now, the information could only really be used by a developer as an informative diagnostic - there is no process that cares if the diff format changes. This is probably a good thing, as 060adabbb8390cf1b0677edaba2bd0488437f83e in this PR (which captures the behaviour of the old diff by adding a unit test) shows that the format of the diff was not perfect!

Replacement

To replace the old diff library, we looked at using alternative diffing libraries (diffx or difflicious) to produce the diff, but getting them to produce the format we wanted (a one-line diff with only changed fields taken from an accept-list, and no colouring) looked like it would take a bit of work - @JamieB-gu pointed out that, given that there were only a few fields to look at, we could write our own diffing logic with little effort! The new logic produces output that is aesthetically better, I think - see the updated unit test for some examples of how the audit log message changes.

prout-bot commented 8 months ago

Seen on PROD (merged by @rtyley 6 minutes and 24 seconds ago) Please check your changes!

rtyley commented 8 months ago

This was deployed at 11:03am today, the updated log messages look to be working well:

image