mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-15 17:25:26 -04:00
fix(calendar): parse "mins"/"hrs" reminder offsets in manage_calendar (#4266)
_reminder_minutes matched the offset with (?:m|min|minute|minutes)\b and (?:h|hr|hour|hours)\b. The trailing \b makes the common plural abbreviations "mins"/"hrs" fail to match (after "min" the "s" is a word char, so no boundary), so reminder_minutes "5 mins" or "2 hrs" returned None and the event was created with no reminder, silently. Widen the two unit regexes and the matching reminder_only description regex to a strict superset that also accepts mins/hrs. The sibling duration parser already accepts these forms (it has no \b), so this only brings the reminder parser in line.
This commit is contained in:
@@ -1579,10 +1579,10 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict:
|
||||
text = str(raw).strip().lower()
|
||||
if text in {"none", "no", "off", "false"}:
|
||||
return None
|
||||
m = re.search(r"(\d+)\s*(?:m|min|minute|minutes)\b", text)
|
||||
m = re.search(r"(\d+)\s*(?:minutes?|mins?|m)\b", text)
|
||||
if m:
|
||||
return max(0, int(m.group(1)))
|
||||
m = re.search(r"(\d+)\s*(?:h|hr|hour|hours)\b", text)
|
||||
m = re.search(r"(\d+)\s*(?:hours?|hrs?|h)\b", text)
|
||||
if m:
|
||||
return max(0, int(m.group(1)) * 60)
|
||||
if text.isdigit():
|
||||
@@ -1595,7 +1595,7 @@ async def do_manage_calendar(content: str, owner: Optional[str] = None) -> Dict:
|
||||
return desc
|
||||
reminder_only = re.compile(
|
||||
r"^\s*(?:remind(?:er)?|alarm)\s*:?\s*\d+\s*"
|
||||
r"(?:m|min|minute|minutes|h|hr|hour|hours)\b.*$",
|
||||
r"(?:minutes?|mins?|m|hours?|hrs?|h)\b.*$",
|
||||
re.I,
|
||||
)
|
||||
return "" if reminder_only.match(desc) else desc
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
"""do_manage_calendar must honour abbreviated reminder phrasings like "mins"/"hrs".
|
||||
|
||||
`_reminder_minutes` parsed the reminder offset with regexes anchored on
|
||||
`(?:m|min|minute|minutes)\b` / `(?:h|hr|hour|hours)\b`. The trailing `\b`
|
||||
made the very common plural abbreviations "mins" and "hrs" fail to match
|
||||
(after "min" the next char "s" is a word char, so no boundary), so a request
|
||||
like ``reminder_minutes: "5 mins"`` silently produced no reminder at all —
|
||||
even though the sibling duration parser (no `\b`) already accepted them.
|
||||
"""
|
||||
|
||||
import json
|
||||
import sys
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.helpers.import_state import clear_fake_database_modules
|
||||
from tests.helpers.sqlite_db import make_temp_sqlite
|
||||
|
||||
clear_fake_database_modules()
|
||||
|
||||
import core.database as cdb
|
||||
from core.database import Note
|
||||
|
||||
_TS, _ENGINE, _TMPDB = make_temp_sqlite(cdb.Base.metadata)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _bind_temp_db(monkeypatch):
|
||||
monkeypatch.setitem(sys.modules, "core.database", cdb)
|
||||
parent = sys.modules.get("core")
|
||||
if parent is not None:
|
||||
monkeypatch.setattr(parent, "database", cdb, raising=False)
|
||||
monkeypatch.setattr(cdb, "SessionLocal", _TS)
|
||||
yield
|
||||
|
||||
|
||||
async def _create_with_reminder(reminder, owner):
|
||||
from src.tool_implementations import do_manage_calendar
|
||||
|
||||
payload = {
|
||||
"action": "create_event",
|
||||
"summary": "Dentist",
|
||||
# Far-future so the reminder is never "already passed".
|
||||
"dtstart": "2030-01-01T10:00:00",
|
||||
"reminder_minutes": reminder,
|
||||
}
|
||||
return await do_manage_calendar(json.dumps(payload), owner=owner)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("reminder,expected", [
|
||||
("5 mins", 5),
|
||||
("10 mins", 10),
|
||||
("2 hrs", 120),
|
||||
("1 hr", 60),
|
||||
("15 minutes", 15), # regression: long form still works
|
||||
("30m", 30), # regression: bare unit still works
|
||||
])
|
||||
async def test_reminder_minutes_accepts_abbreviations(reminder, expected):
|
||||
owner = "tester-" + uuid.uuid4().hex[:6]
|
||||
res = await _create_with_reminder(reminder, owner)
|
||||
assert res.get("exit_code") == 0, res
|
||||
assert f"reminder {expected} min before" in res.get("response", ""), res
|
||||
|
||||
db = _TS()
|
||||
try:
|
||||
note = (
|
||||
db.query(Note)
|
||||
.filter(Note.owner == owner, Note.title == "Reminder: Dentist")
|
||||
.first()
|
||||
)
|
||||
assert note is not None, "reminder note should have been created"
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
async def test_no_reminder_when_offset_absent():
|
||||
owner = "tester-" + uuid.uuid4().hex[:6]
|
||||
from src.tool_implementations import do_manage_calendar
|
||||
|
||||
payload = {
|
||||
"action": "create_event",
|
||||
"summary": "No Reminder Event",
|
||||
"dtstart": "2030-02-01T10:00:00",
|
||||
}
|
||||
res = await do_manage_calendar(json.dumps(payload), owner=owner)
|
||||
assert res.get("exit_code") == 0, res
|
||||
assert "reminder set" not in res.get("response", ""), res
|
||||
Reference in New Issue
Block a user