Fix reminder resend, settings validation, and notice navigation.
Reuse resolved reminder rows instead of inserting duplicates, skip failed keys in batch send, reject invalid scan settings with 400, and bind 去处理 via event delegation after async render. Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
总工
co-authored by
Cursor
multica-agent
Cursor
multica-agent
parent
f17636183d
commit
212b1f9b9b
@@ -3,8 +3,11 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import date, timedelta
|
||||
from pathlib import Path
|
||||
import json
|
||||
import sqlite3
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from bank_importer import auth, reminders
|
||||
from bank_importer.db import connect, migrate, utc_now
|
||||
@@ -187,6 +190,63 @@ class DeliveryAndDedupTests(ReminderTestCase):
|
||||
).fetchone()[0]
|
||||
self.assertEqual(2, events)
|
||||
|
||||
def test_resolved_reminder_reopens_same_row(self) -> None:
|
||||
self._insert_pending_account(self.company_a)
|
||||
finding = next(
|
||||
item
|
||||
for item in reminders.scan_findings(self.connection)
|
||||
if item.rule_key == reminders.RULE_PENDING and item.company_id == self.company_a
|
||||
)
|
||||
first = reminders.deliver_finding(self.connection, finding.dedupe_key, actor=self.admin)
|
||||
self.assertIsNotNone(first)
|
||||
reminders.update_reminder_status(
|
||||
self.connection, first, "resolved", company_id=self.company_a, actor=self.actor_a
|
||||
)
|
||||
second = reminders.deliver_finding(self.connection, finding.dedupe_key, actor=self.admin)
|
||||
self.assertEqual(first, second)
|
||||
row = self.connection.execute(
|
||||
"SELECT send_count, status, dedupe_key FROM reminders WHERE id = ?",
|
||||
(first,),
|
||||
).fetchone()
|
||||
self.assertEqual(2, row["send_count"])
|
||||
self.assertEqual("open", row["status"])
|
||||
count = self.connection.execute(
|
||||
"SELECT COUNT(*) FROM reminders WHERE dedupe_key = ?", (finding.dedupe_key,)
|
||||
).fetchone()[0]
|
||||
self.assertEqual(1, count)
|
||||
events = [
|
||||
item["event_type"]
|
||||
for item in self.connection.execute(
|
||||
"SELECT event_type FROM reminder_events WHERE reminder_id = ? ORDER BY id",
|
||||
(first,),
|
||||
).fetchall()
|
||||
]
|
||||
self.assertEqual(["sent", "resolved", "sent"], events)
|
||||
|
||||
def test_deliver_many_continues_after_one_failure(self) -> None:
|
||||
self._insert_pending_account(self.company_a)
|
||||
finding = next(
|
||||
item
|
||||
for item in reminders.scan_findings(self.connection)
|
||||
if item.rule_key == reminders.RULE_PENDING and item.company_id == self.company_a
|
||||
)
|
||||
original = reminders.deliver_finding
|
||||
|
||||
def flaky(connection, key, **kwargs):
|
||||
if key == "boom":
|
||||
raise sqlite3.IntegrityError("UNIQUE constraint failed: reminders.dedupe_key")
|
||||
return original(connection, key, **kwargs)
|
||||
|
||||
with patch.object(reminders, "deliver_finding", side_effect=flaky):
|
||||
sent = reminders.deliver_many(
|
||||
self.connection, ["boom", finding.dedupe_key], actor=self.admin
|
||||
)
|
||||
self.assertEqual(1, len(sent))
|
||||
self.assertEqual(
|
||||
1,
|
||||
self.connection.execute("SELECT COUNT(*) FROM reminders").fetchone()[0],
|
||||
)
|
||||
|
||||
def test_manual_reminder_each_send_is_separate(self) -> None:
|
||||
first = reminders.send_manual(
|
||||
self.connection,
|
||||
@@ -335,6 +395,79 @@ class SettingsTests(ReminderTestCase):
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"unknown_key": "1"})
|
||||
|
||||
def test_invalid_monthly_start_day_rejected(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"monthly_start_day": "0"})
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"monthly_start_day": "29"})
|
||||
self.assertEqual("5", reminders.get_settings(self.connection)["monthly_start_day"])
|
||||
|
||||
def test_invalid_gap_days_rejected(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"gap_days": "0"})
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"gap_days": "abc"})
|
||||
self.assertEqual("5", reminders.get_settings(self.connection)["gap_days"])
|
||||
|
||||
def test_invalid_scan_time_rejected(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"scan_time": "25:00"})
|
||||
with self.assertRaises(ValueError):
|
||||
reminders.update_settings(self.connection, {"scan_time": "8:0"})
|
||||
self.assertEqual("08:00", reminders.get_settings(self.connection)["scan_time"])
|
||||
|
||||
def test_corrupt_settings_do_not_break_scan(self) -> None:
|
||||
with self.connection:
|
||||
self.connection.execute(
|
||||
"UPDATE reminder_settings SET value = 'not-a-number' WHERE key = 'monthly_start_day'"
|
||||
)
|
||||
self.connection.execute(
|
||||
"UPDATE reminder_settings SET value = '0' WHERE key = 'gap_days'"
|
||||
)
|
||||
findings = reminders.scan_findings(self.connection)
|
||||
self.assertIsInstance(findings, list)
|
||||
|
||||
|
||||
class NoticeListDelegationTests(unittest.TestCase):
|
||||
def test_async_notice_go_handle_uses_event_delegation(self) -> None:
|
||||
source = (Path(__file__).resolve().parents[1] / "web" / "app.js").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
init_body = source.split("function initNotifications()", 1)[1].split("\nfunction ", 1)[0]
|
||||
self.assertIn('list.addEventListener("click"', init_body)
|
||||
self.assertIn('closest("[data-view-link]")', init_body)
|
||||
self.assertIn("showView(viewLink.dataset.viewLink)", init_body)
|
||||
self.assertIn('document.addEventListener("click"', source)
|
||||
self.assertNotIn(
|
||||
'$$("[data-view-link]").forEach((button) => button.addEventListener("click"',
|
||||
source,
|
||||
)
|
||||
|
||||
def test_delegated_lookup_finds_button_inserted_after_init(self) -> None:
|
||||
"""Simulate #notice-list after async replaceChildren: click target is the new button."""
|
||||
list_root = {"id": "notice-list", "parent": None, "attrs": {}}
|
||||
side = {"id": "lr-side", "parent": list_root, "attrs": {}}
|
||||
button = {
|
||||
"id": "go",
|
||||
"parent": side,
|
||||
"attrs": {"data-view-link": "reconcile"},
|
||||
}
|
||||
list_root["children"] = [side]
|
||||
side["children"] = [button]
|
||||
|
||||
def closest(node, attr):
|
||||
current = node
|
||||
while current is not None:
|
||||
if attr in current.get("attrs", {}):
|
||||
return current
|
||||
current = current.get("parent")
|
||||
return None
|
||||
|
||||
clicked = closest(button, "data-view-link")
|
||||
self.assertIsNotNone(clicked)
|
||||
self.assertEqual("reconcile", clicked["attrs"]["data-view-link"])
|
||||
self.assertIs(list_root, clicked["parent"]["parent"])
|
||||
|
||||
|
||||
class MigrationTests(unittest.TestCase):
|
||||
def test_v6_migration_applies_and_rolls_back(self) -> None:
|
||||
|
||||
@@ -287,6 +287,8 @@ class ServerAuthMatrixTests(unittest.TestCase):
|
||||
lambda: anon.get("/api/admin/users"),
|
||||
lambda: anon.get("/api/admin/companies"),
|
||||
lambda: anon.get("/api/admin/audit-log"),
|
||||
lambda: anon.get("/api/admin/reminders"),
|
||||
lambda: anon.get("/api/company/reminders"),
|
||||
lambda: anon.get("/api/me"),
|
||||
):
|
||||
status, _, data = method_check()
|
||||
@@ -387,6 +389,10 @@ class ServerAuthMatrixTests(unittest.TestCase):
|
||||
lambda: self.cashier_a.request("POST", "/api/admin/users/1/enable"),
|
||||
lambda: self.cashier_a.request("POST", "/api/admin/users/1/reset-password"),
|
||||
lambda: self.cashier_a.get("/api/admin/audit-log"),
|
||||
lambda: self.cashier_a.get("/api/admin/reminders"),
|
||||
lambda: self.cashier_a.get("/api/admin/reminders/pending"),
|
||||
lambda: self.cashier_a.post_json("/api/admin/reminder-settings", {"gap_days": "3"}),
|
||||
lambda: self.cashier_a.post_json("/api/admin/reminders/send", {"dedupe_keys": ["x"]}),
|
||||
)
|
||||
for call in calls:
|
||||
status, _, data = call()
|
||||
@@ -557,6 +563,28 @@ class ServerAuthMatrixTests(unittest.TestCase):
|
||||
self.assertNotIn(password, row["detail"] or "")
|
||||
self.assertNotIn(password, row["target"] or "")
|
||||
|
||||
def test_invalid_reminder_settings_return_400_and_do_not_persist(self) -> None:
|
||||
cases = (
|
||||
{"monthly_start_day": "0"},
|
||||
{"gap_days": "0"},
|
||||
{"scan_time": "25:99"},
|
||||
)
|
||||
for payload in cases:
|
||||
with self.subTest(payload=payload):
|
||||
status, _, data = self.admin.post_json(
|
||||
"/api/admin/reminder-settings", {"settings": payload}
|
||||
)
|
||||
self.assertEqual(400, status, data)
|
||||
self.assertEqual("error", as_json(data)["status"])
|
||||
status, _, data = self.admin.get("/api/admin/reminder-settings")
|
||||
self.assertEqual(200, status)
|
||||
settings = as_json(data)["settings"]
|
||||
self.assertEqual("5", settings["monthly_start_day"])
|
||||
self.assertEqual("5", settings["gap_days"])
|
||||
self.assertEqual("08:00", settings["scan_time"])
|
||||
status, _, data = self.admin.get("/api/admin/reminders/pending")
|
||||
self.assertEqual(200, status, data)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user