content modelling/830 show changes to a content block

[Harriethw]

• add edition's field diff to content block versions

we want to show the changes that have been made to
a block when updating - this takes a snapshot of
the difference between the user generated fields
and saves them as a json blob, with the key being
the field name.

I'm hesitant to use json as it isn't typed, but I
explored [1] codifying the data structure
previously and it proved too complicated and
inefficient, especially as the fields need to be
displayed to the user in a certain order of
priority, and we don't know exactly what the
fields we're saving will be. By using the Data
class to show the shape of the items, this is
hopefully a halfway house. Shoving everything in a
json blob seems like a middle ground between just
calculating everything at run time and having a
genuine record of the change.

[1] #9838

• Add content block changes table to timeline

As design

---

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

[pezholio]

I wonder if you might be better having FIELD_DIFF as a separate class with a .all_for_document class method. That way you can test it in isolation a bit better, as well as breaking down the methods in field_diff a bit more. Something like:

class FieldDiff < Data.define(:previous_value, :new_value)
  def self.all_for_document
     # logic from `field_diff` goes here, maybe broken down into smaller methods
  end
end

[pezholio]

I'm also wondering if you could have a way of translating the JSON into a FieldDiff object, that way if there are any changes to the structure, you could handle that programatically, rather than having to deal with JSON objects that may have changed over time

[Harriethw]

good shout about breaking out the logic, will take a look.

I'm also wondering if you could have a way of translating the JSON into a FieldDiff object

I was wondering if there was way to use jsonschema in some way - basically it should always follow this shape

{
  {field_key}: {
    "previous_value": string,
    "new_value": string,
  }
}

but with the field_key being unknown i don't how to say that in rails land 🤔

[pezholio]

Maybe having an array of objects like this might be more predictable:

[
  { "key": string, "previous_value": string, "new_value": string }
]

[Harriethw]

I tried the array part in the last approach but it's a nightmare to then try and work out the order and display it correctly on the front end...

[Harriethw]

though maybe now we are doing the ordering in the audit_trail the frontend is less a problem, as long as the order was preserved when we iterate through to make the rows...

[pezholio]

Could you add an "order" param to each object maybe?

[Harriethw]

Could you add an "order" param to each object maybe?
I think it would be a bit difficult to do that because we don't know beforehand what the keys could be that come out of the details blob, but maybe there's a way around that...

[Harriethw]

ok I've had a go at your suggestions and think it's looking better 👍

[pezholio]

This is loads better! 👍

[pezholio]

This looks ace. Thanks for struggling through it all! A couple of minor comments, but I think it looks good overall 👍

[pezholio]

You could probably just do new(...) here

[pezholio]

You could probably just do map here:

Suggested change

	rows = []
	version.field_diffs.each do |field|
	rows.append([{ text: field["field_name"].humanize }, { text: field["previous_value"] }, { text: field["new_value"] }])
	end
	rows
	version.field_diffs.map do |field|
	[
	{ text: field["field_name"].humanize },
	{ text: field["previous_value"] },
	{ text: field["new_value"] },
	]
	end

(Added some linebreaks for readability too)

[Harriethw]

@pezholio thank you!! have made those suggestions. I don't think I'll be around to watch it go green so feel free to merge on Monday when I'm not in