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

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

Open
MoshiMoshi0 opened this issue Oct 28, 2024 · 8 comments · May be fixed by #2856
Open

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

MoshiMoshi0 opened this issue Oct 28, 2024 · 8 comments · May be fixed by #2856
Assignees
Labels

Comments

@MoshiMoshi0
Copy link

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

@dgtlmoon
Copy link
Owner

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

@MoshiMoshi0
Copy link
Author

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.

@MoshiMoshi0
Copy link
Author

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

@MoshiMoshi0 MoshiMoshi0 changed the title Watch history compare "from" date does not include last viewed date Watch history compare "from" date is not set to last viewed date Oct 28, 2024
@dgtlmoon
Copy link
Owner

hmm maybe off by one bug

@MoshiMoshi0
Copy link
Author

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.

@dgtlmoon
Copy link
Owner

dgtlmoon commented Oct 28, 2024

    # 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

@dgtlmoon
Copy link
Owner

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

@dgtlmoon
Copy link
Owner

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

MoshiMoshi0 added a commit to MoshiMoshi0/changedetection.io that referenced this issue Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants