Refactor watch history/diff page time handling, fixing issue where the last time viewed was not set in the 'history' page automatically (#3293)

changedetectionio/blueprint/ui/views.py

@@ -1,8 +1,7 @@
 from flask import Blueprint, request, redirect, url_for, flash, render_template, make_response, send_from_directory, abort
-from flask_login import current_user
 import os
 import time
-from copy import deepcopy
+from loguru import logger
 
 from changedetectionio.store import ChangeDetectionStore
 from changedetectionio.auth_decorator import login_optionally_required
@@ -78,7 +77,42 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
 
         return output
 
-    @views_blueprint.route("/diff/<string:uuid>", methods=['GET', 'POST'])
+    @views_blueprint.route("/diff/<string:uuid>", methods=['POST'])
+    @login_optionally_required
+    def diff_history_page_build_report(uuid):
+        from changedetectionio import forms
+
+        # More for testing, possible to return the first/only
+        if uuid == 'first':
+            uuid = list(datastore.data['watching'].keys()).pop()
+
+        try:
+            watch = datastore.data['watching'][uuid]
+        except KeyError:
+            flash("No history found for the specified link, bad link?", "error")
+            return redirect(url_for('watchlist.index'))
+
+        # For submission of requesting an extract
+        extract_form = forms.extractDataForm(request.form)
+        if not extract_form.validate():
+            flash("An error occurred, please see below.", "error")
+
+        else:
+            extract_regex = request.form.get('extract_regex').strip()
+            output = watch.extract_regex_from_all_history(extract_regex)
+            if output:
+                watch_dir = os.path.join(datastore.datastore_path, uuid)
+                response = make_response(send_from_directory(directory=watch_dir, path=output, as_attachment=True))
+                response.headers['Content-type'] = 'text/csv'
+                response.headers['Cache-Control'] = 'no-cache, no-store, must-revalidate'
+                response.headers['Pragma'] = 'no-cache'
+                response.headers['Expires'] = "0"
+                return response
+
+            flash('Nothing matches that RegEx', 'error')
+        redirect(url_for('ui_views.diff_history_page', uuid=uuid) + '#extract')
+
+    @views_blueprint.route("/diff/<string:uuid>", methods=['GET'])
     @login_optionally_required
     def diff_history_page(uuid):
         from changedetectionio import forms
@@ -96,60 +130,31 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
 
         # For submission of requesting an extract
         extract_form = forms.extractDataForm(request.form)
-        if request.method == 'POST':
-            if not extract_form.validate():
-                flash("An error occurred, please see below.", "error")
-
-            else:
-                extract_regex = request.form.get('extract_regex').strip()
-                output = watch.extract_regex_from_all_history(extract_regex)
-                if output:
-                    watch_dir = os.path.join(datastore.datastore_path, uuid)
-                    response = make_response(send_from_directory(directory=watch_dir, path=output, as_attachment=True))
-                    response.headers['Content-type'] = 'text/csv'
-                    response.headers['Cache-Control'] = 'no-cache, no-store, must-revalidate'
-                    response.headers['Pragma'] = 'no-cache'
-                    response.headers['Expires'] = 0
-                    return response
-
-                flash('Nothing matches that RegEx', 'error')
-                redirect(url_for('ui_views.diff_history_page', uuid=uuid)+'#extract')
 
         history = watch.history
         dates = list(history.keys())
 
-        if len(dates) < 2:
+        # If a "from_version" was requested, then find it (or the closest one)
-            flash("Not enough saved change detection snapshots to produce a report.", "error")
+        # Also set "from version" to be the closest version to the one that was last viewed.
-            return redirect(url_for('watchlist.index'))
 
-        # Save the current newest history as the most recently viewed
+        best_last_viewed_timestamp = watch.get_from_version_based_on_last_viewed
-        datastore.set_last_viewed(uuid, time.time())
+        from_version_timestamp = best_last_viewed_timestamp if best_last_viewed_timestamp else dates[-2]
+        from_version = request.args.get('from_version', from_version_timestamp )
 
-        # Read as binary and force decode as UTF-8
+        # Use the current one if nothing was specified
-        # Windows may fail decode in python if we just use 'r' mode (chardet decode exception)
+        to_version = request.args.get('to_version', str(dates[-1]))
-        from_version = request.args.get('from_version')
-        from_version_index = -2  # second newest
-        if from_version and from_version in dates:
-            from_version_index = dates.index(from_version)
-        else:
-            from_version = dates[from_version_index]
 
         try:
-            from_version_file_contents = watch.get_history_snapshot(dates[from_version_index])
+            to_version_file_contents = watch.get_history_snapshot(timestamp=to_version)
         except Exception as e:
-            from_version_file_contents = f"Unable to read to-version at index {dates[from_version_index]}.\n"
+            logger.error(f"Unable to read watch history to-version for version {to_version}: {str(e)}")
-
+            to_version_file_contents = f"Unable to read to-version at {to_version}.\n"
-        to_version = request.args.get('to_version')
-        to_version_index = -1
-        if to_version and to_version in dates:
-            to_version_index = dates.index(to_version)
-        else:
-            to_version = dates[to_version_index]
 
         try:
-            to_version_file_contents = watch.get_history_snapshot(dates[to_version_index])
+            from_version_file_contents = watch.get_history_snapshot(timestamp=from_version)
         except Exception as e:
-            to_version_file_contents = "Unable to read to-version at index{}.\n".format(dates[to_version_index])
+            logger.error(f"Unable to read watch history from-version for version {from_version}: {str(e)}")
+            from_version_file_contents = f"Unable to read to-version {from_version}.\n"
 
         screenshot_url = watch.get_screenshot()
 
@@ -163,6 +168,8 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
         if datastore.data['settings']['application'].get('password') or os.getenv("SALTED_PASS", False):
             password_enabled_and_share_is_off = not datastore.data['settings']['application'].get('shared_diff_access')
 
+        datastore.set_last_viewed(uuid, time.time())
+
         output = render_template("diff.html",
                                  current_diff_url=watch['url'],
                                  from_version=str(from_version),

changedetectionio/tests/test_history_consistency.py

@@ -79,3 +79,48 @@ def test_consistent_history(client, live_server, measure_memory_usage):
     json_db_file = os.path.join(live_server.app.config['DATASTORE'].datastore_path, 'url-watches.json')
     with open(json_db_file, 'r') as f:
         assert '"default"' not in f.read(), "'default' probably shouldnt be here, it came from when the 'default' Watch vars were accidently being saved"
+
+
+def test_check_text_history_view(client, live_server):
+
+    with open("test-datastore/endpoint-content.txt", "w") as f:
+        f.write("<html>test-one</html>")
+
+    # Add our URL to the import page
+    test_url = url_for('test_endpoint', _external=True)
+    res = client.post(
+        url_for("imports.import_page"),
+        data={"urls": test_url},
+        follow_redirects=True
+    )
+    assert b"1 Imported" in res.data
+
+    # Give the thread time to pick it up
+    wait_for_all_checks(client)
+
+    # Set second version, Make a change
+    with open("test-datastore/endpoint-content.txt", "w") as f:
+        f.write("<html>test-two</html>")
+
+    client.get(url_for("ui.form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    res = client.get(url_for("ui.ui_views.diff_history_page", uuid="first"))
+    assert b'test-one' in res.data
+    assert b'test-two' in res.data
+
+    # Set third version, Make a change
+    with open("test-datastore/endpoint-content.txt", "w") as f:
+        f.write("<html>test-three</html>")
+
+    client.get(url_for("ui.form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    # It should remember the last viewed time, so the first difference is not shown
+    res = client.get(url_for("ui.ui_views.diff_history_page", uuid="first"))
+    assert b'test-three' in res.data
+    assert b'test-two' in res.data
+    assert b'test-one' not in res.data
+
+    res = client.get(url_for("ui.form_delete", uuid="all"), follow_redirects=True)
+    assert b'Deleted' in res.data

changedetectionio/tests/test_jsonpath_jq_selector.py

@@ -419,13 +419,20 @@ def check_json_ext_filter(json_filter, client, live_server):
     res = client.get(url_for("watchlist.index"))
     assert b'unviewed' in res.data
 
-    res = client.get(url_for("ui.ui_views.diff_history_page", uuid="first"))
+    res = client.get(url_for("ui.ui_views.preview_page", uuid="first"))
 
     # We should never see 'ForSale' because we are selecting on 'Sold' in the rule,
     # But we should know it triggered ('unviewed' assert above)
     assert b'ForSale' not in res.data
     assert b'Sold' in res.data
 
+
+    # And the difference should have both?
+
+    res = client.get(url_for("ui.ui_views.diff_history_page", uuid="first"))
+    assert b'ForSale' in res.data
+    assert b'Sold' in res.data
+
     res = client.get(url_for("ui.form_delete", uuid="all"), follow_redirects=True)
     assert b'Deleted' in res.data