fix(advice): return matched_count, and do not advise about the wrong reading #20

Merged
hermes merged 1 commits from fix/advice-write-guards into master 2026-11-09 22:16:47 +10:00
3 changed files with 58 additions and 2 deletions
+7 -1
View File
@@ -296,7 +296,13 @@ class BeerRepository:
"advice_at": _now_iso(),
}},
)
return result.modified_count
# `matched_count`, not `modified_count`.
#
# `modified_count` is 0 when the document matched but the text
# was already identical - which is a successful write, not a
# refused one. The caller logs "the advice was not stored" when
# this is 0, so it has to mean "the guard refused it".
return result.matched_count
def set_final_reading(self, beer_run_id: str, reading_date: str,
final: bool) -> int:
+9 -1
View File
@@ -106,7 +106,15 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
# wrong number.
saved = next((r for r in readings if r.get("_id") == str(reading_id)), None)
if saved is None:
saved = dict(readings[-1]) if readings else {"date": "", "sg": None}
# The reading is not in the run. Advising about the newest
# reading instead would describe the wrong number, and the
# conditional write would then refuse it anyway - so it is
# better to say so plainly and stop.
log.info(
"no advice for reading %s: it is not one of the run's readings",
reading_id,
)
return
latest = dict(saved)
latest["_id"] = reading_id
+42
View File
@@ -1079,6 +1079,48 @@ def test_the_long_and_short_labels_are_both_in_the_page(client, repo):
assert '<span class="short">Advice</span>' in body
def test_stale_advice_cannot_overwrite_a_comment_only_correction(repo):
"""A comment-only correction still invalidates the old advice.
The gravity alone is not enough to tell two generations apart - a
comment-only correction leaves the gravity unchanged. The editing
token is what tells them apart, because every edit writes a fresh
one.
"""
repo.readings.insert_one({
"_id": "readingone",
"beer_run_id": "1",
"date": "2026-01-01",
"sg": "1040",
"comment": "original comment",
"final_reading": "False",
"advice_token": "token-one",
})
# The slow thread holds the token it was given.
stale_token = "token-one"
# The brewer corrects only the comment.
repo.readings.update_one(
{"_id": "readingone"},
{"$set": {"comment": "corrected comment", "advice_token": "token-two"},
"$unset": {"advice": "", "advice_ok": "", "advice_at": ""}},
)
# The fresh advice lands first, carrying the new token.
assert repo.set_reading_advice(
"readingone", "FRESH ADVICE", ok=True, expected_token="token-two") == 1
# The slow thread lands afterwards, carrying the old token. It must
# be refused.
assert repo.set_reading_advice(
"readingone", "STALE ADVICE", ok=True, expected_token=stale_token) == 0
doc = repo.readings.find_one({"_id": "readingone"})
assert doc["advice"] == "FRESH ADVICE"
assert doc["comment"] == "corrected comment"
def test_a_failed_advice_is_marked_as_such(client, repo):
"""A generation failure reads differently from real advice."""
repo.users.insert_one({"username": "beer", "password": "beer_pass"})