Fixes #14329: Improve diffs for custom fields

[JCWasmx86]

Fixes: #14329

This improves the diff by comparing the changes dicts recursively. It changes the Diff and the Pre/Post-Change Data

That's one issue I found, but it seems unrelated, as even without my patch, the type of the floating-point field seems to change:

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

[jeremystretch]

@arthanson @jeffgdotorg and I talked about this today. The consensus was that we need to address the inconsistency in how the summary difference and the pre-/post-change data are presented. Looking at the screenshot below, note that while the "difference" panel is accurate, the changes highlighted in the pre- & post-change panels still show the entire custom_fields as having changed. (@JCWasmx86 does call this out in the original PR description above.)

IMO the pre-/post-change panel highlights should exactly match the summary above, or we should remove the highlighting in that section.

[JCWasmx86]

I've implemented the requested changes: (Only works for custom_fields, not e.g. for tag lists) But it will only show the changed fields, asI'm not sure how to mix them with unchanged fields in an elegant manner

Sadly I wasn't able to find the reason that ffield changes from float to str despite just editing another custom field.

[JCWasmx86]

I've got it working in a somewhat unelegant manner.

I take the list of custom_fields, convert it into such a list:

[
  ('cf_name', 'cf_value', True)
  ('cf_name2', 'cf_value2', False)
]

And within django templates I parse this and transform this into JSON like that:

"custom_fields": {
  "cf_name": "cf_value" // Will be colored red/green as the third item in the tuple is True
  "cf_name2": "cf_value2" // Won't be colored, as it's False
}

sadly this won't work with MultiObject Custom Fields, as it would end up like this:

"custom_fields": {
  "cf_name": [
1,
2,
3,
]
}

For that I've created a custom template tag. It transforms a string by:

• Preserving everything from the first line
• Indenting everything else, so it looks aligned in the GUI

I don't claim this is the best solution, but it's honestly the only one I could come up with. I'm open for alternative ideas how to make this work

[JCWasmx86]

Hello @jeremystretch,

I would love to get this merged in near future. Can you either review this (Especially how I solved those diffs in the Pre/Post Change Data!) please or give me a list of open points that are missing?

[jeremystretch]

I'm afraid this sort of approach isn't going to be maintainable long-term. The diffing implementation needs to operate generically; we can't give custom fields specifically any special treatment. (Consider what changes would be needed to support some similarly structured field in a future release.) Ultimately, we want a solution that can follow nested data structures and roll back up their diffs, without any context about the data contained within.

[JCWasmx86]

Ok, I have one last idea how to implement this in a sane manner:

• Dump "Object before" and "Object after into a formatted JSON-string => One Attribute = One line (Except for arrays/nested objects).
• Diff line by line (With a bit of smartness for arrays/objects) into two lists removed and added that are just lists of (line_as_string, has_changed)
• Convert the lists to HTML within the templates

Do you think this could be a more maintainable approach?

[JCWasmx86]

I made this script: https://gist.github.com/JCWasmx86/95f449abf2a8958b22d0e48279b07ba7

Do you think that goes into the right direction? It's a bit less efficient than it could be, but I think it gives good results, e.g.

It works some-what generic on strings, but a sorted JSON-like thing (Like in the changelog pre/post data), it should work great

[JCWasmx86]

This will have to be cleaned up a bit, if you think this approach is better. If you don't think this approach has a future, this PR can be closed, as I'm out of ideas.

We basically just convert two strings (Before/After) into two lists of tuples (str, bool).

And it needs a few more tests, but I only write them if this approach has a future