fix(advice): return matched_count, and do not advise about the wrong reading #20
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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"})
|
||||
|
||||
Reference in New Issue
Block a user