Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show more content change details #868

Merged
merged 1 commit into from
Apr 5, 2019
Merged

show more content change details #868

merged 1 commit into from
Apr 5, 2019

Conversation

akash1810
Copy link
Member

Specifically dates and publish information (if available).

This is to help diagnose issues.

image

Specifically dates and publish information (if available).

This is to help diagnose issues.
@paperboyo
Copy link
Contributor

paperboyo commented Apr 4, 2019

Great! Couldn’t we revive full audit here or am I dreaming that it was there but removed for lack of space (which now came back)?

Irregardless, wouldn’t it be more readable like:

Created at
19 Mar 2019 09:09 by akash.askoolum@guardian.co.uk

etc.? Or am I reading it wrong?

@harpreetpurewal @akemitakagi

@akash1810
Copy link
Member Author

akash1810 commented Apr 4, 2019

Couldn’t we revive full audit here or am I dreaming that it was there

Audit was removed a while ago as part of the GDPR work. One day we'll update snapshotter to work with atoms, this'll act as a more useful audit log than just rows of text.

wouldn’t it be more readable like [...]

😍 Yes! However I'm fundamentally lazy and this was the fastest way to get something out whilst reusing existing code and components.

Ideally, we'd take inspiration from Composer's management tab wrt formatting:

image

@akash1810 akash1810 merged commit bda9532 into master Apr 5, 2019
@akash1810 akash1810 deleted the aa-more-ccd branch April 5, 2019 13:48
@prout-bot
Copy link

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

rtyley added a commit that referenced this pull request Jan 31, 2024
This removes the unmaintained bizzabo/diff library ("ai.x" %% "diff") from media-atom-maker,
as a prelude to the Scala 2.13 upgrade in PR #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.

Originally, there was some functionality in the media-atom-maker UI involving an audit
button, but this was removed with #759 in February 2018, and the ELK became the place this
information was sent to (the only place) with #767. See also #868 (comment) 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 format changes.

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 - so,
given that there were only a few fields to look at, we wrote a custom diff function, and
extracted the diffing method out, so it could be easily unit-tested.
rtyley added a commit that referenced this pull request Jan 31, 2024
This removes the unmaintained bizzabo/diff library ("ai.x" %% "diff") from media-atom-maker,
as a prelude to the Scala 2.13 upgrade in PR #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.

Originally, there was some functionality in the media-atom-maker UI involving an audit
button, but this was removed with #759 in February 2018, and the ELK became the place this
information was sent to (the only place) with #767. See also #868 (comment) 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 format changes.

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 - so,
given that there were only a few fields to look at, we wrote a custom diff function, and
extracted the diffing method out, so it could be easily unit-tested.
rtyley added a commit that referenced this pull request Feb 1, 2024
This removes the unmaintained bizzabo/diff library ("ai.x" %% "diff") from media-atom-maker,
as a prelude to the Scala 2.13 upgrade in PR #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.

Originally, there was some functionality in the media-atom-maker UI involving an audit
button, but this was removed with #759 in February 2018, and the ELK became the place this
information was sent to (the only place) with #767. See also #868 (comment) 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 format changes.

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 - so,
given that there were only a few fields to look at, we wrote a custom diff function, and
extracted the diffing method out, so it could be easily unit-tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants