feat: enhance group duty pin command functionality
- Updated the `pin_duty_cmd` to handle cases where no message ID is found by sending a new duty message, pinning it, saving the pin, and scheduling the next update. - Improved error handling for message sending and pinning operations, providing appropriate replies based on success or failure. - Enhanced unit tests to cover the new behavior, ensuring proper functionality and error handling in various scenarios.
This commit is contained in:
@@ -246,7 +246,34 @@ async def pin_duty_cmd(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No
|
||||
loop = asyncio.get_running_loop()
|
||||
message_id = await loop.run_in_executor(None, _sync_get_message_id, chat_id)
|
||||
if message_id is None:
|
||||
await update.message.reply_text(t(lang, "pin_duty.no_message"))
|
||||
text = await loop.run_in_executor(
|
||||
None, lambda: _get_duty_message_text_sync(lang)
|
||||
)
|
||||
try:
|
||||
msg = await context.bot.send_message(chat_id=chat_id, text=text)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning(
|
||||
"Failed to send duty message for pin_duty chat_id=%s: %s", chat_id, e
|
||||
)
|
||||
await update.message.reply_text(t(lang, "pin_duty.failed"))
|
||||
return
|
||||
pinned = False
|
||||
try:
|
||||
await context.bot.pin_chat_message(
|
||||
chat_id=chat_id,
|
||||
message_id=msg.message_id,
|
||||
disable_notification=True,
|
||||
)
|
||||
pinned = True
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning("Failed to pin message for pin_duty chat_id=%s: %s", chat_id, e)
|
||||
await loop.run_in_executor(None, _sync_save_pin, chat_id, msg.message_id)
|
||||
next_end = await loop.run_in_executor(None, _get_next_shift_end_sync)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
if pinned:
|
||||
await update.message.reply_text(t(lang, "pin_duty.pinned"))
|
||||
else:
|
||||
await update.message.reply_text(t(lang, "pin_duty.could_not_pin_make_admin"))
|
||||
return
|
||||
try:
|
||||
await context.bot.pin_chat_message(
|
||||
|
||||
@@ -306,8 +306,8 @@ async def test_pin_duty_cmd_group_pins_and_replies_pinned():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pin_duty_cmd_no_message_id_replies_no_message():
|
||||
"""pin_duty_cmd: no pin record (_sync_get_message_id -> None) -> reply pin_duty.no_message."""
|
||||
async def test_pin_duty_cmd_no_message_id_creates_sends_pins_saves_schedules_replies_pinned():
|
||||
"""pin_duty_cmd: no pin record -> send_message, pin, save_pin, schedule, reply pinned."""
|
||||
update = MagicMock()
|
||||
update.message = MagicMock()
|
||||
update.message.reply_text = AsyncMock()
|
||||
@@ -316,14 +316,97 @@ async def test_pin_duty_cmd_no_message_id_replies_no_message():
|
||||
update.effective_chat.id = 100
|
||||
update.effective_user = MagicMock()
|
||||
context = MagicMock()
|
||||
context.bot = MagicMock()
|
||||
new_msg = MagicMock()
|
||||
new_msg.message_id = 42
|
||||
context.bot.send_message = AsyncMock(return_value=new_msg)
|
||||
context.bot.pin_chat_message = AsyncMock()
|
||||
context.application = MagicMock()
|
||||
context.application.job_queue = MagicMock()
|
||||
context.application.job_queue.get_jobs_by_name = MagicMock(return_value=[])
|
||||
context.application.job_queue.run_once = MagicMock()
|
||||
|
||||
with patch("duty_teller.handlers.group_duty_pin.get_lang", return_value="en"):
|
||||
with patch.object(mod, "_sync_get_message_id", return_value=None):
|
||||
with patch("duty_teller.handlers.group_duty_pin.t") as mock_t:
|
||||
mock_t.return_value = "No message to pin"
|
||||
await mod.pin_duty_cmd(update, context)
|
||||
update.message.reply_text.assert_called_once_with("No message to pin")
|
||||
mock_t.assert_called_with("en", "pin_duty.no_message")
|
||||
with patch.object(mod, "_get_duty_message_text_sync", return_value="Duty text"):
|
||||
with patch.object(mod, "_sync_save_pin") as mock_save:
|
||||
with patch.object(mod, "_get_next_shift_end_sync", return_value=None):
|
||||
with patch.object(mod, "_schedule_next_update", AsyncMock()):
|
||||
with patch("duty_teller.handlers.group_duty_pin.t") as mock_t:
|
||||
mock_t.return_value = "Pinned"
|
||||
await mod.pin_duty_cmd(update, context)
|
||||
context.bot.send_message.assert_called_once_with(chat_id=100, text="Duty text")
|
||||
context.bot.pin_chat_message.assert_called_once_with(
|
||||
chat_id=100, message_id=42, disable_notification=True
|
||||
)
|
||||
mock_save.assert_called_once_with(100, 42)
|
||||
update.message.reply_text.assert_called_once_with("Pinned")
|
||||
mock_t.assert_called_with("en", "pin_duty.pinned")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pin_duty_cmd_no_message_id_send_message_raises_replies_failed():
|
||||
"""pin_duty_cmd: no pin record, send_message raises BadRequest -> reply pin_duty.failed."""
|
||||
update = MagicMock()
|
||||
update.message = MagicMock()
|
||||
update.message.reply_text = AsyncMock()
|
||||
update.effective_chat = MagicMock()
|
||||
update.effective_chat.type = "group"
|
||||
update.effective_chat.id = 100
|
||||
update.effective_user = MagicMock()
|
||||
context = MagicMock()
|
||||
context.bot = MagicMock()
|
||||
context.bot.send_message = AsyncMock(side_effect=BadRequest("Chat not found"))
|
||||
context.application = MagicMock()
|
||||
|
||||
with patch("duty_teller.handlers.group_duty_pin.get_lang", return_value="en"):
|
||||
with patch.object(mod, "_sync_get_message_id", return_value=None):
|
||||
with patch.object(mod, "_get_duty_message_text_sync", return_value="Duty"):
|
||||
with patch.object(mod, "_sync_save_pin") as mock_save:
|
||||
with patch.object(mod, "_schedule_next_update", AsyncMock()) as mock_schedule:
|
||||
with patch("duty_teller.handlers.group_duty_pin.t") as mock_t:
|
||||
mock_t.return_value = "Failed"
|
||||
await mod.pin_duty_cmd(update, context)
|
||||
update.message.reply_text.assert_called_once_with("Failed")
|
||||
mock_t.assert_called_with("en", "pin_duty.failed")
|
||||
mock_save.assert_not_called()
|
||||
mock_schedule.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pin_duty_cmd_no_message_id_pin_raises_saves_and_replies_could_not_pin():
|
||||
"""pin_duty_cmd: no pin record, pin_chat_message raises -> save pin, reply could_not_pin_make_admin."""
|
||||
update = MagicMock()
|
||||
update.message = MagicMock()
|
||||
update.message.reply_text = AsyncMock()
|
||||
update.effective_chat = MagicMock()
|
||||
update.effective_chat.type = "group"
|
||||
update.effective_chat.id = 100
|
||||
update.effective_user = MagicMock()
|
||||
context = MagicMock()
|
||||
context.bot = MagicMock()
|
||||
new_msg = MagicMock()
|
||||
new_msg.message_id = 43
|
||||
context.bot.send_message = AsyncMock(return_value=new_msg)
|
||||
context.bot.pin_chat_message = AsyncMock(side_effect=Forbidden("Not enough rights"))
|
||||
context.application = MagicMock()
|
||||
context.application.job_queue = MagicMock()
|
||||
context.application.job_queue.get_jobs_by_name = MagicMock(return_value=[])
|
||||
context.application.job_queue.run_once = MagicMock()
|
||||
|
||||
with patch("duty_teller.handlers.group_duty_pin.get_lang", return_value="en"):
|
||||
with patch.object(mod, "_sync_get_message_id", return_value=None):
|
||||
with patch.object(mod, "_get_duty_message_text_sync", return_value="Duty"):
|
||||
with patch.object(mod, "_sync_save_pin") as mock_save:
|
||||
with patch.object(mod, "_get_next_shift_end_sync", return_value=None):
|
||||
with patch.object(mod, "_schedule_next_update", AsyncMock()):
|
||||
with patch("duty_teller.handlers.group_duty_pin.t") as mock_t:
|
||||
mock_t.return_value = "Make me admin to pin"
|
||||
await mod.pin_duty_cmd(update, context)
|
||||
context.bot.send_message.assert_called_once_with(chat_id=100, text="Duty")
|
||||
mock_save.assert_called_once_with(100, 43)
|
||||
update.message.reply_text.assert_called_once_with("Make me admin to pin")
|
||||
mock_t.assert_called_with("en", "pin_duty.could_not_pin_make_admin")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -52,7 +52,9 @@ export function buildDayDetailContent(dateKey, duties, eventSummaries) {
|
||||
? t(lang, "duty.today") + ", " + ddmm
|
||||
: ddmm;
|
||||
|
||||
const dutyList = (duties || []).filter((d) => d.event_type === "duty");
|
||||
const dutyList = (duties || [])
|
||||
.filter((d) => d.event_type === "duty")
|
||||
.sort((a, b) => new Date(a.start_at || 0) - new Date(b.start_at || 0));
|
||||
const unavailableList = (duties || []).filter((d) => d.event_type === "unavailable");
|
||||
const vacationList = (duties || []).filter((d) => d.event_type === "vacation");
|
||||
const summaries = eventSummaries || [];
|
||||
|
||||
41
webapp/js/dayDetail.test.js
Normal file
41
webapp/js/dayDetail.test.js
Normal file
@@ -0,0 +1,41 @@
|
||||
/**
|
||||
* Unit tests for buildDayDetailContent.
|
||||
* Verifies dutyList is sorted by start_at before display.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll } from "vitest";
|
||||
import { buildDayDetailContent } from "./dayDetail.js";
|
||||
|
||||
describe("buildDayDetailContent", () => {
|
||||
beforeAll(() => {
|
||||
document.body.innerHTML =
|
||||
'<div id="calendar"></div><div id="monthTitle"></div>' +
|
||||
'<div id="dutyList"></div><div id="loading"></div><div id="error"></div>' +
|
||||
'<div id="accessDenied"></div><div class="header"></div><div class="weekdays"></div>' +
|
||||
'<button id="prevMonth"></button><button id="nextMonth"></button>';
|
||||
});
|
||||
|
||||
it("sorts duty list by start_at when input order is wrong", () => {
|
||||
const dateKey = "2025-02-25";
|
||||
const duties = [
|
||||
{
|
||||
event_type: "duty",
|
||||
full_name: "Петров",
|
||||
start_at: "2025-02-25T14:00:00",
|
||||
end_at: "2025-02-25T18:00:00",
|
||||
},
|
||||
{
|
||||
event_type: "duty",
|
||||
full_name: "Иванов",
|
||||
start_at: "2025-02-25T09:00:00",
|
||||
end_at: "2025-02-25T14:00:00",
|
||||
},
|
||||
];
|
||||
const html = buildDayDetailContent(dateKey, duties, []);
|
||||
expect(html).toContain("Иванов");
|
||||
expect(html).toContain("Петров");
|
||||
const ivanovPos = html.indexOf("Иванов");
|
||||
const petrovPos = html.indexOf("Петров");
|
||||
expect(ivanovPos).toBeLessThan(petrovPos);
|
||||
});
|
||||
});
|
||||
@@ -115,7 +115,14 @@ function buildDutyItemTimePrefix(item, idx, total, hintDay, sep, fromLabel, toLa
|
||||
if (endSameDay && endHHMM && endHHMM !== startHHMM) {
|
||||
timePrefix += " " + toLabel + sep + endHHMM;
|
||||
}
|
||||
} else if (startSameDay && startHHMM) {
|
||||
/* First of multiple, but starts today — show full range */
|
||||
timePrefix = fromLabel + sep + startHHMM;
|
||||
if (endSameDay && endHHMM && endHHMM !== startHHMM) {
|
||||
timePrefix += " " + toLabel + sep + endHHMM;
|
||||
}
|
||||
} else if (endHHMM) {
|
||||
/* Continuation from previous day — only end time */
|
||||
timePrefix = toLabel + sep + endHHMM;
|
||||
}
|
||||
} else if (idx > 0) {
|
||||
|
||||
99
webapp/js/hints.test.js
Normal file
99
webapp/js/hints.test.js
Normal file
@@ -0,0 +1,99 @@
|
||||
/**
|
||||
* Unit tests for getDutyMarkerRows and buildDutyItemTimePrefix logic.
|
||||
* Covers: sorting order preservation, idx=0 with total>1 and startSameDay.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll } from "vitest";
|
||||
import { getDutyMarkerRows } from "./hints.js";
|
||||
|
||||
const FROM = "from";
|
||||
const TO = "until";
|
||||
const SEP = "\u00a0";
|
||||
|
||||
describe("getDutyMarkerRows", () => {
|
||||
beforeAll(() => {
|
||||
document.body.innerHTML = '<div id="calendar"></div>';
|
||||
});
|
||||
|
||||
it("preserves input order (caller must sort by start_at before passing)", () => {
|
||||
const hintDay = "2025-02-25";
|
||||
const duties = [
|
||||
{
|
||||
full_name: "Иванов",
|
||||
start_at: "2025-02-25T14:00:00",
|
||||
end_at: "2025-02-25T18:00:00",
|
||||
},
|
||||
{
|
||||
full_name: "Петров",
|
||||
start_at: "2025-02-25T09:00:00",
|
||||
end_at: "2025-02-25T14:00:00",
|
||||
},
|
||||
];
|
||||
const rows = getDutyMarkerRows(duties, hintDay, SEP, FROM, TO);
|
||||
expect(rows).toHaveLength(2);
|
||||
expect(rows[0].fullName).toBe("Иванов");
|
||||
expect(rows[1].fullName).toBe("Петров");
|
||||
});
|
||||
|
||||
it("first of multiple with startSameDay shows full range (from HH:MM to HH:MM)", () => {
|
||||
const hintDay = "2025-02-25";
|
||||
const duties = [
|
||||
{
|
||||
full_name: "Иванов",
|
||||
start_at: "2025-02-25T09:00:00",
|
||||
end_at: "2025-02-25T14:00:00",
|
||||
},
|
||||
{
|
||||
full_name: "Петров",
|
||||
start_at: "2025-02-25T14:00:00",
|
||||
end_at: "2025-02-25T18:00:00",
|
||||
},
|
||||
].sort((a, b) => new Date(a.start_at) - new Date(b.start_at));
|
||||
|
||||
const rows = getDutyMarkerRows(duties, hintDay, SEP, FROM, TO);
|
||||
expect(rows).toHaveLength(2);
|
||||
expect(rows[0].fullName).toBe("Иванов");
|
||||
expect(rows[0].timePrefix).toContain("09:00");
|
||||
expect(rows[0].timePrefix).toContain("14:00");
|
||||
expect(rows[0].timePrefix).toContain(FROM);
|
||||
expect(rows[0].timePrefix).toContain(TO);
|
||||
});
|
||||
|
||||
it("first of multiple continuation from previous day shows only end time", () => {
|
||||
const hintDay = "2025-02-25";
|
||||
const duties = [
|
||||
{
|
||||
full_name: "Иванов",
|
||||
start_at: "2025-02-24T22:00:00",
|
||||
end_at: "2025-02-25T06:00:00",
|
||||
},
|
||||
{
|
||||
full_name: "Петров",
|
||||
start_at: "2025-02-25T09:00:00",
|
||||
end_at: "2025-02-25T14:00:00",
|
||||
},
|
||||
].sort((a, b) => new Date(a.start_at) - new Date(b.start_at));
|
||||
|
||||
const rows = getDutyMarkerRows(duties, hintDay, SEP, FROM, TO);
|
||||
expect(rows).toHaveLength(2);
|
||||
expect(rows[0].fullName).toBe("Иванов");
|
||||
expect(rows[0].timePrefix).not.toContain(FROM);
|
||||
expect(rows[0].timePrefix).toContain(TO);
|
||||
expect(rows[0].timePrefix).toContain("06:00");
|
||||
});
|
||||
|
||||
it("multiple duties in one day — correct order when input is pre-sorted", () => {
|
||||
const hintDay = "2025-02-25";
|
||||
const duties = [
|
||||
{ full_name: "A", start_at: "2025-02-25T09:00:00", end_at: "2025-02-25T12:00:00" },
|
||||
{ full_name: "B", start_at: "2025-02-25T12:00:00", end_at: "2025-02-25T15:00:00" },
|
||||
{ full_name: "C", start_at: "2025-02-25T15:00:00", end_at: "2025-02-25T18:00:00" },
|
||||
].sort((a, b) => new Date(a.start_at) - new Date(b.start_at));
|
||||
|
||||
const rows = getDutyMarkerRows(duties, hintDay, SEP, FROM, TO);
|
||||
expect(rows.map((r) => r.fullName)).toEqual(["A", "B", "C"]);
|
||||
expect(rows[0].timePrefix).toContain("09:00");
|
||||
expect(rows[1].timePrefix).toContain("12:00");
|
||||
expect(rows[2].timePrefix).toContain("15:00");
|
||||
});
|
||||
});
|
||||
12
webapp/package.json
Normal file
12
webapp/package.json
Normal file
@@ -0,0 +1,12 @@
|
||||
{
|
||||
"name": "duty-teller-webapp",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"test": "vitest run",
|
||||
"test:watch": "vitest"
|
||||
},
|
||||
"devDependencies": {
|
||||
"happy-dom": "^15.0.0",
|
||||
"vitest": "^2.0.0"
|
||||
}
|
||||
}
|
||||
6
webapp/vitest.config.js
Normal file
6
webapp/vitest.config.js
Normal file
@@ -0,0 +1,6 @@
|
||||
export default {
|
||||
test: {
|
||||
environment: "happy-dom",
|
||||
include: ["js/**/*.test.js"],
|
||||
},
|
||||
};
|
||||
Reference in New Issue
Block a user