Refactor telegram bot workflow and admin tools - #1
Open
soloveyska1 wants to merge 9 commits into
Open
Conversation
Руководство для ревьюераЭтот PR полностью перерабатывает Telegram-бота, выделяя сохранение данных в отдельный класс DataStore, вводя класс данных OrderRecord, реорганизуя обработчики в дискретные функции на основе состояний с единым диспетчером обратных вызовов, стандартизируя пользовательские интерфейсы с помощью вспомогательных функций для клавиатур и режимов синтаксического анализа HTML, обогащая панель администратора статистикой, экспортом в Excel и элементами управления режимами ценообразования, а также соответствующим образом обновляя документацию и зависимости. Диаграмма последовательности для управляемого процесса создания заказаsequenceDiagram
actor User
participant Bot
participant DataStore
User->>Bot: Start order (select type)
Bot->>User: Show order types
User->>Bot: Select type
Bot->>User: Prompt for topic
User->>Bot: Send topic
Bot->>User: Prompt for deadline
User->>Bot: Select deadline
Bot->>User: Prompt for requirements
User->>Bot: Send requirements or /skip
Bot->>User: Show upsell options
User->>Bot: Select upsell(s)
Bot->>User: Show order summary
User->>Bot: Confirm order
Bot->>DataStore: add_order(user_id, OrderRecord)
DataStore-->>Bot: Order saved
Bot->>User: Confirmation and next steps
Диаграмма классов для новых типов DataStore и OrderRecordclassDiagram
class DataStore {
+settings: Dict[str, str]
+prices: Dict[str, Dict[str, int]]
+referrals: Dict[str, List[int]]
+orders: Dict[str, List[Dict[str, object]]]
+feedbacks: Dict[str, List[str]]
+user_logs: Dict[str, List[Dict[str, str]]]
+get_pricing_mode() str
+set_pricing_mode(mode: str)
+add_referral(referrer_id: int, new_user_id: int) bool
+log_action(user_id: int, username: Optional[str], action: str)
+add_feedback(user_id: int, feedback: str)
+next_order_id(user_id: int) int
+add_order(user_id: int, order: OrderRecord) OrderRecord
+get_orders(user_id: int) List[Dict[str, object]]
+total_spent(user_id: int) int
+get_referrals(user_id: int) List[int]
+export_orders() Optional[Path]
+get_statistics() Dict[str, int]
}
class OrderRecord {
+order_id: int
+type_key: str
+topic: str
+deadline_days: int
+deadline_date: str
+requirements: str
+upsells: List[str]
+price: int
+status: str
+created_at: str
}
DataStore --> OrderRecord : uses
Изменения на уровне файлов
Советы и командыВзаимодействие с Sourcery
Настройка вашего опытаПолучите доступ к вашей панели управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideThis PR overhauls the Telegram bot by extracting data persistence into a dedicated DataStore class, introducing an OrderRecord dataclass, reorganizing handlers into discrete state-based functions with a single callback dispatcher, standardizing UI flows via helper functions for keyboards and HTML parse modes, enriching the admin panel with statistics, Excel export and pricing-mode controls, and refreshing documentation and dependencies accordingly. Sequence diagram for guided order creation flowsequenceDiagram
actor User
participant Bot
participant DataStore
User->>Bot: Start order (select type)
Bot->>User: Show order types
User->>Bot: Select type
Bot->>User: Prompt for topic
User->>Bot: Send topic
Bot->>User: Prompt for deadline
User->>Bot: Select deadline
Bot->>User: Prompt for requirements
User->>Bot: Send requirements or /skip
Bot->>User: Show upsell options
User->>Bot: Select upsell(s)
Bot->>User: Show order summary
User->>Bot: Confirm order
Bot->>DataStore: add_order(user_id, OrderRecord)
DataStore-->>Bot: Order saved
Bot->>User: Confirmation and next steps
Class diagram for new DataStore and OrderRecord typesclassDiagram
class DataStore {
+settings: Dict[str, str]
+prices: Dict[str, Dict[str, int]]
+referrals: Dict[str, List[int]]
+orders: Dict[str, List[Dict[str, object]]]
+feedbacks: Dict[str, List[str]]
+user_logs: Dict[str, List[Dict[str, str]]]
+get_pricing_mode() str
+set_pricing_mode(mode: str)
+add_referral(referrer_id: int, new_user_id: int) bool
+log_action(user_id: int, username: Optional[str], action: str)
+add_feedback(user_id: int, feedback: str)
+next_order_id(user_id: int) int
+add_order(user_id: int, order: OrderRecord) OrderRecord
+get_orders(user_id: int) List[Dict[str, object]]
+total_spent(user_id: int) int
+get_referrals(user_id: int) List[int]
+export_orders() Optional[Path]
+get_statistics() Dict[str, int]
}
class OrderRecord {
+order_id: int
+type_key: str
+topic: str
+deadline_days: int
+deadline_date: str
+requirements: str
+upsells: List[str]
+price: int
+status: str
+created_at: str
}
DataStore --> OrderRecord : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Привет! Я просмотрел твои изменения — вот несколько замечаний:
- Функция handle_callback очень большая и имеет много ветвлений — рассмотри возможность ее рефакторинга в таблицу диспетчеризации или отдельные функции-обработчики для улучшения читаемости и удобства поддержки.
- Использование единого STATE_NAVIGATION почти для всех потоков обратных вызовов усложняет настройку и отладку ConversationHandler; определи более детальные состояния, чтобы четко разделить различные шаги диалога.
- DataStore._save_json записывает данные непосредственно на диск при каждом обновлении без обработки ошибок — оберни операции записи файлов в try/except или пакетно их обрабатывай, чтобы предотвратить неожиданные сбои и улучшить производительность.
Запрос для AI-агентов
Пожалуйста, учти комментарии из этого код-ревью:
## Общие комментарии
- Функция handle_callback очень большая и имеет много ветвлений — рассмотри возможность ее рефакторинга в таблицу диспетчеризации или отдельные функции-обработчики для улучшения читаемости и удобства поддержки.
- Использование единого STATE_NAVIGATION почти для всех потоков обратных вызовов усложняет настройку и отладку ConversationHandler; определи более детальные состояния, чтобы четко разделить различные шаги диалога.
- DataStore._save_json записывает данные непосредственно на диск при каждом обновлении без обработки ошибок — оберни операции записи файлов в try/except или пакетно их обрабатывай, чтобы предотвратить неожиданные сбои и улучшить производительность.
## Индивидуальные комментарии
### Comment 1
<location> `bot.py:398` </location>
<code_context>
+
+def get_order_draft(context: ContextTypes.DEFAULT_TYPE) -> Dict[str, object]:
+ draft = context.user_data.setdefault("order_draft", {})
+ draft.setdefault("upsells", set())
+ return draft
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Storing sets in user_data may cause serialization issues.
Since sets are not JSON serializable, using them in user_data can break persistence. Use a list for 'upsells' to avoid serialization errors.
</issue_to_address>
### Comment 2
<location> `bot.py:519` </location>
<code_context>
+ return InlineKeyboardMarkup(keyboard)
+
+
+async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ draft = get_order_draft(context)
+ raw_selected = draft.get("upsells", set())
</code_context>
<issue_to_address>
**suggestion:** The upsell selection logic may not handle type consistency for 'upsells'.
Handling both sets and lists for 'upsells' increases complexity and risk of bugs. Standardizing on a list would make the logic clearer and safer.
Suggested implementation:
```python
async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
draft = get_order_draft(context)
raw_selected = draft.get("upsells", [])
selected_list = list(raw_selected) if isinstance(raw_selected, (set, list)) else []
draft["upsells"] = selected_list
if update.message:
await update.message.reply_text(
"Хотите добавить дополнительные материалы? Презентация или речь экономят время на подготовке!",
reply_markup=build_upsell_keyboard(selected_list),
)
else:
query = update.callback_query
await query.edit_message_text(
```
You should also review any other code that reads or writes to `draft["upsells"]` elsewhere in the codebase to ensure it always expects and uses a list, not a set.
If `build_upsell_keyboard` expects a set, update it to accept a list and convert it to a set internally if needed.
</issue_to_address>
### Comment 3
<location> `bot.py:584` </location>
<code_context>
+ return await show_main_menu(update, context)
+
+
+async def confirm_order(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ user = update.effective_user
+ if not user:
</code_context>
<issue_to_address>
**question (bug_risk):** No discount is applied for multiple orders in the new flow.
If the removal of multi-order discounts is intentional, clarify this in the UI. Otherwise, restore the previous discount logic for multiple orders.
</issue_to_address>
### Comment 4
<location> `bot.py:973` </location>
<code_context>
+ await update.message.reply_text("Команда не распознана. Используйте кнопки в меню.")
+ return STATE_ADMIN
+
+async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ query = update.callback_query
+ await query.answer()
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the long if/elif chain in handle_callback with a dispatch table for callback handlers to simplify the code.
You’ve done an excellent job extracting all of the “building blocks” (keyboards, formatting, datastore) into small focused functions and classes. The only remaining hot-spot of complexity is the giant `handle_callback` if/elif chain. You can collapse it into a simple dispatch table—no functionality changes, but you immediately remove dozens of lines of branching. For example:
```python
# at module‐top, build a mapping from your callback_data → handler
CALLBACK_HANDLERS: dict[str, Callable[..., Awaitable[int]]] = {
"main:root": show_main_menu,
"main:order": show_order_types,
"order:list": show_order_types,
# …
"order:type": show_order_details, # handles order:type:<key>
"order:new": prompt_order_topic, # handles order:new:<key>
# …
"order:confirm": confirm_order,
"order:cancel": cancel_order,
# …
}
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
await update.callback_query.answer()
data = update.callback_query.data # e.g. "order:type:course_empirical"
parts = data.split(":", maxsplit=2)
cmd = ":".join(parts[:2]) # e.g. "order:type"
arg = parts[2] if len(parts) == 3 else None
handler = CALLBACK_HANDLERS.get(cmd)
if handler:
# if your handler needs the extra arg, pass it along
return await handler(update, context, arg) if arg else await handler(update, context)
# fallback
await update.callback_query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)
```
Steps to apply:
1. Create the `CALLBACK_HANDLERS` dict at the top of your module.
2. Convert each `if data.startswith("…")` or `if data == "…"` into a single key in that dict (you can still match prefixes using the first two segments).
3. Replace the big `if/elif` block in `handle_callback` with the generic lookup above.
This trivially collapses ~50 lines of branching into a handful of lines, keeps all existing behavior, and makes adding new callbacks a one-liner.
</issue_to_address>
### Comment 5
<location> `bot.py:272` </location>
<code_context>
active_orders = sum(1 for order in orders_flat if order.get("status", "").lower() not in {"готов", "завершен"})
</code_context>
<issue_to_address>
**suggestion (code-quality):** Simplify constant sum() call ([`simplify-constant-sum`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/simplify-constant-sum))
```suggestion
active_orders = sum(bool(order.get("status", "").lower() not in {"готов", "завершен"})
```
<br/><details><summary>Объяснение</summary>Поскольку `sum` складывает значения, он обрабатывает `True` как `1`, а `False` как `0`. Мы используем
этот факт для упрощения генераторного выражения внутри вызова `sum`.
</details>
</issue_to_address>
### Comment 6
<location> `bot.py:1053-1061` </location>
<code_context>
if referrer_id != user.id and store.add_referral(referrer_id, user.id):
if ADMIN_CHAT_ID:
try:
await context.bot.send_message(
ADMIN_CHAT_ID,
f"Новый реферал: {user.id} (пригласил {referrer_id})",
)
except Exception as exc: # pragma: no cover
logger.error("Failed to notify admin about referral: %s", exc)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if referrer_id != user.id and store.add_referral(referrer_id, user.id) and ADMIN_CHAT_ID:
try:
await context.bot.send_message(
ADMIN_CHAT_ID,
f"Новый реферал: {user.id} (пригласил {referrer_id})",
)
except Exception as exc: # pragma: no cover
logger.error("Failed to notify admin about referral: %s", exc)
```
<br/><details><summary>Объяснение</summary>Слишком большая вложенность может затруднить понимание кода, и это особенно
актуально для Python, где нет скобок, помогающих разграничить
различные уровни вложенности.
Чтение глубоко вложенного кода сбивает с толку, поскольку приходится отслеживать, какие
условия относятся к каким уровням. Поэтому мы стремимся уменьшить вложенность там,
где это возможно, и ситуация, когда два условия `if` могут быть объединены с помощью
`and`, является легкой победой.
</details>
</issue_to_address>
### Comment 7
<location> `bot.py:259-260` </location>
<code_context>
def export_orders(self) -> Optional[Path]:
records: List[Dict[str, object]] = []
for user_id, orders in self.orders.items():
for order in orders:
records.append({"user_id": user_id, **order})
if not records:
return None
df = pd.DataFrame(records)
export_path = DATA_DIR / "orders_export.xlsx"
df.to_excel(export_path, index=False)
return export_path
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace a for append loop with list extend ([`for-append-to-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/for-append-to-extend/))
```suggestion
records.extend({"user_id": user_id, **order} for order in orders)
```
</issue_to_address>
### Comment 8
<location> `bot.py:288-290` </location>
<code_context>
def log_user_action(update: Update, action: str) -> None:
user = update.effective_user
if not user:
return
store.log_action(user.id, user.username, action)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 9
<location> `bot.py:301` </location>
<code_context>
def calculate_price(order_type: str, days_left: int, complexity: float = 1.0, upsells: Iterable[str] = ()) -> int:
price_info = store.prices.get(order_type) or DEFAULT_PRICES.get(order_type)
if not price_info:
logger.warning("Unknown order type for pricing: %s", order_type)
return 0
price = int(price_info.get("base", 0) * complexity)
mode = store.get_pricing_mode()
if mode == "hard":
if days_left < 7:
price = int(price * 1.3)
elif days_left < 15:
price = int(price * 1.15)
else:
if days_left < 3:
price = int(price * 1.3)
elif days_left < 7:
price = int(price * 1.15)
for upsell in upsells:
option = UPSELL_OPTIONS.get(upsell)
if option:
price += option["price"]
return price
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Merge duplicate blocks in conditional ([`merge-duplicate-blocks`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-duplicate-blocks/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Remove redundant conditional ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
</issue_to_address>
### Comment 10
<location> `bot.py:344-345` </location>
<code_context>
def order_type_name_from_record(order: Dict[str, object]) -> str:
type_key = order.get("type_key")
if type_key in ORDER_TYPES:
return ORDER_TYPES[type_key]["name"]
legacy_name = order.get("type")
if legacy_name:
return str(legacy_name)
return order_type_name_from_key(type_key if isinstance(type_key, str) else None)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if legacy_name := order.get("type"):
```
</issue_to_address>
### Comment 11
<location> `bot.py:355-356` </location>
<code_context>
def format_order_summary(draft: Dict[str, object], price: int) -> str:
order_type = ORDER_TYPES.get(draft.get("type_key")) or {}
upsells = draft.get("upsells", [])
upsell_lines = []
for upsell in upsells:
info = UPSELL_OPTIONS.get(upsell)
if info:
upsell_lines.append(f"• {info['title']} (+{info['price']} ₽)")
upsell_text = "\n".join(upsell_lines) if upsell_lines else "—"
deadline_days = int(draft.get("deadline_days", 0))
deadline_date = (datetime.now() + timedelta(days=deadline_days)).strftime("%d.%m.%Y")
return (
f"<b>Проверим данные перед оформлением:</b>\n\n"
f"Тип: {order_type.get('icon', '')} {order_type.get('name', 'Неизвестно')}\n"
f"Тема: {html.escape(str(draft.get('topic', 'не указана')))}\n"
f"Срок: {deadline_days} дн. (до {deadline_date})\n"
f"Требования: {html.escape(str(draft.get('requirements', 'не указаны')))}\n"
f"Доп. услуги: {upsell_text}\n\n"
f"Итого к оплате: <b>{price} ₽</b>"
)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if info := UPSELL_OPTIONS.get(upsell):
```
</issue_to_address>
### Comment 12
<location> `bot.py:514-515` </location>
<code_context>
def build_upsell_keyboard(selected: Iterable[str]) -> InlineKeyboardMarkup:
keyboard: List[List[InlineKeyboardButton]] = []
selected_set = set(selected)
for key, info in UPSELL_OPTIONS.items():
prefix = "✅" if key in selected_set else "➕"
keyboard.append(
[InlineKeyboardButton(f"{prefix} {info['title']} (+{info['price']} ₽)", callback_data=f"order:upsell:{key}")]
)
keyboard.append([InlineKeyboardButton("Продолжить", callback_data="order:summary")])
keyboard.append([InlineKeyboardButton("Отменить", callback_data="order:cancel")])
return InlineKeyboardMarkup(keyboard)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge consecutive list appends into a single extend ([`merge-list-appends-into-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-list-appends-into-extend/))
```suggestion
keyboard.extend(
(
[
InlineKeyboardButton(
"Продолжить", callback_data="order:summary"
)
],
[InlineKeyboardButton("Отменить", callback_data="order:cancel")],
)
)
```
</issue_to_address>
### Comment 13
<location> `bot.py:688-690` </location>
<code_context>
async def calculator_select_deadline(update: Update, context: ContextTypes.DEFAULT_TYPE, key: str) -> int:
context.user_data.setdefault("calculator", {})["type"] = key
keyboard = []
for days in (3, 7, 14, 21, 30):
keyboard.append([InlineKeyboardButton(f"{days} дней", callback_data=f"calc:deadline:{days}")])
keyboard.append([InlineKeyboardButton("⬅️ Назад", callback_data="main:calculator")])
await update.callback_query.edit_message_text(
"Выберите срок выполнения:", reply_markup=InlineKeyboardMarkup(keyboard)
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**suggestion (code-quality):** Convert for loop into list comprehension ([`list-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/list-comprehension/))
```suggestion
keyboard = [
[
InlineKeyboardButton(
f"{days} дней", callback_data=f"calc:deadline:{days}"
)
]
for days in (3, 7, 14, 21, 30)
]
```
</issue_to_address>
### Comment 14
<location> `bot.py:774` </location>
<code_context>
async def show_user_orders(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
user = update.effective_user
if not user:
return STATE_NAVIGATION
orders = store.get_orders(user.id)
if not orders:
text = "Пока нет заказов. Самое время оформить первый!"
else:
lines = []
for order in orders[-10:]:
name = order_type_name_from_record(order)
lines.append(
f"#{order.get('order_id')} — {name}\n"
f"Тема: {order.get('topic')}\n"
f"Срок: {order.get('deadline_date')}\n"
f"Статус: {order.get('status', 'в работе')}\n"
)
text = "\n".join(lines)
keyboard = [[InlineKeyboardButton("⬅️ Назад", callback_data="main:profile")]]
await update.callback_query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 15
<location> `bot.py:893` </location>
<code_context>
async def admin_show_orders(update: Update) -> int:
records = [order for orders in store.orders.values() for order in orders]
records = sorted(records, key=lambda item: item.get("created_at", ""), reverse=True)[:10]
if not records:
text = "Заказов пока нет."
else:
lines = []
for record in records:
name = order_type_name_from_record(record)
lines.append(
f"#{record.get('order_id')} — {name}\n"
f"Тема: {record.get('topic')}\n"
f"Статус: {record.get('status', 'в работе')}\n"
f"Цена: {record.get('price')} ₽\n"
)
text = "\n".join(lines)
await update.callback_query.edit_message_text(
text, reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("⬅️ Назад", callback_data="admin:menu")]])
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 16
<location> `bot.py:941` </location>
<code_context>
async def admin_show_logs(update: Update) -> int:
last_logs = []
for user_id, logs in store.user_logs.items():
if logs:
last_logs.append((user_id, logs[-1]))
if not last_logs:
text = "Логи пока пусты."
else:
lines = [
f"{user_id}: {entry['action']} ({entry['timestamp']})"
for user_id, entry in last_logs[-10:]
]
text = "\n".join(lines)
await update.callback_query.edit_message_text(
text, reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("⬅️ Назад", callback_data="admin:menu")]])
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Convert for loop into list comprehension ([`list-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/list-comprehension/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>
### Comment 17
<location> `bot.py:980` </location>
<code_context>
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
query = update.callback_query
await query.answer()
data = query.data
log_user_action(update, f"callback:{data}")
if data == "main:root":
return await show_main_menu(update, context)
if data == "main:order" or data == "order:list":
return await show_order_types(update, context)
if data.startswith("order:type:"):
key = data.split(":", maxsplit=2)[2]
return await show_order_details(update, key)
if data.startswith("order:new:"):
key = data.split(":", maxsplit=2)[2]
return await prompt_order_topic(update, context, key)
if data.startswith("order:deadline:"):
days = int(data.split(":", maxsplit=2)[2])
return await handle_deadline(update, context, days)
if data.startswith("order:upsell:"):
key = data.split(":", maxsplit=2)[2]
if key:
return await toggle_upsell(update, context, key)
if data == "order:upsell":
return await show_upsell_menu(update, context)
if data == "order:summary":
return await show_order_summary_step(update, context)
if data == "order:confirm":
return await confirm_order(update, context)
if data == "order:cancel":
return await cancel_order(update, context)
if data == "main:prices":
return await show_price_list(update, context)
if data.startswith("prices:detail:"):
key = data.split(":", maxsplit=2)[2]
return await show_price_detail(update, key)
if data == "main:calculator":
return await show_calculator(update, context)
if data.startswith("calc:type:"):
key = data.split(":", maxsplit=2)[2]
return await calculator_select_deadline(update, context, key)
if data.startswith("calc:deadline:"):
days = int(data.split(":", maxsplit=2)[2])
return await calculator_select_complexity(update, context, days)
if data.startswith("calc:complexity:"):
complexity = float(data.split(":", maxsplit=2)[2])
return await show_calculation_result(update, context, complexity)
if data == "main:profile" or data == "profile:back":
return await show_profile(update, context)
if data == "profile:orders":
return await show_user_orders(update, context)
if data == "profile:feedback":
return await request_feedback(update, context)
if data == "main:faq":
return await show_faq(update, context)
if data.startswith("faq:item:"):
idx = int(data.split(":", maxsplit=2)[2])
return await show_faq_item(update, idx)
if data == "admin:menu":
return await show_admin_menu(update, context)
if data == "admin:stats":
return await admin_show_stats(update)
if data == "admin:orders":
return await admin_show_orders(update)
if data == "admin:pricing":
return await admin_request_pricing_mode(update, context)
if data == "admin:export":
return await admin_export(update, context)
if data == "admin:logs":
return await admin_show_logs(update)
await query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Replace multiple comparisons of same variable with `in` operator [×2] ([`merge-comparisons`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-comparisons/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>Sourcery бесплатен для открытого исходного кода — если вам нравятся наши обзоры, пожалуйста, поделитесь ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The handle_callback function is very large with many branches—consider refactoring it into a dispatch table or separate handler functions to improve readability and maintainability.
- Using a single STATE_NAVIGATION for almost all callback flows makes the ConversationHandler configuration and debugging harder; define more granular states to clearly separate different dialog steps.
- DataStore._save_json writes directly to disk on every update without error handling—wrap file writes in try/except or batch them to prevent unexpected crashes and improve performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The handle_callback function is very large with many branches—consider refactoring it into a dispatch table or separate handler functions to improve readability and maintainability.
- Using a single STATE_NAVIGATION for almost all callback flows makes the ConversationHandler configuration and debugging harder; define more granular states to clearly separate different dialog steps.
- DataStore._save_json writes directly to disk on every update without error handling—wrap file writes in try/except or batch them to prevent unexpected crashes and improve performance.
## Individual Comments
### Comment 1
<location> `bot.py:398` </location>
<code_context>
+
+def get_order_draft(context: ContextTypes.DEFAULT_TYPE) -> Dict[str, object]:
+ draft = context.user_data.setdefault("order_draft", {})
+ draft.setdefault("upsells", set())
+ return draft
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Storing sets in user_data may cause serialization issues.
Since sets are not JSON serializable, using them in user_data can break persistence. Use a list for 'upsells' to avoid serialization errors.
</issue_to_address>
### Comment 2
<location> `bot.py:519` </location>
<code_context>
+ return InlineKeyboardMarkup(keyboard)
+
+
+async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ draft = get_order_draft(context)
+ raw_selected = draft.get("upsells", set())
</code_context>
<issue_to_address>
**suggestion:** The upsell selection logic may not handle type consistency for 'upsells'.
Handling both sets and lists for 'upsells' increases complexity and risk of bugs. Standardizing on a list would make the logic clearer and safer.
Suggested implementation:
```python
async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
draft = get_order_draft(context)
raw_selected = draft.get("upsells", [])
selected_list = list(raw_selected) if isinstance(raw_selected, (set, list)) else []
draft["upsells"] = selected_list
if update.message:
await update.message.reply_text(
"Хотите добавить дополнительные материалы? Презентация или речь экономят время на подготовке!",
reply_markup=build_upsell_keyboard(selected_list),
)
else:
query = update.callback_query
await query.edit_message_text(
```
You should also review any other code that reads or writes to `draft["upsells"]` elsewhere in the codebase to ensure it always expects and uses a list, not a set.
If `build_upsell_keyboard` expects a set, update it to accept a list and convert it to a set internally if needed.
</issue_to_address>
### Comment 3
<location> `bot.py:584` </location>
<code_context>
+ return await show_main_menu(update, context)
+
+
+async def confirm_order(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ user = update.effective_user
+ if not user:
</code_context>
<issue_to_address>
**question (bug_risk):** No discount is applied for multiple orders in the new flow.
If the removal of multi-order discounts is intentional, clarify this in the UI. Otherwise, restore the previous discount logic for multiple orders.
</issue_to_address>
### Comment 4
<location> `bot.py:973` </location>
<code_context>
+ await update.message.reply_text("Команда не распознана. Используйте кнопки в меню.")
+ return STATE_ADMIN
+
+async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+ query = update.callback_query
+ await query.answer()
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the long if/elif chain in handle_callback with a dispatch table for callback handlers to simplify the code.
You’ve done an excellent job extracting all of the “building blocks” (keyboards, formatting, datastore) into small focused functions and classes. The only remaining hot-spot of complexity is the giant `handle_callback` if/elif chain. You can collapse it into a simple dispatch table—no functionality changes, but you immediately remove dozens of lines of branching. For example:
```python
# at module‐top, build a mapping from your callback_data → handler
CALLBACK_HANDLERS: dict[str, Callable[..., Awaitable[int]]] = {
"main:root": show_main_menu,
"main:order": show_order_types,
"order:list": show_order_types,
# …
"order:type": show_order_details, # handles order:type:<key>
"order:new": prompt_order_topic, # handles order:new:<key>
# …
"order:confirm": confirm_order,
"order:cancel": cancel_order,
# …
}
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
await update.callback_query.answer()
data = update.callback_query.data # e.g. "order:type:course_empirical"
parts = data.split(":", maxsplit=2)
cmd = ":".join(parts[:2]) # e.g. "order:type"
arg = parts[2] if len(parts) == 3 else None
handler = CALLBACK_HANDLERS.get(cmd)
if handler:
# if your handler needs the extra arg, pass it along
return await handler(update, context, arg) if arg else await handler(update, context)
# fallback
await update.callback_query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)
```
Steps to apply:
1. Create the `CALLBACK_HANDLERS` dict at the top of your module.
2. Convert each `if data.startswith("…")` or `if data == "…"` into a single key in that dict (you can still match prefixes using the first two segments).
3. Replace the big `if/elif` block in `handle_callback` with the generic lookup above.
This trivially collapses ~50 lines of branching into a handful of lines, keeps all existing behavior, and makes adding new callbacks a one-liner.
</issue_to_address>
### Comment 5
<location> `bot.py:272` </location>
<code_context>
active_orders = sum(1 for order in orders_flat if order.get("status", "").lower() not in {"готов", "завершен"})
</code_context>
<issue_to_address>
**suggestion (code-quality):** Simplify constant sum() call ([`simplify-constant-sum`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/simplify-constant-sum))
```suggestion
active_orders = sum(bool(order.get("status", "").lower() not in {"готов", "завершен"})
```
<br/><details><summary>Explanation</summary>As `sum` add the values it treats `True` as `1`, and `False` as `0`. We make use
of this fact to simplify the generator expression inside the `sum` call.
</details>
</issue_to_address>
### Comment 6
<location> `bot.py:1053-1061` </location>
<code_context>
if referrer_id != user.id and store.add_referral(referrer_id, user.id):
if ADMIN_CHAT_ID:
try:
await context.bot.send_message(
ADMIN_CHAT_ID,
f"Новый реферал: {user.id} (пригласил {referrer_id})",
)
except Exception as exc: # pragma: no cover
logger.error("Failed to notify admin about referral: %s", exc)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if referrer_id != user.id and store.add_referral(referrer_id, user.id) and ADMIN_CHAT_ID:
try:
await context.bot.send_message(
ADMIN_CHAT_ID,
f"Новый реферал: {user.id} (пригласил {referrer_id})",
)
except Exception as exc: # pragma: no cover
logger.error("Failed to notify admin about referral: %s", exc)
```
<br/><details><summary>Explanation</summary>Too much nesting can make code difficult to understand, and this is especially
true in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 7
<location> `bot.py:259-260` </location>
<code_context>
def export_orders(self) -> Optional[Path]:
records: List[Dict[str, object]] = []
for user_id, orders in self.orders.items():
for order in orders:
records.append({"user_id": user_id, **order})
if not records:
return None
df = pd.DataFrame(records)
export_path = DATA_DIR / "orders_export.xlsx"
df.to_excel(export_path, index=False)
return export_path
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace a for append loop with list extend ([`for-append-to-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/for-append-to-extend/))
```suggestion
records.extend({"user_id": user_id, **order} for order in orders)
```
</issue_to_address>
### Comment 8
<location> `bot.py:288-290` </location>
<code_context>
def log_user_action(update: Update, action: str) -> None:
user = update.effective_user
if not user:
return
store.log_action(user.id, user.username, action)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 9
<location> `bot.py:301` </location>
<code_context>
def calculate_price(order_type: str, days_left: int, complexity: float = 1.0, upsells: Iterable[str] = ()) -> int:
price_info = store.prices.get(order_type) or DEFAULT_PRICES.get(order_type)
if not price_info:
logger.warning("Unknown order type for pricing: %s", order_type)
return 0
price = int(price_info.get("base", 0) * complexity)
mode = store.get_pricing_mode()
if mode == "hard":
if days_left < 7:
price = int(price * 1.3)
elif days_left < 15:
price = int(price * 1.15)
else:
if days_left < 3:
price = int(price * 1.3)
elif days_left < 7:
price = int(price * 1.15)
for upsell in upsells:
option = UPSELL_OPTIONS.get(upsell)
if option:
price += option["price"]
return price
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Merge duplicate blocks in conditional ([`merge-duplicate-blocks`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-duplicate-blocks/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Remove redundant conditional ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
</issue_to_address>
### Comment 10
<location> `bot.py:344-345` </location>
<code_context>
def order_type_name_from_record(order: Dict[str, object]) -> str:
type_key = order.get("type_key")
if type_key in ORDER_TYPES:
return ORDER_TYPES[type_key]["name"]
legacy_name = order.get("type")
if legacy_name:
return str(legacy_name)
return order_type_name_from_key(type_key if isinstance(type_key, str) else None)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if legacy_name := order.get("type"):
```
</issue_to_address>
### Comment 11
<location> `bot.py:355-356` </location>
<code_context>
def format_order_summary(draft: Dict[str, object], price: int) -> str:
order_type = ORDER_TYPES.get(draft.get("type_key")) or {}
upsells = draft.get("upsells", [])
upsell_lines = []
for upsell in upsells:
info = UPSELL_OPTIONS.get(upsell)
if info:
upsell_lines.append(f"• {info['title']} (+{info['price']} ₽)")
upsell_text = "\n".join(upsell_lines) if upsell_lines else "—"
deadline_days = int(draft.get("deadline_days", 0))
deadline_date = (datetime.now() + timedelta(days=deadline_days)).strftime("%d.%m.%Y")
return (
f"<b>Проверим данные перед оформлением:</b>\n\n"
f"Тип: {order_type.get('icon', '')} {order_type.get('name', 'Неизвестно')}\n"
f"Тема: {html.escape(str(draft.get('topic', 'не указана')))}\n"
f"Срок: {deadline_days} дн. (до {deadline_date})\n"
f"Требования: {html.escape(str(draft.get('requirements', 'не указаны')))}\n"
f"Доп. услуги: {upsell_text}\n\n"
f"Итого к оплате: <b>{price} ₽</b>"
)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if info := UPSELL_OPTIONS.get(upsell):
```
</issue_to_address>
### Comment 12
<location> `bot.py:514-515` </location>
<code_context>
def build_upsell_keyboard(selected: Iterable[str]) -> InlineKeyboardMarkup:
keyboard: List[List[InlineKeyboardButton]] = []
selected_set = set(selected)
for key, info in UPSELL_OPTIONS.items():
prefix = "✅" if key in selected_set else "➕"
keyboard.append(
[InlineKeyboardButton(f"{prefix} {info['title']} (+{info['price']} ₽)", callback_data=f"order:upsell:{key}")]
)
keyboard.append([InlineKeyboardButton("Продолжить", callback_data="order:summary")])
keyboard.append([InlineKeyboardButton("Отменить", callback_data="order:cancel")])
return InlineKeyboardMarkup(keyboard)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge consecutive list appends into a single extend ([`merge-list-appends-into-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-list-appends-into-extend/))
```suggestion
keyboard.extend(
(
[
InlineKeyboardButton(
"Продолжить", callback_data="order:summary"
)
],
[InlineKeyboardButton("Отменить", callback_data="order:cancel")],
)
)
```
</issue_to_address>
### Comment 13
<location> `bot.py:688-690` </location>
<code_context>
async def calculator_select_deadline(update: Update, context: ContextTypes.DEFAULT_TYPE, key: str) -> int:
context.user_data.setdefault("calculator", {})["type"] = key
keyboard = []
for days in (3, 7, 14, 21, 30):
keyboard.append([InlineKeyboardButton(f"{days} дней", callback_data=f"calc:deadline:{days}")])
keyboard.append([InlineKeyboardButton("⬅️ Назад", callback_data="main:calculator")])
await update.callback_query.edit_message_text(
"Выберите срок выполнения:", reply_markup=InlineKeyboardMarkup(keyboard)
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**suggestion (code-quality):** Convert for loop into list comprehension ([`list-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/list-comprehension/))
```suggestion
keyboard = [
[
InlineKeyboardButton(
f"{days} дней", callback_data=f"calc:deadline:{days}"
)
]
for days in (3, 7, 14, 21, 30)
]
```
</issue_to_address>
### Comment 14
<location> `bot.py:774` </location>
<code_context>
async def show_user_orders(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
user = update.effective_user
if not user:
return STATE_NAVIGATION
orders = store.get_orders(user.id)
if not orders:
text = "Пока нет заказов. Самое время оформить первый!"
else:
lines = []
for order in orders[-10:]:
name = order_type_name_from_record(order)
lines.append(
f"#{order.get('order_id')} — {name}\n"
f"Тема: {order.get('topic')}\n"
f"Срок: {order.get('deadline_date')}\n"
f"Статус: {order.get('status', 'в работе')}\n"
)
text = "\n".join(lines)
keyboard = [[InlineKeyboardButton("⬅️ Назад", callback_data="main:profile")]]
await update.callback_query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 15
<location> `bot.py:893` </location>
<code_context>
async def admin_show_orders(update: Update) -> int:
records = [order for orders in store.orders.values() for order in orders]
records = sorted(records, key=lambda item: item.get("created_at", ""), reverse=True)[:10]
if not records:
text = "Заказов пока нет."
else:
lines = []
for record in records:
name = order_type_name_from_record(record)
lines.append(
f"#{record.get('order_id')} — {name}\n"
f"Тема: {record.get('topic')}\n"
f"Статус: {record.get('status', 'в работе')}\n"
f"Цена: {record.get('price')} ₽\n"
)
text = "\n".join(lines)
await update.callback_query.edit_message_text(
text, reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("⬅️ Назад", callback_data="admin:menu")]])
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
</issue_to_address>
### Comment 16
<location> `bot.py:941` </location>
<code_context>
async def admin_show_logs(update: Update) -> int:
last_logs = []
for user_id, logs in store.user_logs.items():
if logs:
last_logs.append((user_id, logs[-1]))
if not last_logs:
text = "Логи пока пусты."
else:
lines = [
f"{user_id}: {entry['action']} ({entry['timestamp']})"
for user_id, entry in last_logs[-10:]
]
text = "\n".join(lines)
await update.callback_query.edit_message_text(
text, reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("⬅️ Назад", callback_data="admin:menu")]])
)
return STATE_NAVIGATION
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Convert for loop into list comprehension ([`list-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/list-comprehension/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>
### Comment 17
<location> `bot.py:980` </location>
<code_context>
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
query = update.callback_query
await query.answer()
data = query.data
log_user_action(update, f"callback:{data}")
if data == "main:root":
return await show_main_menu(update, context)
if data == "main:order" or data == "order:list":
return await show_order_types(update, context)
if data.startswith("order:type:"):
key = data.split(":", maxsplit=2)[2]
return await show_order_details(update, key)
if data.startswith("order:new:"):
key = data.split(":", maxsplit=2)[2]
return await prompt_order_topic(update, context, key)
if data.startswith("order:deadline:"):
days = int(data.split(":", maxsplit=2)[2])
return await handle_deadline(update, context, days)
if data.startswith("order:upsell:"):
key = data.split(":", maxsplit=2)[2]
if key:
return await toggle_upsell(update, context, key)
if data == "order:upsell":
return await show_upsell_menu(update, context)
if data == "order:summary":
return await show_order_summary_step(update, context)
if data == "order:confirm":
return await confirm_order(update, context)
if data == "order:cancel":
return await cancel_order(update, context)
if data == "main:prices":
return await show_price_list(update, context)
if data.startswith("prices:detail:"):
key = data.split(":", maxsplit=2)[2]
return await show_price_detail(update, key)
if data == "main:calculator":
return await show_calculator(update, context)
if data.startswith("calc:type:"):
key = data.split(":", maxsplit=2)[2]
return await calculator_select_deadline(update, context, key)
if data.startswith("calc:deadline:"):
days = int(data.split(":", maxsplit=2)[2])
return await calculator_select_complexity(update, context, days)
if data.startswith("calc:complexity:"):
complexity = float(data.split(":", maxsplit=2)[2])
return await show_calculation_result(update, context, complexity)
if data == "main:profile" or data == "profile:back":
return await show_profile(update, context)
if data == "profile:orders":
return await show_user_orders(update, context)
if data == "profile:feedback":
return await request_feedback(update, context)
if data == "main:faq":
return await show_faq(update, context)
if data.startswith("faq:item:"):
idx = int(data.split(":", maxsplit=2)[2])
return await show_faq_item(update, idx)
if data == "admin:menu":
return await show_admin_menu(update, context)
if data == "admin:stats":
return await admin_show_stats(update)
if data == "admin:orders":
return await admin_show_orders(update)
if data == "admin:pricing":
return await admin_request_pricing_mode(update, context)
if data == "admin:export":
return await admin_export(update, context)
if data == "admin:logs":
return await admin_show_logs(update)
await query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Replace multiple comparisons of same variable with `in` operator [×2] ([`merge-comparisons`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-comparisons/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| def get_order_draft(context: ContextTypes.DEFAULT_TYPE) -> Dict[str, object]: | ||
| draft = context.user_data.setdefault("order_draft", {}) | ||
| draft.setdefault("upsells", set()) |
There was a problem hiding this comment.
issue (bug_risk): Хранение множеств в user_data может вызвать проблемы с сериализацией.
Поскольку множества не сериализуются в JSON, их использование в user_data может нарушить персистентность. Используй список для 'upsells', чтобы избежать ошибок сериализации.
Original comment in English
issue (bug_risk): Storing sets in user_data may cause serialization issues.
Since sets are not JSON serializable, using them in user_data can break persistence. Use a list for 'upsells' to avoid serialization errors.
| return InlineKeyboardMarkup(keyboard) | ||
|
|
||
|
|
||
| async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int: |
There was a problem hiding this comment.
suggestion: Логика выбора допродаж может не обеспечивать согласованность типов для 'upsells'.
Обработка как множеств, так и списков для 'upsells' увеличивает сложность и риск возникновения ошибок. Стандартизация на списке сделает логику более понятной и безопасной.
Предлагаемая реализация:
async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
draft = get_order_draft(context)
raw_selected = draft.get("upsells", [])
selected_list = list(raw_selected) if isinstance(raw_selected, (set, list)) else []
draft["upsells"] = selected_list
if update.message:
await update.message.reply_text(
"Хотите добавить дополнительные материалы? Презентация или речь экономят время на подготовке!",
reply_markup=build_upsell_keyboard(selected_list),
)
else:
query = update.callback_query
await query.edit_message_text(Тебе также следует проверить любой другой код, который читает или записывает draft["upsells"] в других частях кодовой базы, чтобы убедиться, что он всегда ожидает и использует список, а не множество.
Если build_upsell_keyboard ожидает множество, обнови его, чтобы он принимал список и при необходимости преобразовывал его во множество внутри.
Original comment in English
suggestion: The upsell selection logic may not handle type consistency for 'upsells'.
Handling both sets and lists for 'upsells' increases complexity and risk of bugs. Standardizing on a list would make the logic clearer and safer.
Suggested implementation:
async def show_upsell_menu(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
draft = get_order_draft(context)
raw_selected = draft.get("upsells", [])
selected_list = list(raw_selected) if isinstance(raw_selected, (set, list)) else []
draft["upsells"] = selected_list
if update.message:
await update.message.reply_text(
"Хотите добавить дополнительные материалы? Презентация или речь экономят время на подготовке!",
reply_markup=build_upsell_keyboard(selected_list),
)
else:
query = update.callback_query
await query.edit_message_text(You should also review any other code that reads or writes to draft["upsells"] elsewhere in the codebase to ensure it always expects and uses a list, not a set.
If build_upsell_keyboard expects a set, update it to accept a list and convert it to a set internally if needed.
| return await show_main_menu(update, context) | ||
|
|
||
|
|
||
| async def confirm_order(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int: |
There was a problem hiding this comment.
question (bug_risk): В новом потоке не применяется скидка на несколько заказов.
Если удаление скидок на несколько заказов преднамеренно, уточни это в пользовательском интерфейсе. В противном случае, восстанови предыдущую логику скидок для нескольких заказов.
Original comment in English
question (bug_risk): No discount is applied for multiple orders in the new flow.
If the removal of multi-order discounts is intentional, clarify this in the UI. Otherwise, restore the previous discount logic for multiple orders.
| await update.message.reply_text("Команда не распознана. Используйте кнопки в меню.") | ||
| return STATE_ADMIN | ||
|
|
||
| async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int: |
There was a problem hiding this comment.
issue (complexity): Рассмотри возможность замены длинной цепочки if/elif в handle_callback на таблицу диспетчеризации для обработчиков обратных вызовов, чтобы упростить код.
Ты отлично справился с извлечением всех «строительных блоков» (клавиатур, форматирования, хранилища данных) в небольшие сфокусированные функции и классы. Единственным оставшимся очагом сложности является гигантская цепочка if/elif в handle_callback. Ты можешь свернуть ее в простую таблицу диспетчеризации — никаких изменений функциональности, но ты немедленно удалишь десятки строк ветвлений. Например:
# at module‐top, build a mapping from your callback_data → handler
CALLBACK_HANDLERS: dict[str, Callable[..., Awaitable[int]]] = {
"main:root": show_main_menu,
"main:order": show_order_types,
"order:list": show_order_types,
# …
"order:type": show_order_details, # handles order:type:<key>
"order:new": prompt_order_topic, # handles order:new:<key>
# …
"order:confirm": confirm_order,
"order:cancel": cancel_order,
# …
}
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
await update.callback_query.answer()
data = update.callback_query.data # e.g. "order:type:course_empirical"
parts = data.split(":", maxsplit=2)
cmd = ":".join(parts[:2]) # e.g. "order:type"
arg = parts[2] if len(parts) == 3 else None
handler = CALLBACK_HANDLERS.get(cmd)
if handler:
# if your handler needs the extra arg, pass it along
return await handler(update, context, arg) if arg else await handler(update, context)
# fallback
await update.callback_query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)Шаги по применению:
- Создай словарь
CALLBACK_HANDLERSв начале твоего модуля. - Преобразуй каждое
if data.startswith("…")илиif data == "…"в один ключ в этом словаре (ты все еще можешь сопоставлять префиксы, используя первые два сегмента). - Замени большой блок
if/elifвhandle_callbackна общий поиск, приведенный выше.
Это тривиально сокращает около 50 строк ветвлений до нескольких строк, сохраняет все существующее поведение и делает добавление новых обратных вызовов однострочником.
Original comment in English
issue (complexity): Consider replacing the long if/elif chain in handle_callback with a dispatch table for callback handlers to simplify the code.
You’ve done an excellent job extracting all of the “building blocks” (keyboards, formatting, datastore) into small focused functions and classes. The only remaining hot-spot of complexity is the giant handle_callback if/elif chain. You can collapse it into a simple dispatch table—no functionality changes, but you immediately remove dozens of lines of branching. For example:
# at module‐top, build a mapping from your callback_data → handler
CALLBACK_HANDLERS: dict[str, Callable[..., Awaitable[int]]] = {
"main:root": show_main_menu,
"main:order": show_order_types,
"order:list": show_order_types,
# …
"order:type": show_order_details, # handles order:type:<key>
"order:new": prompt_order_topic, # handles order:new:<key>
# …
"order:confirm": confirm_order,
"order:cancel": cancel_order,
# …
}
async def handle_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
await update.callback_query.answer()
data = update.callback_query.data # e.g. "order:type:course_empirical"
parts = data.split(":", maxsplit=2)
cmd = ":".join(parts[:2]) # e.g. "order:type"
arg = parts[2] if len(parts) == 3 else None
handler = CALLBACK_HANDLERS.get(cmd)
if handler:
# if your handler needs the extra arg, pass it along
return await handler(update, context, arg) if arg else await handler(update, context)
# fallback
await update.callback_query.edit_message_text("Команда не распознана. Возвращаюсь в меню.")
return await show_main_menu(update, context)Steps to apply:
- Create the
CALLBACK_HANDLERSdict at the top of your module. - Convert each
if data.startswith("…")orif data == "…"into a single key in that dict (you can still match prefixes using the first two segments). - Replace the big
if/elifblock inhandle_callbackwith the generic lookup above.
This trivially collapses ~50 lines of branching into a handful of lines, keeps all existing behavior, and makes adding new callbacks a one-liner.
| orders_flat = [order for orders in self.orders.values() for order in orders] | ||
| total_orders = len(orders_flat) | ||
| total_revenue = int(sum(int(order.get("price", 0)) for order in orders_flat)) | ||
| active_orders = sum(1 for order in orders_flat if order.get("status", "").lower() not in {"готов", "завершен"}) |
There was a problem hiding this comment.
suggestion (code-quality): Упрости вызов sum() с константой (simplify-constant-sum)
Suggested change
| active_orders = sum(1 for order in orders_flat if order.get("status", "").lower() not in {"готов", "завершен"}) | |
| active_orders = sum(bool(order.get("status", "").lower() not in {"готов", "завершен"}) |
Объяснение
Посколькуsum складывает значения, он обрабатывает True как 1, а False как 0. Мы используемэтот факт для упрощения генераторного выражения внутри вызова
sum.
Original comment in English
suggestion (code-quality): Simplify constant sum() call (simplify-constant-sum)
Suggested change
| active_orders = sum(1 for order in orders_flat if order.get("status", "").lower() not in {"готов", "завершен"}) | |
| active_orders = sum(bool(order.get("status", "").lower() not in {"готов", "завершен"}) |
Explanation
Assum add the values it treats True as 1, and False as 0. We make useof this fact to simplify the generator expression inside the
sum call.
Comment on lines
+688
to
+690
| keyboard = [] | ||
| for days in (3, 7, 14, 21, 30): | ||
| keyboard.append([InlineKeyboardButton(f"{days} дней", callback_data=f"calc:deadline:{days}")]) |
There was a problem hiding this comment.
suggestion (code-quality): Преобразуй цикл for в списковое включение (list-comprehension)
Suggested change
| keyboard = [] | |
| for days in (3, 7, 14, 21, 30): | |
| keyboard.append([InlineKeyboardButton(f"{days} дней", callback_data=f"calc:deadline:{days}")]) | |
| keyboard = [ | |
| [ | |
| InlineKeyboardButton( | |
| f"{days} дней", callback_data=f"calc:deadline:{days}" | |
| ) | |
| ] | |
| for days in (3, 7, 14, 21, 30) | |
| ] |
Original comment in English
suggestion (code-quality): Convert for loop into list comprehension (list-comprehension)
Suggested change
| keyboard = [] | |
| for days in (3, 7, 14, 21, 30): | |
| keyboard.append([InlineKeyboardButton(f"{days} дней", callback_data=f"calc:deadline:{days}")]) | |
| keyboard = [ | |
| [ | |
| InlineKeyboardButton( | |
| f"{days} дней", callback_data=f"calc:deadline:{days}" | |
| ) | |
| ] | |
| for days in (3, 7, 14, 21, 30) | |
| ] |
| user = update.effective_user | ||
| if not user: | ||
| return STATE_NAVIGATION | ||
| orders = store.get_orders(user.id) |
There was a problem hiding this comment.
issue (code-quality): Мы обнаружили следующие проблемы:
- Используй именованное выражение для упрощения присваивания и условного оператора (
use-named-expression) - Поменяй местами ветви if/else (
swap-if-else-branches)
Original comment in English
issue (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Swap if/else branches (
swap-if-else-branches)
|
|
||
| async def admin_show_orders(update: Update) -> int: | ||
| records = [order for orders in store.orders.values() for order in orders] | ||
| records = sorted(records, key=lambda item: item.get("created_at", ""), reverse=True)[:10] |
There was a problem hiding this comment.
issue (code-quality): Мы обнаружили следующие проблемы:
- Используй именованное выражение для упрощения присваивания и условного оператора (
use-named-expression) - Поменяй местами ветви if/else (
swap-if-else-branches)
Original comment in English
issue (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Swap if/else branches (
swap-if-else-branches)
|
|
||
|
|
||
| async def admin_show_logs(update: Update) -> int: | ||
| last_logs = [] |
There was a problem hiding this comment.
issue (code-quality): Мы обнаружили следующие проблемы:
- Преобразуй цикл for в списковое включение (
list-comprehension) - Поменяй местами ветви if/else (
swap-if-else-branches) - Используй именованное выражение для упрощения присваивания и условного оператора (
use-named-expression)
Original comment in English
issue (code-quality): We've found these issues:
- Convert for loop into list comprehension (
list-comprehension) - Swap if/else branches (
swap-if-else-branches) - Use named expression to simplify assignment and conditional (
use-named-expression)
| log_user_action(update, f"callback:{data}") | ||
| if data == "main:root": | ||
| return await show_main_menu(update, context) | ||
| if data == "main:order" or data == "order:list": |
There was a problem hiding this comment.
issue (code-quality): Мы обнаружили следующие проблемы:
- Замени несколько сравнений одной и той же переменной оператором
in[×2] (merge-comparisons) - Используй именованное выражение для упрощения присваивания и условного оператора (
use-named-expression)
Original comment in English
issue (code-quality): We've found these issues:
- Replace multiple comparisons of same variable with
inoperator [×2] (merge-comparisons) - Use named expression to simplify assignment and conditional (
use-named-expression)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68c88efeb150832fbcb75915e1b655e8
Сводка от Sourcery
Рефакторинг Telegram-бота путем перепроектирования управления данными вокруг абстракции DataStore, структурирования заказов как dataclass'ов и оптимизации обработчиков диалогов. Расширение панели администратора статистикой и экспортом в Excel, консолидация конструкторов клавиатур, принудительное выполнение проверок конфигурации и обновление документации по настройке и использованию.
Новые возможности:
Улучшения:
Сборка:
Документация:
Original summary in English
Summary by Sourcery
Refactor the Telegram bot by redesigning data management around a DataStore abstraction, structuring orders as dataclasses, and streamlining conversation handlers. Enhance the admin panel with statistics and Excel exports, consolidate keyboard builders, enforce configuration checks, and update documentation for setup and usage.
New Features:
Enhancements:
Build:
Documentation: