This is my version of the event logging patch. It has very simple database and .py code, most of the "complexity" is in the HTML template. I think that this way, it is easier to understand.
Comments?
Michal Sojka (4): Add humanize filter for Jinja2 templates Extend database schema for event logging RFC: Add event logging Move Events block before user rename/id block
app.py | 62 +++++++++++++++++++++++++++++++-- coffee_db.py | 23 +++++++++++++ coffee_db.sql | 22 ++++++++++++ templates/main.js | 31 ++++++++++++++++- templates/user.html | 84 ++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 213 insertions(+), 9 deletions(-)
This filter converts date/time values into relative time strings such as "an hour ago".
It will be used in later commits. --- app.py | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/app.py b/app.py index b4d7e41..75b93d7 100644 --- a/app.py +++ b/app.py @@ -11,10 +11,11 @@ from io import BytesIO import coffee_db as db import time import sys -from datetime import date, timedelta +from datetime import date, timedelta, datetime, timezone
from json import loads from requests import post +import jinja2
db.init_db() app = Flask(__name__) @@ -22,6 +23,51 @@ CORS(app, supports_credentials=True) app.secret_key = b'_5#y2L"F4Q8z\n\xec]/'
+# Inspired by https://shubhamjain.co/til/how-to-render-human-readable-time-in-jinja/, +# updated to our needs +def humanize_ts(time): + """ + Convert date in ISO format to relative, human readable string + like 'an hour ago', 'Yesterday', '3 months ago', + 'just now', etc + """ + if jinja2.is_undefined(time): + return time + now = datetime.now(timezone.utc) + if time[-1] == 'Z': # Convert Zulu time zone to datetime compatible format + time = time[0:-1] + '+00:00' + diff = now - datetime.fromisoformat(time) + second_diff = diff.seconds + day_diff = diff.days + + if day_diff < 0: + return '' + + if day_diff == 0: + if second_diff < 10: + return "just now" + if second_diff < 60: + return str(int(second_diff)) + " seconds ago" + if second_diff < 120: + return "a minute ago" + if second_diff < 3600: + return str(int(second_diff / 60)) + " minutes ago" + if second_diff < 7200: + return "an hour ago" + if second_diff < 86400: + return str(int(second_diff / 3600)) + " hours ago" + if day_diff == 1: + return "Yesterday" + if day_diff < 7: + return str(day_diff) + " days ago" + if day_diff < 31: + return str(int(day_diff / 7)) + " weeks ago" + if day_diff < 365: + return str(int(day_diff / 30)) + " months ago" + return str(int(day_diff / 365)) + " years ago" + +app.jinja_env.filters['humanize'] = humanize_ts + @app.route('/') def hello(): if "uid" in session:
Actual event logging will be implemented in the next commits. --- coffee_db.sql | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/coffee_db.sql b/coffee_db.sql index 51f83aa..7f872d2 100644 --- a/coffee_db.sql +++ b/coffee_db.sql @@ -38,6 +38,28 @@ insert or ignore into days values (0),(1),(2),(3),(4),(5),(6) ;
+create table if not exists event_types ( + id integer primary key, + name varchar(255) not null +); + +insert or ignore into event_types values + (0, 'COFFEE_PACK_OPENED'), + (1, 'COFFEE_PACK_LAST_OPENED'), + (2, 'COFFEE_MACHINE_CLEANED'), + (3, 'MILK_CONTAINER_CLEANED'), + (4, 'MILK_CONTAINER_PILL_CLEANED') -- pill cleaning implies cleaning of the container +; + +create table if not exists events ( + id integer primary key, + event_type integer not null, + user_id varchar(24) references users(id), + time datetime default current_timestamp not null, + foreign key(event_type) references event_types(id) +); +create index if not exists idx_events on events (event_type, time); + CREATE TABLE if not exists identifiers ( `userid` varchar ( 24 ) NOT NULL, `id` varchar ( 24 ) PRIMARY KEY NOT NULL,
Compared to the previous version of the patch, now the main logic resides in templates/user.html. I think that this way it is a bit easier to understand what's going on.
The stuff in main.js related to eventName variable needs to be updated or cleaned up. I think I didn't get how it was supposed to work in the previous version of the patch. --- app.py | 14 ++++++++-- coffee_db.py | 23 +++++++++++++++++ templates/main.js | 31 ++++++++++++++++++++++- templates/user.html | 62 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 3 deletions(-)
diff --git a/app.py b/app.py index 75b93d7..faeb82b 100644 --- a/app.py +++ b/app.py @@ -113,11 +113,14 @@ def user(): counts=counts, identifiers=db.list_user_identifiers(uid), iid=session["iid"], - stamp=time.time() + stamp=time.time(), + last_events=db.last_events() ) # TODO: Replace stamp parameter with proper cache control HTTP # headers in response - return render_template('user.html', stamp=time.time()) + return render_template('user.html', stamp=time.time(), + last_events=db.last_events(), + )
@app.route('/user/rename') @@ -262,6 +265,13 @@ def coffee_add(): db.add_coffee(session["iid"], json["flavor"], json["time"]) return redirect(url_for('user'))
+@app.route("/event/add", methods=["POST"]) +def event_add(): + if request.method == "POST": + json = request.json + print("User '%(uid)s' registered event %(event_name)s at %(time)s" % json) + db.add_event(json["uid"], json["event_name"], json["time"]) + return redirect(url_for('user'))
# TODO: Remove me - unused @app.route("/coffee/count") diff --git a/coffee_db.py b/coffee_db.py index 25551f9..24d6e39 100644 --- a/coffee_db.py +++ b/coffee_db.py @@ -99,6 +99,16 @@ def add_coffee(uid, flavor, time=None): c.execute("insert or ignore into coffees (id, flavor, time) values (?,?,?)", (uid, flavor, time)) close_db(conn)
+ +def add_event(uid, event_name, time): + conn, c = open_db() + c.execute("""insert into events (user_id, event_type, time) + values (?, (SELECT id FROM event_types WHERE name = ?), ?)""", + (uid, event_name, time)) + close_db(conn) + + + def flavors(): conn, c = open_db() res = list(c.execute("select distinct name, ord from flavors")) @@ -193,3 +203,16 @@ def drink_count(uid=None, start=None, stop=None): "left join identifiers ids on co.id = ids.id where " + " and ".join(clauses) + " group by fl.type " "order by fl.ord asc", args)) + +def last_events(): + """Return mapping with event names as keys and SQLite time string of + the last event as values. + """ + + conn, c = open_db() + + res = dict(c.execute("""select name, MAX(time) + from events as e left join event_types as et on e.event_type = et.id + group by name""")) + close_db(conn) + return res diff --git a/templates/main.js b/templates/main.js index 417dc40..0483423 100644 --- a/templates/main.js +++ b/templates/main.js @@ -29,6 +29,9 @@ function replayOfflineQueue() { }
var flavorChosen; +var eventName; // TODO... +var eventStatus; +var eventAction;
// Central function to update UI elements. To ensure that the UI is // consistent, other code should only change state variables and then @@ -48,8 +51,14 @@ function updateUI()
if (id_user !== undefined) { document.getElementById("nextStep").innerHTML = "Now select a beverage on the coffee machineā¦"; - } else { + } else if (flavorChosen !== undefined) { document.getElementById("nextStep").innerHTML = "Enjoy your " + flavorChosen + "!"; + // clean the coffee machine makes it cleaned + // ^ Action ^ Name ^ Status + } else if (eventStatus !== undefined && eventStatus !== "") { + document.getElementById("nextStep").innerHTML = "You have " + eventStatus + " the " + eventName + ". Thanks!"; + } else { + document.getElementById("nextStep").innerHTML = "Event '" + eventAction + "' reported for the " + eventName + ". Thanks!"; }
if (timeToLogout !== undefined) @@ -171,6 +180,7 @@ function logout() { id_user = undefined; timeToLogout = undefined; identifier_registration = false; + window.scrollTo(0, 0); // Scroll up }
function countingTimeLogout(count_time) @@ -212,6 +222,25 @@ function addCoffee(flavor, time = new Date()) { } }
+ +function addEvent(event_name, time = new Date()) { + var data = JSON.stringify({ + time: time.toISOString(), + event_name: event_name, + uid: id_user + }); + if (id_user) { + ajax("POST", "event/add", data, "user"); + id_user = undefined; + eventName = name; // TODO... + eventStatus = status; + eventAction = action; + countingTimeLogout(10); //mean 10 seconds + window.scrollTo(0, 0); // Scroll up + } +} + + function addIdentifier_start() { identifier_registration = true; document.getElementById("addIdentifier").disabled = true; diff --git a/templates/user.html b/templates/user.html index 5ad7b90..ed0156d 100644 --- a/templates/user.html +++ b/templates/user.html @@ -1,3 +1,26 @@ +<style> +.events { + margin: 0.8em; + margin-bottom: 1.5em; + padding: 2px; + border-spacing: 2em 0; + text-align: center; +} + +.events-list { + border: 1px solid black; + padding: 0.5em; + display: inline-block; + vertical-align: top; +} + +.events-list h4 { + margin: 0.5ex; + margin-bottom: 1ex; + border-bottom: 1px dashed black; +} +</style> + {% if name %} <form style="position: absolute; right: 15%; width: 15%; height: 15%;"> <button type="button" id="logout_button" onclick="logout()" style="width: 100%; height: 100%;">logout</button> @@ -77,3 +100,42 @@ <img src="{{ url_for('coffee_graph_history', _external=True, stamp=stamp) }}"> <img src="{{ url_for('coffee_graph_flavors', _external=True, stamp=stamp, days=7) }}"> {% endif %} + + <br /> + <form> + <h3>{{ "Record event" if name else "Events" }}:</h3> + {% macro event_box(title, events) -%} + <div class="events-list"> + <h4>{{title}}</h4> + {% set verb = { + "COFFEE_MACHINE_CLEANED": "cleaned", + "COFFEE_PACK_OPENED": "opened", + "COFFEE_PACK_LAST_OPENED": "last opened", + "MILK_CONTAINER_CLEANED": "cleaned", + "MILK_CONTAINER_PILL_CLEANED": "pill cleaned", + } + %} + {% if name %} {# User logged in - show action buttons #} + {% for event in events %} + <input type="button" value="{{ verb[event] }}" + onclick="addEvent('{{ event }}')" /> + ({{ last_events[event] | humanize if event in last_events else "never" }}) + <br /> + {% endfor %} + {% else %} {# Nobody logged in - show last times #} + {# Calculate maximum timestamp of all relevant events #} + {% set when = last_events.items() | selectattr(0, 'in', events) | map(attribute=1) | max | humanize %} + {% if when %} + {{ verb[events[0]] }} {{ when }} + {% else %} + never {{ verb[events[0]] }} + {% endif %} + {% endif %} + </div> + {%- endmacro %} + <div class="events"> + {{ event_box('Coffee machine', ['COFFEE_MACHINE_CLEANED'] ) }} + {{ event_box('Coffee pack', ['COFFEE_PACK_OPENED', 'COFFEE_PACK_LAST_OPENED'] ) }} + {{ event_box('Milk container', ['MILK_CONTAINER_CLEANED', 'MILK_CONTAINER_PILL_CLEANED'] ) }} + </div> + </form>
I have mainly cosmetics comments, please take them as that.
On Tue, Aug 18, 2020 at 09:19:22PM +0200, Michal Sojka wrote:
Compared to the previous version of the patch, now the main logic resides in templates/user.html. I think that this way it is a bit easier to understand what's going on.
The stuff in main.js related to eventName variable needs to be updated or cleaned up. I think I didn't get how it was supposed to work in the previous version of the patch.
app.py | 14 ++++++++-- coffee_db.py | 23 +++++++++++++++++ templates/main.js | 31 ++++++++++++++++++++++- templates/user.html | 62 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 3 deletions(-)
diff --git a/app.py b/app.py index 75b93d7..faeb82b 100644 --- a/app.py +++ b/app.py @@ -113,11 +113,14 @@ def user(): counts=counts, identifiers=db.list_user_identifiers(uid), iid=session["iid"],
stamp=time.time()
stamp=time.time(),
# TODO: Replace stamp parameter with proper cache control HTTP # headers in responselast_events=db.last_events() )
- return render_template('user.html', stamp=time.time())
- return render_template('user.html', stamp=time.time(),
last_events=db.last_events(),
)
@app.route('/user/rename') @@ -262,6 +265,13 @@ def coffee_add(): db.add_coffee(session["iid"], json["flavor"], json["time"]) return redirect(url_for('user'))
+@app.route("/event/add", methods=["POST"])
POST always create new object by (REST) convention. Therefore, sending POST to endpoint "/event" is clearer.
+def event_add():
- if request.method == "POST":
I think (but not sure) that this is unnecessary due to that only POST method is allowed (`methods=["POST"]`).
json = request.json
print("User '%(uid)s' registered event %(event_name)s at %(time)s" % json)
db.add_event(json["uid"], json["event_name"], json["time"])
- return redirect(url_for('user'))
# TODO: Remove me - unused @app.route("/coffee/count") diff --git a/coffee_db.py b/coffee_db.py index 25551f9..24d6e39 100644 --- a/coffee_db.py +++ b/coffee_db.py @@ -99,6 +99,16 @@ def add_coffee(uid, flavor, time=None): c.execute("insert or ignore into coffees (id, flavor, time) values (?,?,?)", (uid, flavor, time)) close_db(conn)
+def add_event(uid, event_name, time):
- conn, c = open_db()
- c.execute("""insert into events (user_id, event_type, time)
values (?, (SELECT id FROM event_types WHERE name = ?), ?)""",
(uid, event_name, time))
- close_db(conn)
Different number of blank lines between functions looks ugly.
def flavors(): conn, c = open_db() res = list(c.execute("select distinct name, ord from flavors")) @@ -193,3 +203,16 @@ def drink_count(uid=None, start=None, stop=None): "left join identifiers ids on co.id = ids.id where " + " and ".join(clauses) + " group by fl.type " "order by fl.ord asc", args))
+def last_events():
- """Return mapping with event names as keys and SQLite time string of
- the last event as values.
- """
- conn, c = open_db()
- res = dict(c.execute("""select name, MAX(time)
from events as e left join event_types as et on e.event_type = et.id
group by name"""))
- close_db(conn)
- return res
diff --git a/templates/main.js b/templates/main.js index 417dc40..0483423 100644 --- a/templates/main.js +++ b/templates/main.js @@ -29,6 +29,9 @@ function replayOfflineQueue() { }
var flavorChosen; +var eventName; // TODO... +var eventStatus; +var eventAction;
// Central function to update UI elements. To ensure that the UI is // consistent, other code should only change state variables and then @@ -48,8 +51,14 @@ function updateUI()
if (id_user !== undefined) { document.getElementById("nextStep").innerHTML = "Now select a beverage on the coffee machineā¦";
} else {
} else if (flavorChosen !== undefined) { document.getElementById("nextStep").innerHTML = "Enjoy your " + flavorChosen + "!";
// clean the coffee machine makes it cleaned
// ^ Action ^ Name ^ Status
} else if (eventStatus !== undefined && eventStatus !== "") {
document.getElementById("nextStep").innerHTML = "You have " + eventStatus + " the " + eventName + ". Thanks!";
} else {
document.getElementById("nextStep").innerHTML = "Event '" + eventAction + "' reported for the " + eventName + ". Thanks!"; } if (timeToLogout !== undefined)
@@ -171,6 +180,7 @@ function logout() { id_user = undefined; timeToLogout = undefined; identifier_registration = false;
- window.scrollTo(0, 0); // Scroll up
}
function countingTimeLogout(count_time) @@ -212,6 +222,25 @@ function addCoffee(flavor, time = new Date()) { } }
+function addEvent(event_name, time = new Date()) {
- var data = JSON.stringify({
time: time.toISOString(),
event_name: event_name,
uid: id_user
- });
- if (id_user) {
ajax("POST", "event/add", data, "user");
See POST event route comment.
id_user = undefined;
eventName = name; // TODO...
eventStatus = status;
eventAction = action;
countingTimeLogout(10); //mean 10 seconds
window.scrollTo(0, 0); // Scroll up
- }
+}
function addIdentifier_start() { identifier_registration = true; document.getElementById("addIdentifier").disabled = true; diff --git a/templates/user.html b/templates/user.html index 5ad7b90..ed0156d 100644 --- a/templates/user.html +++ b/templates/user.html @@ -1,3 +1,26 @@ +<style> +.events {
- margin: 0.8em;
- margin-bottom: 1.5em;
- padding: 2px;
- border-spacing: 2em 0;
- text-align: center;
+}
+.events-list {
- border: 1px solid black;
- padding: 0.5em;
- display: inline-block;
- vertical-align: top;
+}
+.events-list h4 {
- margin: 0.5ex;
- margin-bottom: 1ex;
- border-bottom: 1px dashed black;
+} +</style>
{% if name %} <form style="position: absolute; right: 15%; width: 15%; height: 15%;"> <button type="button" id="logout_button" onclick="logout()" style="width: 100%; height: 100%;">logout</button> @@ -77,3 +100,42 @@ <img src="{{ url_for('coffee_graph_history', _external=True, stamp=stamp) }}"> <img src="{{ url_for('coffee_graph_flavors', _external=True, stamp=stamp, days=7) }}"> {% endif %}
<br />
<form>
<h3>{{ "Record event" if name else "Events" }}:</h3>
{% macro event_box(title, events) -%}
<div class="events-list">
<h4>{{title}}</h4>
{% set verb = {
"COFFEE_MACHINE_CLEANED": "cleaned",
"COFFEE_PACK_OPENED": "opened",
"COFFEE_PACK_LAST_OPENED": "last opened",
"MILK_CONTAINER_CLEANED": "cleaned",
"MILK_CONTAINER_PILL_CLEANED": "pill cleaned",
}
%}
{% if name %} {# User logged in - show action buttons #}
{% for event in events %}
<input type="button" value="{{ verb[event] }}"
onclick="addEvent('{{ event }}')" />
({{ last_events[event] | humanize if event in last_events else "never" }})
<br />
{% endfor %}
{% else %} {# Nobody logged in - show last times #}
{# Calculate maximum timestamp of all relevant events #}
{% set when = last_events.items() | selectattr(0, 'in', events) | map(attribute=1) | max | humanize %}
{% if when %}
{{ verb[events[0]] }} {{ when }}
{% else %}
never {{ verb[events[0]] }}
{% endif %}
{% endif %}
</div>
{%- endmacro %}
<div class="events">
{{ event_box('Coffee machine', ['COFFEE_MACHINE_CLEANED'] ) }}
{{ event_box('Coffee pack', ['COFFEE_PACK_OPENED', 'COFFEE_PACK_LAST_OPENED'] ) }}
{{ event_box('Milk container', ['MILK_CONTAINER_CLEANED', 'MILK_CONTAINER_PILL_CLEANED'] ) }}
</div>
</form>
-- 2.28.0.rc2
Coffee mailing list Coffee@rtime.felk.cvut.cz https://rtime.felk.cvut.cz/mailman/listinfo/coffee
On Wed, Aug 19 2020, Jiri Vlasak wrote:
+@app.route("/event/add", methods=["POST"])
POST always create new object by (REST) convention. Therefore, sending POST to endpoint "/event" is clearer.
Good point.
+def event_add():
- if request.method == "POST":
I think (but not sure) that this is unnecessary due to that only POST method is allowed (`methods=["POST"]`).
Yes, it's correct. I think we (somebody) should remove those unnecessary ifs also from the other functions.
+def add_event(uid, event_name, time):
- conn, c = open_db()
- c.execute("""insert into events (user_id, event_type, time)
values (?, (SELECT id FROM event_types WHERE name = ?), ?)""",
(uid, event_name, time))
- close_db(conn)
Different number of blank lines between functions looks ugly.
I unified the spacing in all files. This was not the only inconsistency in the number of blank lines.
Will send v2 soon.
-M.
Events block will probably be used more often, so this will result in less scrolling. --- templates/user.html | 68 ++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/templates/user.html b/templates/user.html index ed0156d..31b3d8e 100644 --- a/templates/user.html +++ b/templates/user.html @@ -21,6 +21,9 @@ } </style>
+{###############} +{# Graphs etc. #} +{###############} {% if name %} <form style="position: absolute; right: 15%; width: 15%; height: 15%;"> <button type="button" id="logout_button" onclick="logout()" style="width: 100%; height: 100%;">logout</button> @@ -66,34 +69,6 @@ </form> {% endif %} </p> - <br /> - <p> - <form> - <label for="username">Username:</label> - <input id="username" type="text" name="name"> - <input type="button" value="rename" onclick="renameUser()"> - </form> - </p> - <form> - <table style="padding: 2px"> - <tr> - <td colspan="4" align="center"><b>Identifiers:</b></td> - </tr> - {% for id in identifiers %} - <tr> - <td><input type="text" id="identifier_name_{{ loop.index }}" value="{{ id[2] }}" /></td> - <td {% if iid == id[1] %} style="font-weight: bold" {% endif %}>#<span id="identifier_{{ loop.index }}">{{ id[1] }}</span></td> - <td><input type="button" value="rename" onclick="renameIdentifier({{ loop.index }})" /></td> - <td><input type="button" value="remove" onclick="disableIdentifier('{{ id[1] }}')" /></td> - </tr> - {% endfor %} - <tr> - <td><input type="button" id="addIdentifier" value="add identifier" onclick="addIdentifier_start()" /></td> - <td colspan="2" style="visibility: hidden" id="labelIdentifier"><b>Use your identifier.</b></td> - <td><input type="button" id="abortIdentifier" value="abort" disabled onclick="addIdentifier_finish()" /></td> - </tr> - </table> - </form> {% else %} <p>Use your card/token to log in...</p>
@@ -101,6 +76,9 @@ <img src="{{ url_for('coffee_graph_flavors', _external=True, stamp=stamp, days=7) }}"> {% endif %}
+{##########} +{# Events #} +{##########} <br /> <form> <h3>{{ "Record event" if name else "Events" }}:</h3> @@ -139,3 +117,37 @@ {{ event_box('Milk container', ['MILK_CONTAINER_CLEANED', 'MILK_CONTAINER_PILL_CLEANED'] ) }} </div> </form> + +{##################################} +{# User name and chip idnetifiers #} +{##################################} +{% if name %} + <br /> + <p> + <form> + <label for="username">Username:</label> + <input id="username" type="text" name="name"> + <input type="button" value="rename" onclick="renameUser()"> + </form> + </p> + <form> + <table style="padding: 2px"> + <tr> + <td colspan="4" align="center"><b>Identifiers:</b></td> + </tr> + {% for id in identifiers %} + <tr> + <td><input type="text" id="identifier_name_{{ loop.index }}" value="{{ id[2] }}" /></td> + <td {% if iid == id[1] %} style="font-weight: bold" {% endif %}>#<span id="identifier_{{ loop.index }}">{{ id[1] }}</span></td> + <td><input type="button" value="rename" onclick="renameIdentifier({{ loop.index }})" /></td> + <td><input type="button" value="remove" onclick="disableIdentifier('{{ id[1] }}')" /></td> + </tr> + {% endfor %} + <tr> + <td><input type="button" id="addIdentifier" value="add identifier" onclick="addIdentifier_start()" /></td> + <td colspan="2" style="visibility: hidden" id="labelIdentifier"><b>Use your identifier.</b></td> + <td><input type="button" id="abortIdentifier" value="abort" disabled onclick="addIdentifier_finish()" /></td> + </tr> + </table> + </form> +{% endif %}