dgtlmoon/changedetection.io

Watch history compare "from" date is not set to last viewed date

Opened this issue · 8 comments

Describe the bug
When multiple changes are detected since last time you view history, the diff from date does not start from the last history check but the next one. This means the diff does not show all changes since your last history view.

Version
v0.47.03

To Reproduce
Steps to reproduce the behavior:

  • Add watch for "https://time.is/"
  • Recheck
  • Watch detects change and turns bold
  • View History - last seen change is from 11:09:32
    firefox_uMqJfGmBlR
  • Run recheck multiple times
  • Watch detects multiple changes and turns bold
  • View History - from date does not start at 11:09:32 so changes from 11:09:32 to 11:10:44 are included in the diff
    firefox_fXoGIaFvLd

Expected behavior
The history diff from date should start from last seen history date (11:09:32 in above example)

#2580 might be related

this is not a bug, its because it wants to show you the differences FROM the last time you viewed it

But it does not show differences from the last time I viewed it, it shows differences from the next change after the last time I viewed it.
If last time I viewed history was 11:09:32 then the diff from should start from that date, not the next after.

If this is not a bug then it is very confusing behavior because this means the diff is missing changes that happened from 11:09:32 to 11:10:44, and each time I have to move the from date by one.

Ok, I've spun up a server that adds a number each time the page is viewed:
firefox_PdlWFqFNfj

This is after I run "Recheck" multiple times and view the history as in the repro:
firefox_CHlhkdjG5J

You can clearly see that the "3" is not detected as added in the diff.
If the "from" date was set correctly you can see it is included in the diff:
firefox_1ewZadXTMA

hmm maybe off by one bug

I think its just a small overlook when implementing #1989

# Given an arbitrary timestamp, find the closest next key
# For example, last_viewed = 1000 so it should return the next 1001 timestamp
#
# used for the [diff] button so it can preset a smarter from_version
@property
def get_next_snapshot_key_to_last_viewed(self):
"""Unfortunately for now timestamp is stored as string key"""
keys = list(self.history.keys())
if not keys:
return None
last_viewed = int(self.get('last_viewed'))
prev_k = keys[0]
sorted_keys = sorted(keys, key=lambda x: int(x))
sorted_keys.reverse()
# When the 'last viewed' timestamp is greater than the newest snapshot, return second last
if last_viewed > int(sorted_keys[0]):
return sorted_keys[1]
for k in sorted_keys:
if int(k) < last_viewed:
if prev_k == sorted_keys[0]:
# Return the second last one so we dont recommend the same version compares itself
return sorted_keys[1]
return prev_k
prev_k = k
return keys[0]

It would have to be changed to return key one before last viewed date.
Technically the current implementation correctly returns first not viewed history entry, but to get the full diff it needs to be one before that.

    # For example, last_viewed = 1000 so it should return the next 1001 timestamp

Yeah maybe needs to be the actual last_viewed item (1000), not the NEXT one,as you say

so basically its thinking the last-viewed is actually the one AFTER the last-viewed

it should also tell you on that page which is the "last viewed"