Show what was modified between entry history items

[shemeshg]

Allow user understand what has changed in the history view
NOTE: # ( Explain large or complex code modifications. )
Simple change,

Fixes #2621

Screenshots

Testing strategy

Perform all types of changes, and preview each of them in the History View.
TIP: # ( We expect new code to be covered by unit tests and documented with doc blocks! )

Type of change

• ✅ Bug fix (non-breaking change that fixes an issue)
• ✅ New feature (change that adds functionality)
• ✅ Refactor (significant modification to existing code)
  Only,

-    m_historyModel->setEntries(m_entry->historyItems());
+    m_historyModel->setEntries(m_entry->historyItems(), m_entry);

[droidmonkey]

Why show the notes? You have that as a change test already. I think showing the size of the history entry, number of attachments, and a calculated age (days, months, years depending on value) would be nice fields to have.

[shemeshg]

Why show the notes? You have that as a change test already. I think showing the size of the history entry, number of attachments, and a calculated age (days, months, years depending on value) would be nice fields to have.

All Corrected

[michaelk83]

The Size and Age columns are unclear. The column header should clarify what is being measured (size of what? age of what?), and either the header or rows should show units: Is the age in days? hours? weeks? Is the size in bytes? number of entries? something else?

Also it would be nice to see a diff between two history rows when selected - how was the data modified? (Or on a simpler level, at least show the old and new values next to each other.) But that's better left to a separate PR.

[droidmonkey]

Yah I tested this and it was not clear to me what a lot of the info meant. This needs a polish.

[droidmonkey]

Major overhaul! This is now ready for prime time. @michaelk83 @phoerious @hifi

[codecov-commenter]

Codecov Report

Merging #6789 (4fffb22) into develop (12990e5) will increase coverage by 0.06%.
The diff coverage is 80.88%.

@@             Coverage Diff             @@
##           develop    #6789      +/-   ##
===========================================
+ Coverage    64.58%   64.64%   +0.06%     
===========================================
  Files          335      335              
  Lines        42066    42172     +106     
===========================================
+ Hits         27166    27258      +92     
- Misses       14900    14914      +14     

Impacted Files	Coverage Δ
src/core/Tools.h	0.00% <ø> (ø)
src/gui/entry/EditEntryWidget.cpp	72.16% <40.00%> (-0.18%)	⬇️
src/core/Tools.cpp	71.33% <77.78%> (+0.69%)	⬆️
src/gui/entry/EntryHistoryModel.cpp	82.17% <85.86%> (+4.39%)	⬆️
src/core/Entry.cpp	84.12% <0.00%> (+0.10%)	⬆️
src/core/FileWatcher.cpp	86.75% <0.00%> (+1.20%)	⬆️
src/core/Base32.cpp	98.07% <0.00%> (+1.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12990e5...4fffb22. Read the comment docs.

[droidmonkey]

Kind of funny that the gui tests pass 🤣 I didn't touch them and made major changes to the way history works under the hood. Might need to address that some day.

[michaelk83]

Looks good. I can see from the code that "Size" is how much the entry takes up in the DB. It's still not entirely clear to me from the UI alone, but I can't think of any other interpretations, so I guess it's good enough.

The date and age don't add up in the first two rows. Different age, but same timestamp. Was the timestamp not updated (a bug)?

Also, rounding down 10 days to 1 week seems like a bit much. Maybe change the cut-off for "X weeks" to 2 and up?

[droidmonkey]

Current is meant to indicate (along with the bold) that it is the current version of the entry. To make the differences column make any sense it was necessary to show the current entry as a "history item" so you can see the true lineage. This also means the last line item will always contain no differences since it is the base item.

Size is consistent with the main entry view.

[michaelk83]

I don't have a problem with the "Current", but the 5 min old time is the same as the current one. That doesn't make sense. I'd expect to see that 5 min difference in the timestamp too.

Hmm, are you saying the "Current" version is also 5 min old? How about "5 minute(s) (Current)"?

[droidmonkey]

Yes they are both 5 min old. I wanted to keep it short because anything longer tends to get cut off. The age column is just a relative human readable version of the modified time. It isn't meant to be gospel. Gospel is the modified time column.

[michaelk83]

Understandable. It's just confusing this way. I would just drop the "Current" label and stick to ages + bold. You can re-add the "Current" as a tooltip.

Btw, with the "Current" label as is, does sorting by the Age column still work as expected?

[droidmonkey]

Yes sorting still works, sorting is based on the actual seconds from today value. I don't think it is confusing, will wait for other feedback. It distinguishes it from the other, actual, history items along with the bold.

[michaelk83]

The confusing part for me was not clearly seeing the age of the current version. But I guess that's because I don't know when that screenshot was taken, so can't tell the age from the timestamp (without some mental juggling based on the older versions).

[droidmonkey]

I can support "Current (x days)"

[droidmonkey]

Made the tweak mentioned above and also used rounding in the human readable time difference for weeks and months. 1.5 weeks -> 2 weeks. That should give a better feel for the near term timeframes.

Keep in mind the (s) at the end will end up being replaced with the singular and plural forms once translated.