Require contact info and manage attachments in order flow - #2
Open
soloveyska1 wants to merge 4 commits into
Open
Require contact info and manage attachments in order flow#2soloveyska1 wants to merge 4 commits into
soloveyska1 wants to merge 4 commits into
Conversation
Руководство для рецензентаЭтот PR обогащает процесс оформления заказа, добавляя необязательную загрузку файлов и обязательный шаг ввода контактных данных, обновляя модель данных заказа, улучшая уведомления как для пользователей, так и для администраторов с помощью форматированного HTML-вывода и отправки вложений, а также обновляя панель администратора для навигации, просмотра, отмены или удаления отдельных заказов, а также содержит мелкие доработки пользовательского интерфейса и констант. Диаграмма последовательности для оформления заказа с контактными данными и вложениямиsequenceDiagram
actor User
participant Bot
participant Admin
User->>Bot: Start order
Bot->>User: Ask for order type
User->>Bot: Select type
Bot->>User: Ask for topic
User->>Bot: Enter topic
Bot->>User: Ask for deadline
User->>Bot: Select deadline
Bot->>User: Ask for requirements
User->>Bot: Enter requirements or skip
Bot->>User: Prompt for file uploads
User->>Bot: Upload files (optional)
User->>Bot: /done or /skip
Bot->>User: Request contact info (required)
User->>Bot: Enter contact info
Bot->>User: Ask for upsells
User->>Bot: Select upsells (optional)
Bot->>User: Confirm cart
User->>Bot: Confirm order
Bot->>Admin: Send order summary with contact and attachments
Bot->>User: Send confirmation
Диаграмма последовательности для действий по управлению заказами в панели администратораsequenceDiagram
actor Admin
participant Bot
Admin->>Bot: Open admin panel
Bot->>Admin: Show list of orders
Admin->>Bot: Select order
Bot->>Admin: Show order details (with contact and attachments info)
Admin->>Bot: Cancel or delete order
Bot->>Admin: Update order status or remove order
Bot->>Admin: Confirm action
Диаграмма классов для обновленной структуры данных заказаclassDiagram
class Order {
+type: str
+topic: str
+deadline_days: int
+requirements: str
+upsells: list
+price: int
+status: str
+attachments: list
+contact: str
+created_at: str
+order_id: int
}
class Attachment {
+type: str
+file_id: str
+file_name: str
+mime_type: str
+file_unique_id: str
+caption: str
}
Order "1" -- "*" Attachment: contains
Изменения на уровне файлов
Советы и командыВзаимодействие с Sourcery
Настройка вашего опытаПолучите доступ к своей панели управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideThis PR enriches the order flow by inserting optional file uploads and a required contact step, updating the order data model, enhancing both user and admin notifications with rich HTML output and attachment dispatch, and upgrading the admin panel to navigate, view, cancel, or delete individual orders, plus minor UI and constant tweaks. Sequence diagram for order placement with contact and attachmentssequenceDiagram
actor User
participant Bot
participant Admin
User->>Bot: Start order
Bot->>User: Ask for order type
User->>Bot: Select type
Bot->>User: Ask for topic
User->>Bot: Enter topic
Bot->>User: Ask for deadline
User->>Bot: Select deadline
Bot->>User: Ask for requirements
User->>Bot: Enter requirements or skip
Bot->>User: Prompt for file uploads
User->>Bot: Upload files (optional)
User->>Bot: /done or /skip
Bot->>User: Request contact info (required)
User->>Bot: Enter contact info
Bot->>User: Ask for upsells
User->>Bot: Select upsells (optional)
Bot->>User: Confirm cart
User->>Bot: Confirm order
Bot->>Admin: Send order summary with contact and attachments
Bot->>User: Send confirmation
Sequence diagram for admin panel order management actionssequenceDiagram
actor Admin
participant Bot
Admin->>Bot: Open admin panel
Bot->>Admin: Show list of orders
Admin->>Bot: Select order
Bot->>Admin: Show order details (with contact and attachments info)
Admin->>Bot: Cancel or delete order
Bot->>Admin: Update order status or remove order
Bot->>Admin: Confirm action
Class diagram for updated order data structureclassDiagram
class Order {
+type: str
+topic: str
+deadline_days: int
+requirements: str
+upsells: list
+price: int
+status: str
+attachments: list
+contact: str
+created_at: str
+order_id: int
}
class Attachment {
+type: str
+file_id: str
+file_name: str
+mime_type: str
+file_unique_id: str
+caption: str
}
Order "1" -- "*" Attachment: contains
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.
Привет! Я просмотрел твои изменения — вот несколько замечаний:
- Рассмотрите возможность рефакторинга логики загрузки файлов и сбора контактов в выделенные модули или классы, чтобы уменьшить размер bot.py и улучшить удобство сопровождения.
- Добавьте проверку для загружаемых вложений (например, размер файла, тип MIME), чтобы предотвратить отправку пользователями слишком больших или неподдерживаемых файлов.
- Список заказов администратора в настоящее время отображает все элементы сразу; рассмотрите возможность реализации пагинации или динамической загрузки для более эффективного управления большим количеством заказов.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refactoring the file upload and contact collection logic into dedicated modules or classes to reduce the size of bot.py and improve maintainability.
- Add validation for uploaded attachments (e.g., file size, mime type) to prevent users from sending excessively large or unsupported files.
- The admin orders list currently renders all items at once; consider implementing pagination or dynamic loading to manage large numbers of orders more effectively.
## Individual Comments
### Comment 1
<location> `bot.py:77-86` </location>
<code_context>
+def format_contact_link(contact: str) -> str:
</code_context>
<issue_to_address>
**suggestion:** Consider handling contacts with non-standard characters more robustly.
Telegram usernames may include numbers and are case-insensitive, but your current validation excludes some valid characters like dashes or periods. Consider updating the allowed character set or notifying users when their input is invalid.
</issue_to_address>
### Comment 2
<location> `bot.py:99-108` </location>
<code_context>
+def build_order_details(uid: str, order: dict) -> str:
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Escape upsell titles to prevent HTML injection.
Escape each upsell title individually before joining to avoid malformed output if titles contain HTML special characters.
Suggested implementation:
```python
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
upsells = order.get('upsells', [])
escaped_upsell_titles = [escape(str(title)) for title in upsells]
upsell_titles_str = ', '.join(escaped_upsell_titles) if escaped_upsell_titles else 'Нет'
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Упселлы: {upsell_titles_str}",
```
If the upsell titles are displayed elsewhere or if the upsells field is not a list of titles, you may need to adjust the extraction logic accordingly. Also, ensure that the `escape` function is imported from the correct module (e.g., `from html import escape`) at the top of the file if not already present.
</issue_to_address>
### Comment 3
<location> `bot.py:371-380` </location>
<code_context>
+ return UPLOAD_FILES
+
+async def skip_files(update: Update, context: ContextTypes.DEFAULT_TYPE):
+ context.user_data['current_files'] = []
+ await update.message.reply_text("Пропускаем прикрепление файлов.")
+ return await request_contact(update, context)
+
+async def finish_files(update: Update, context: ContextTypes.DEFAULT_TYPE):
</code_context>
<issue_to_address>
**issue (performance):** Potential for large file uploads without size or count limits.
Currently, there are no checks on file size or count, which may cause performance or storage problems. Please implement limits and notify users when these are exceeded.
</issue_to_address>
### Comment 4
<location> `bot.py:445-452` </location>
<code_context>
+ return INPUT_CONTACT
+
+async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
+ contact = update.message.text.strip()
+ if not contact:
+ await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.")
+ return INPUT_CONTACT
+ context.user_data['current_contact'] = contact
+ context.user_data['last_contact'] = contact
+ return await add_upsell(update, context)
+
# Добавление допуслуг
</code_context>
<issue_to_address>
**suggestion:** Contact field validation may be too permissive.
The current implementation accepts any non-empty string as contact information, which may allow invalid entries. Please implement validation for expected contact formats and notify users when their input is invalid.
```suggestion
import re
def is_valid_contact(contact: str) -> bool:
# Email
email_pattern = r"^[\w\.-]+@[\w\.-]+\.\w+$"
# Phone (simple international and local formats)
phone_pattern = r"^\+?\d{10,15}$"
# Telegram username
telegram_pattern = r"^@[\w\d_]{5,}$"
return (
re.match(email_pattern, contact)
or re.match(phone_pattern, contact)
or re.match(telegram_pattern, contact)
)
async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
contact = update.message.text.strip()
if not contact:
await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.")
return INPUT_CONTACT
if not is_valid_contact(contact):
await update.message.reply_text(
"Пожалуйста, укажите корректный контакт: email, номер телефона (10-15 цифр, можно с +), или Telegram username (@username)."
)
return INPUT_CONTACT
context.user_data['current_contact'] = contact
context.user_data['last_contact'] = contact
return await add_upsell(update, context)
```
</issue_to_address>
### Comment 5
<location> `bot.py:667` </location>
<code_context>
return await show_admin_menu(update, context)
# Обработчик админ-меню
-async def admin_menu_handler(update: Update, context: ContextTypes.DEFAULT_TYPE):
- query = update.callback_query
- await query.answer()
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring large handler cascades and file/contact flows into separate modules with dispatch maps and conversation handlers.
```markdown
You’ve done great work adding file-upload/contact flows and richer admin screens, but `bot.py` is now ~1 200 LOC with two huge `if/elif` cascades and even a duplicated `add_upsell()` definition. Two quick wins:
1) **Extract & dispatch admin callbacks**
Replace the big `admin_menu_handler` `if/elif` tree with a small dispatch map + per-action functions. E.g.:
```python
# admin.py
from telegram import InlineKeyboardMarkup, InlineKeyboardButton
from telegram.constants import ParseMode
async def list_orders(update, context):
query = update.callback_query
await query.answer()
buttons = []
for uid, orders in ORDERS.items():
for o in orders:
buttons.append([
InlineKeyboardButton(
f"#{o['order_id']} ({uid})",
callback_data=f"admin_order|{uid}|{o['order_id']}"
)
])
buttons.append([InlineKeyboardButton("Назад", callback_data="admin_menu")])
await query.edit_message_text(
"📋 Заказы:\n" + ("\n".join(f"#{o['order_id']} от {uid}: {o['status']}"
for uid, orders in ORDERS.items() for o in orders)
or "Пусто"),
reply_markup=InlineKeyboardMarkup(buttons)
)
return ADMIN_MENU
async def view_order(update, context, uid, oid):
query = update.callback_query
await query.answer()
order = next((o for o in ORDERS.get(uid, []) if str(o['order_id'])==oid), None)
text = build_order_details(uid, order) if order else "Заказ не найден."
kb = [[InlineKeyboardButton("⬅️ Назад", callback_data="admin_orders")]]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(kb), parse_mode=ParseMode.HTML)
return ADMIN_MENU
# … similarly: cancel_order(), delete_order()
ADMIN_ACTIONS = {
"admin_orders": list_orders,
"admin_order": view_order,
"admin_cancel": cancel_order,
"admin_delete": delete_order,
}
async def admin_menu_handler(update, context):
data = (await update.callback_query).data
key, *args = data.split("|")
handler = ADMIN_ACTIONS.get(key)
if handler:
return await handler(update, context, *args)
return await show_admin_menu(update, context)
```
In your main file just import `admin_menu_handler` and remove the big cascade.
2) **Move file-upload/contact steps into their own module**
Right now you have 6 functions in a row for UPLOAD_FILES→INPUT_CONTACT. Bundle them into a mini `ConversationHandler` in `file_flow.py` and register it separately:
```python
# file_flow.py
from telegram.ext import (
ConversationHandler, MessageHandler, CommandHandler, filters, CallbackQueryHandler
)
states = {
'UPLOAD_FILES': [ # use your UPLOAD_FILES constant
MessageHandler(filters.Document.ALL, handle_document_upload),
MessageHandler(filters.PHOTO, handle_photo_upload),
# …
],
'INPUT_CONTACT': [
MessageHandler(filters.TEXT & ~filters.COMMAND, input_contact),
],
}
file_upload_flow = ConversationHandler(
entry_points=[CallbackQueryHandler(prompt_file_upload, pattern='^add_upsell$')],
states=states,
fallbacks=[
CommandHandler('skip', skip_files),
CommandHandler('done', finish_files),
]
)
```
Then in your setup:
```python
from file_flow import file_upload_flow
application.add_handler(file_upload_flow)
```
3) **Remove the duplicate `add_upsell()`** – consolidate its logic into one place.
These steps will shrink `bot.py` by hundreds of lines, improve readability, and make each flow easier to test.
</issue_to_address>
### Comment 6
<location> `bot.py:634` </location>
<code_context>
f"Требования: {requirements if requirements else 'Нет'}",
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
f"Требования: {requirements or 'Нет'}",
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>
### Comment 7
<location> `bot.py:105` </location>
<code_context>
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Контакт: {format_contact_link(order.get('contact'))}",
f"Требования: {escape(order.get('requirements', 'Нет'))}",
]
if order.get('upsells'):
upsells_readable = ', '.join(UPSELL_TITLES.get(code, code) for code in order['upsells'])
lines.append(f"Допы: {escape(upsells_readable)}")
else:
lines.append("Допы: нет")
lines.append(f"Файлов: {len(order.get('attachments') or [])}")
if order.get('created_at'):
lines.append(f"Создан: {escape(order['created_at'])}")
return '<br>'.join(lines)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove unnecessary casts to int, str, float or bool ([`remove-unnecessary-cast`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-cast/))
```suggestion
f'Пользователь: <a href=\"{user_link}\">{escape(uid)}</a>',
```
</issue_to_address>
### Comment 8
<location> `bot.py:434-435` </location>
<code_context>
async def request_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
context.user_data.pop('current_contact', None)
prompt = (
"Укажите контакт, куда менеджеру написать (Telegram, ВКонтакте, почта). Это обязательное поле."
)
last_contact = context.user_data.get('last_contact')
if last_contact:
prompt += f"\nРанее вы указывали: {last_contact}. Можно отправить его снова или написать другой."
if update.message:
await update.message.reply_text(prompt)
elif update.callback_query:
query = update.callback_query
await query.answer()
await query.edit_message_text(prompt)
return INPUT_CONTACT
</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 last_contact := context.user_data.get('last_contact'):
```
</issue_to_address>
### Comment 9
<location> `bot.py:888` </location>
<code_context>
async def admin_menu_handler(update: Update, context: ContextTypes.DEFAULT_TYPE):
query = update.callback_query
await query.answer()
data = query.data
if data == 'admin_menu':
return await show_admin_menu(update, context)
if data == 'admin_orders':
text_lines = []
buttons = []
for uid, ords in ORDERS.items():
for ord_data in ords:
text_lines.append(f"#{ord_data.get('order_id', 'N/A')} от {uid}: {ord_data.get('status', 'новый')}")
buttons.append([
InlineKeyboardButton(
f"#{ord_data.get('order_id', 'N/A')} ({uid})",
callback_data=f"admin_order|{uid}|{ord_data.get('order_id', 'N/A')}"
)
])
if not text_lines:
text = "Заказы отсутствуют."
else:
text = "📋 Заказы:\n" + "\n".join(text_lines[:20])
buttons.append([InlineKeyboardButton("Назад", callback_data='admin_menu')])
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(buttons))
return ADMIN_MENU
if data.startswith('admin_order|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order = next((o for o in ORDERS.get(uid, []) if str(o.get('order_id')) == order_id_str), None)
if not order:
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
text = build_order_details(uid, order)
keyboard = [
[InlineKeyboardButton("Отменить заказ", callback_data=f'admin_cancel|{uid}|{order_id_str}')],
[InlineKeyboardButton("Удалить заказ", callback_data=f'admin_delete|{uid}|{order_id_str}')],
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard), parse_mode=ParseMode.HTML)
return ADMIN_MENU
if data.startswith('admin_cancel|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order = next((o for o in ORDERS.get(uid, []) if str(o.get('order_id')) == order_id_str), None)
if not order:
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order['status'] = 'отменен'
save_json(ORDERS_FILE, ORDERS)
text = f"Статус заказа #{order_id_str} обновлен на 'отменен'."
keyboard = [
[InlineKeyboardButton("Посмотреть заказ", callback_data=f'admin_order|{uid}|{order_id_str}')],
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
if data.startswith('admin_delete|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
orders_list = ORDERS.get(uid, [])
new_list = [o for o in orders_list if str(o.get('order_id')) != order_id_str]
if len(new_list) == len(orders_list):
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
if new_list:
ORDERS[uid] = new_list
else:
ORDERS.pop(uid, None)
save_json(ORDERS_FILE, ORDERS)
text = f"Заказ #{order_id_str} удален."
keyboard = [
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')],
[InlineKeyboardButton("Админ-меню", callback_data='admin_menu')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
text = ""
keyboard = [[InlineKeyboardButton("Назад", callback_data='admin_menu')]]
if data == 'admin_users':
text = "👥 Пользователи:\n" + "\n".join(f"ID: {uid}" for uid in ORDERS.keys())
elif data == 'admin_logs':
text = "📊 Логи (последние 10):\n"
for uid, logs in list(USER_LOGS.items())[-10:]:
if logs:
text += f"Пользователь {uid}: {logs[-1]['action']}\n"
elif data == 'admin_prices':
text = f"Текущий режим: {current_pricing_mode}\nВведите новый режим (hard/light):"
context.user_data['admin_state'] = 'change_mode'
elif data == 'admin_export':
df = pd.DataFrame([{'user_id': uid, **ord} for uid, ords in ORDERS.items() for ord in ords])
export_file = os.path.join(DATA_DIR, 'orders_export.csv')
df.to_csv(export_file, index=False)
await context.bot.send_document(ADMIN_CHAT_ID, open(export_file, 'rb'))
os.remove(export_file)
text = "📤 Экспорт отправлен!"
elif data == 'back_to_main':
return await main_menu(update, context)
await query.edit_message_text(text or "Неизвестная команда. Возвращаюсь в админ-меню.", reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
</code_context>
<issue_to_address>
**issue (code-quality):** Low code quality found in admin\_menu\_handler - 12% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details></s>
</details>
***
<details>
<summary>Sourcery is free for open source - if you like our reviews please consider sharing them ✨</summary>
- [X](https://twitter.com/intent/tweet?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20got%20an%20instant%20code%20review%20from%20%40SourceryAI%2C%20and%20it%20was%20brilliant%21%20It%27s%20free%20for%20open%20source%20and%20has%20a%20free%20trial%20for%20private%20code.%20Check%20it%20out%20https%3A//sourcery.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https://sourcery.ai)
- [Facebook](https://www.facebook.com/sharer/sharer.php?u=https://sourcery.ai)
</details>
<sub>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
</sub>
<details>
<summary>Original comment in English</summary>
Hey there - I've reviewed your changes - here's some feedback:
- Consider refactoring the file upload and contact collection logic into dedicated modules or classes to reduce the size of bot.py and improve maintainability.
- Add validation for uploaded attachments (e.g., file size, mime type) to prevent users from sending excessively large or unsupported files.
- The admin orders list currently renders all items at once; consider implementing pagination or dynamic loading to manage large numbers of orders more effectively.
<details>
<summary>Prompt for AI Agents</summary>
~~~markdown
Please address the comments from this code review:
## Overall Comments
- Consider refactoring the file upload and contact collection logic into dedicated modules or classes to reduce the size of bot.py and improve maintainability.
- Add validation for uploaded attachments (e.g., file size, mime type) to prevent users from sending excessively large or unsupported files.
- The admin orders list currently renders all items at once; consider implementing pagination or dynamic loading to manage large numbers of orders more effectively.
## Individual Comments
### Comment 1
<location> `bot.py:77-86` </location>
<code_context>
+def format_contact_link(contact: str) -> str:
</code_context>
<issue_to_address>
**suggestion:** Consider handling contacts with non-standard characters more robustly.
Telegram usernames may include numbers and are case-insensitive, but your current validation excludes some valid characters like dashes or periods. Consider updating the allowed character set or notifying users when their input is invalid.
</issue_to_address>
### Comment 2
<location> `bot.py:99-108` </location>
<code_context>
+def build_order_details(uid: str, order: dict) -> str:
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Escape upsell titles to prevent HTML injection.
Escape each upsell title individually before joining to avoid malformed output if titles contain HTML special characters.
Suggested implementation:
```python
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
upsells = order.get('upsells', [])
escaped_upsell_titles = [escape(str(title)) for title in upsells]
upsell_titles_str = ', '.join(escaped_upsell_titles) if escaped_upsell_titles else 'Нет'
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Упселлы: {upsell_titles_str}",
```
If the upsell titles are displayed elsewhere or if the upsells field is not a list of titles, you may need to adjust the extraction logic accordingly. Also, ensure that the `escape` function is imported from the correct module (e.g., `from html import escape`) at the top of the file if not already present.
</issue_to_address>
### Comment 3
<location> `bot.py:371-380` </location>
<code_context>
+ return UPLOAD_FILES
+
+async def skip_files(update: Update, context: ContextTypes.DEFAULT_TYPE):
+ context.user_data['current_files'] = []
+ await update.message.reply_text("Пропускаем прикрепление файлов.")
+ return await request_contact(update, context)
+
+async def finish_files(update: Update, context: ContextTypes.DEFAULT_TYPE):
</code_context>
<issue_to_address>
**issue (performance):** Potential for large file uploads without size or count limits.
Currently, there are no checks on file size or count, which may cause performance or storage problems. Please implement limits and notify users when these are exceeded.
</issue_to_address>
### Comment 4
<location> `bot.py:445-452` </location>
<code_context>
+ return INPUT_CONTACT
+
+async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
+ contact = update.message.text.strip()
+ if not contact:
+ await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.")
+ return INPUT_CONTACT
+ context.user_data['current_contact'] = contact
+ context.user_data['last_contact'] = contact
+ return await add_upsell(update, context)
+
# Добавление допуслуг
</code_context>
<issue_to_address>
**suggestion:** Contact field validation may be too permissive.
The current implementation accepts any non-empty string as contact information, which may allow invalid entries. Please implement validation for expected contact formats and notify users when their input is invalid.
```suggestion
import re
def is_valid_contact(contact: str) -> bool:
# Email
email_pattern = r"^[\w\.-]+@[\w\.-]+\.\w+$"
# Phone (simple international and local formats)
phone_pattern = r"^\+?\d{10,15}$"
# Telegram username
telegram_pattern = r"^@[\w\d_]{5,}$"
return (
re.match(email_pattern, contact)
or re.match(phone_pattern, contact)
or re.match(telegram_pattern, contact)
)
async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
contact = update.message.text.strip()
if not contact:
await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.")
return INPUT_CONTACT
if not is_valid_contact(contact):
await update.message.reply_text(
"Пожалуйста, укажите корректный контакт: email, номер телефона (10-15 цифр, можно с +), или Telegram username (@username)."
)
return INPUT_CONTACT
context.user_data['current_contact'] = contact
context.user_data['last_contact'] = contact
return await add_upsell(update, context)
```
</issue_to_address>
### Comment 5
<location> `bot.py:667` </location>
<code_context>
return await show_admin_menu(update, context)
# Обработчик админ-меню
-async def admin_menu_handler(update: Update, context: ContextTypes.DEFAULT_TYPE):
- query = update.callback_query
- await query.answer()
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring large handler cascades and file/contact flows into separate modules with dispatch maps and conversation handlers.
```markdown
You’ve done great work adding file-upload/contact flows and richer admin screens, but `bot.py` is now ~1 200 LOC with two huge `if/elif` cascades and even a duplicated `add_upsell()` definition. Two quick wins:
1) **Extract & dispatch admin callbacks**
Replace the big `admin_menu_handler` `if/elif` tree with a small dispatch map + per-action functions. E.g.:
```python
# admin.py
from telegram import InlineKeyboardMarkup, InlineKeyboardButton
from telegram.constants import ParseMode
async def list_orders(update, context):
query = update.callback_query
await query.answer()
buttons = []
for uid, orders in ORDERS.items():
for o in orders:
buttons.append([
InlineKeyboardButton(
f"#{o['order_id']} ({uid})",
callback_data=f"admin_order|{uid}|{o['order_id']}"
)
])
buttons.append([InlineKeyboardButton("Назад", callback_data="admin_menu")])
await query.edit_message_text(
"📋 Заказы:\n" + ("\n".join(f"#{o['order_id']} от {uid}: {o['status']}"
for uid, orders in ORDERS.items() for o in orders)
or "Пусто"),
reply_markup=InlineKeyboardMarkup(buttons)
)
return ADMIN_MENU
async def view_order(update, context, uid, oid):
query = update.callback_query
await query.answer()
order = next((o for o in ORDERS.get(uid, []) if str(o['order_id'])==oid), None)
text = build_order_details(uid, order) if order else "Заказ не найден."
kb = [[InlineKeyboardButton("⬅️ Назад", callback_data="admin_orders")]]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(kb), parse_mode=ParseMode.HTML)
return ADMIN_MENU
# … similarly: cancel_order(), delete_order()
ADMIN_ACTIONS = {
"admin_orders": list_orders,
"admin_order": view_order,
"admin_cancel": cancel_order,
"admin_delete": delete_order,
}
async def admin_menu_handler(update, context):
data = (await update.callback_query).data
key, *args = data.split("|")
handler = ADMIN_ACTIONS.get(key)
if handler:
return await handler(update, context, *args)
return await show_admin_menu(update, context)
```
In your main file just import `admin_menu_handler` and remove the big cascade.
2) **Move file-upload/contact steps into their own module**
Right now you have 6 functions in a row for UPLOAD_FILES→INPUT_CONTACT. Bundle them into a mini `ConversationHandler` in `file_flow.py` and register it separately:
```python
# file_flow.py
from telegram.ext import (
ConversationHandler, MessageHandler, CommandHandler, filters, CallbackQueryHandler
)
states = {
'UPLOAD_FILES': [ # use your UPLOAD_FILES constant
MessageHandler(filters.Document.ALL, handle_document_upload),
MessageHandler(filters.PHOTO, handle_photo_upload),
# …
],
'INPUT_CONTACT': [
MessageHandler(filters.TEXT & ~filters.COMMAND, input_contact),
],
}
file_upload_flow = ConversationHandler(
entry_points=[CallbackQueryHandler(prompt_file_upload, pattern='^add_upsell$')],
states=states,
fallbacks=[
CommandHandler('skip', skip_files),
CommandHandler('done', finish_files),
]
)
```
Then in your setup:
```python
from file_flow import file_upload_flow
application.add_handler(file_upload_flow)
```
3) **Remove the duplicate `add_upsell()`** – consolidate its logic into one place.
These steps will shrink `bot.py` by hundreds of lines, improve readability, and make each flow easier to test.
</issue_to_address>
### Comment 6
<location> `bot.py:634` </location>
<code_context>
f"Требования: {requirements if requirements else 'Нет'}",
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
f"Требования: {requirements or 'Нет'}",
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>
### Comment 7
<location> `bot.py:105` </location>
<code_context>
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Контакт: {format_contact_link(order.get('contact'))}",
f"Требования: {escape(order.get('requirements', 'Нет'))}",
]
if order.get('upsells'):
upsells_readable = ', '.join(UPSELL_TITLES.get(code, code) for code in order['upsells'])
lines.append(f"Допы: {escape(upsells_readable)}")
else:
lines.append("Допы: нет")
lines.append(f"Файлов: {len(order.get('attachments') or [])}")
if order.get('created_at'):
lines.append(f"Создан: {escape(order['created_at'])}")
return '<br>'.join(lines)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove unnecessary casts to int, str, float or bool ([`remove-unnecessary-cast`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-cast/))
```suggestion
f'Пользователь: <a href=\"{user_link}\">{escape(uid)}</a>',
```
</issue_to_address>
### Comment 8
<location> `bot.py:434-435` </location>
<code_context>
async def request_contact(update: Update, context: ContextTypes.DEFAULT_TYPE):
context.user_data.pop('current_contact', None)
prompt = (
"Укажите контакт, куда менеджеру написать (Telegram, ВКонтакте, почта). Это обязательное поле."
)
last_contact = context.user_data.get('last_contact')
if last_contact:
prompt += f"\nРанее вы указывали: {last_contact}. Можно отправить его снова или написать другой."
if update.message:
await update.message.reply_text(prompt)
elif update.callback_query:
query = update.callback_query
await query.answer()
await query.edit_message_text(prompt)
return INPUT_CONTACT
</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 last_contact := context.user_data.get('last_contact'):
```
</issue_to_address>
### Comment 9
<location> `bot.py:888` </location>
<code_context>
async def admin_menu_handler(update: Update, context: ContextTypes.DEFAULT_TYPE):
query = update.callback_query
await query.answer()
data = query.data
if data == 'admin_menu':
return await show_admin_menu(update, context)
if data == 'admin_orders':
text_lines = []
buttons = []
for uid, ords in ORDERS.items():
for ord_data in ords:
text_lines.append(f"#{ord_data.get('order_id', 'N/A')} от {uid}: {ord_data.get('status', 'новый')}")
buttons.append([
InlineKeyboardButton(
f"#{ord_data.get('order_id', 'N/A')} ({uid})",
callback_data=f"admin_order|{uid}|{ord_data.get('order_id', 'N/A')}"
)
])
if not text_lines:
text = "Заказы отсутствуют."
else:
text = "📋 Заказы:\n" + "\n".join(text_lines[:20])
buttons.append([InlineKeyboardButton("Назад", callback_data='admin_menu')])
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(buttons))
return ADMIN_MENU
if data.startswith('admin_order|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order = next((o for o in ORDERS.get(uid, []) if str(o.get('order_id')) == order_id_str), None)
if not order:
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
text = build_order_details(uid, order)
keyboard = [
[InlineKeyboardButton("Отменить заказ", callback_data=f'admin_cancel|{uid}|{order_id_str}')],
[InlineKeyboardButton("Удалить заказ", callback_data=f'admin_delete|{uid}|{order_id_str}')],
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard), parse_mode=ParseMode.HTML)
return ADMIN_MENU
if data.startswith('admin_cancel|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order = next((o for o in ORDERS.get(uid, []) if str(o.get('order_id')) == order_id_str), None)
if not order:
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
order['status'] = 'отменен'
save_json(ORDERS_FILE, ORDERS)
text = f"Статус заказа #{order_id_str} обновлен на 'отменен'."
keyboard = [
[InlineKeyboardButton("Посмотреть заказ", callback_data=f'admin_order|{uid}|{order_id_str}')],
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
if data.startswith('admin_delete|'):
try:
_, uid, order_id_str = data.split('|', 2)
except ValueError:
await query.edit_message_text("Некорректный идентификатор заказа.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
orders_list = ORDERS.get(uid, [])
new_list = [o for o in orders_list if str(o.get('order_id')) != order_id_str]
if len(new_list) == len(orders_list):
await query.edit_message_text("Заказ не найден.", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("Назад", callback_data='admin_orders')]]))
return ADMIN_MENU
if new_list:
ORDERS[uid] = new_list
else:
ORDERS.pop(uid, None)
save_json(ORDERS_FILE, ORDERS)
text = f"Заказ #{order_id_str} удален."
keyboard = [
[InlineKeyboardButton("⬅️ К списку", callback_data='admin_orders')],
[InlineKeyboardButton("Админ-меню", callback_data='admin_menu')]
]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
text = ""
keyboard = [[InlineKeyboardButton("Назад", callback_data='admin_menu')]]
if data == 'admin_users':
text = "👥 Пользователи:\n" + "\n".join(f"ID: {uid}" for uid in ORDERS.keys())
elif data == 'admin_logs':
text = "📊 Логи (последние 10):\n"
for uid, logs in list(USER_LOGS.items())[-10:]:
if logs:
text += f"Пользователь {uid}: {logs[-1]['action']}\n"
elif data == 'admin_prices':
text = f"Текущий режим: {current_pricing_mode}\nВведите новый режим (hard/light):"
context.user_data['admin_state'] = 'change_mode'
elif data == 'admin_export':
df = pd.DataFrame([{'user_id': uid, **ord} for uid, ords in ORDERS.items() for ord in ords])
export_file = os.path.join(DATA_DIR, 'orders_export.csv')
df.to_csv(export_file, index=False)
await context.bot.send_document(ADMIN_CHAT_ID, open(export_file, 'rb'))
os.remove(export_file)
text = "📤 Экспорт отправлен!"
elif data == 'back_to_main':
return await main_menu(update, context)
await query.edit_message_text(text or "Неизвестная команда. Возвращаюсь в админ-меню.", reply_markup=InlineKeyboardMarkup(keyboard))
return ADMIN_MENU
</code_context>
<issue_to_address>
**issue (code-quality):** Low code quality found in admin\_menu\_handler - 12% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+77
to
+86
| def format_contact_link(contact: str) -> str: | ||
| if not contact: | ||
| return 'не указан' | ||
| contact = contact.strip() | ||
| if not contact: | ||
| return 'не указан' | ||
| safe_display = escape(contact) | ||
| if contact.startswith('@') and len(contact) > 1: | ||
| username = contact[1:].split()[0] | ||
| if username and all(ch.isalnum() or ch == '_' for ch in username): |
There was a problem hiding this comment.
suggestion: Рассмотрите возможность более надежной обработки контактов с нестандартными символами.
Имена пользователей Telegram могут содержать цифры и нечувствительны к регистру, но ваша текущая проверка исключает некоторые допустимые символы, такие как дефисы или точки. Рассмотрите возможность обновления разрешенного набора символов или уведомления пользователей, если их ввод недействителен.
Original comment in English
suggestion: Consider handling contacts with non-standard characters more robustly.
Telegram usernames may include numbers and are case-insensitive, but your current validation excludes some valid characters like dashes or periods. Consider updating the allowed character set or notifying users when their input is invalid.
Comment on lines
+99
to
+108
| def build_order_details(uid: str, order: dict) -> str: | ||
| order_id = order.get('order_id', 'N/A') | ||
| order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно')) | ||
| user_link = escape(f"tg://user?id={uid}", quote=True) | ||
| lines = [ | ||
| f"<b>Заказ #{order_id}</b>", | ||
| f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>", | ||
| f"Тип: {escape(order_name)}", | ||
| f"Тема: {escape(order.get('topic', 'Без темы'))}", | ||
| f"Срок: {order.get('deadline_days', 'N/A')} дней", |
There was a problem hiding this comment.
🚨 suggestion (security): Экранируйте заголовки допродаж, чтобы предотвратить внедрение HTML-кода.
Экранируйте каждый заголовок допродажи по отдельности перед объединением, чтобы избежать некорректного вывода, если заголовки содержат специальные символы HTML.
Предлагаемая реализация:
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
upsells = order.get('upsells', [])
escaped_upsell_titles = [escape(str(title)) for title in upsells]
upsell_titles_str = ', '.join(escaped_upsell_titles) if escaped_upsell_titles else 'Нет'
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Упселлы: {upsell_titles_str}",Если заголовки допродаж отображаются в других местах или если поле допродаж не является списком заголовков, вам может потребоваться соответствующая корректировка логики извлечения. Также убедитесь, что функция escape импортирована из правильного модуля (например, from html import escape) в начале файла, если она еще не присутствует.
Original comment in English
🚨 suggestion (security): Escape upsell titles to prevent HTML injection.
Escape each upsell title individually before joining to avoid malformed output if titles contain HTML special characters.
Suggested implementation:
def build_order_details(uid: str, order: dict) -> str:
order_id = order.get('order_id', 'N/A')
order_name = ORDER_TYPES.get(order.get('type'), {}).get('name', order.get('type', 'Неизвестно'))
user_link = escape(f"tg://user?id={uid}", quote=True)
upsells = order.get('upsells', [])
escaped_upsell_titles = [escape(str(title)) for title in upsells]
upsell_titles_str = ', '.join(escaped_upsell_titles) if escaped_upsell_titles else 'Нет'
lines = [
f"<b>Заказ #{order_id}</b>",
f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>",
f"Тип: {escape(order_name)}",
f"Тема: {escape(order.get('topic', 'Без темы'))}",
f"Срок: {order.get('deadline_days', 'N/A')} дней",
f"Статус: {escape(order.get('status', 'неизвестно'))}",
f"Упселлы: {upsell_titles_str}",If the upsell titles are displayed elsewhere or if the upsells field is not a list of titles, you may need to adjust the extraction logic accordingly. Also, ensure that the escape function is imported from the correct module (e.g., from html import escape) at the top of the file if not already present.
Comment on lines
+371
to
+380
| context.user_data['current_files'] = [] | ||
| context.user_data.pop('current_contact', None) | ||
| text = ( | ||
| "Прикрепите файлы для заказа (если они есть). Отправьте все документы подряд.\n" | ||
| "Когда закончите, нажмите /done. Если файлов нет, нажмите /skip." | ||
| ) | ||
| if update.message: | ||
| await update.message.reply_text(text) | ||
| elif update.callback_query: | ||
| query = update.callback_query |
There was a problem hiding this comment.
issue (performance): Возможность загрузки больших файлов без ограничений по размеру или количеству.
В настоящее время отсутствуют проверки размера или количества файлов, что может привести к проблемам с производительностью или хранением. Пожалуйста, реализуйте ограничения и уведомляйте пользователей о их превышении.
Original comment in English
issue (performance): Potential for large file uploads without size or count limits.
Currently, there are no checks on file size or count, which may cause performance or storage problems. Please implement limits and notify users when these are exceeded.
Comment on lines
+445
to
+452
| async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE): | ||
| contact = update.message.text.strip() | ||
| if not contact: | ||
| await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.") | ||
| return INPUT_CONTACT | ||
| context.user_data['current_contact'] = contact | ||
| context.user_data['last_contact'] = contact | ||
| return await add_upsell(update, context) |
There was a problem hiding this comment.
suggestion: Проверка поля контакта может быть слишком свободной.
Текущая реализация принимает любую непустую строку в качестве контактной информации, что может допускать неверные записи. Пожалуйста, реализуйте проверку ожидаемых форматов контактов и уведомляйте пользователей, когда их ввод недействителен.
Suggested change
| async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE): | |
| contact = update.message.text.strip() | |
| if not contact: | |
| await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.") | |
| return INPUT_CONTACT | |
| context.user_data['current_contact'] = contact | |
| context.user_data['last_contact'] = contact | |
| return await add_upsell(update, context) | |
| import re | |
| def is_valid_contact(contact: str) -> bool: | |
| email_pattern = r"^[\w\.-]+@[\w\.-]+\.\w+$" | |
| # Phone (simple international and local formats) | |
| phone_pattern = r"^\+?\d{10,15}$" | |
| # Telegram username | |
| telegram_pattern = r"^@[\w\d_]{5,}$" | |
| return ( | |
| re.match(email_pattern, contact) | |
| or re.match(phone_pattern, contact) | |
| or re.match(telegram_pattern, contact) | |
| ) | |
| async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE): | |
| contact = update.message.text.strip() | |
| if not contact: | |
| await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.") | |
| return INPUT_CONTACT | |
| if not is_valid_contact(contact): | |
| await update.message.reply_text( | |
| "Пожалуйста, укажите корректный контакт: email, номер телефона (10-15 цифр, можно с +), или Telegram username (@username)." | |
| ) | |
| return INPUT_CONTACT | |
| context.user_data['current_contact'] = contact | |
| context.user_data['last_contact'] = contact | |
| return await add_upsell(update, context) |
Original comment in English
suggestion: Contact field validation may be too permissive.
The current implementation accepts any non-empty string as contact information, which may allow invalid entries. Please implement validation for expected contact formats and notify users when their input is invalid.
Suggested change
| async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE): | |
| contact = update.message.text.strip() | |
| if not contact: | |
| await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.") | |
| return INPUT_CONTACT | |
| context.user_data['current_contact'] = contact | |
| context.user_data['last_contact'] = contact | |
| return await add_upsell(update, context) | |
| import re | |
| def is_valid_contact(contact: str) -> bool: | |
| email_pattern = r"^[\w\.-]+@[\w\.-]+\.\w+$" | |
| # Phone (simple international and local formats) | |
| phone_pattern = r"^\+?\d{10,15}$" | |
| # Telegram username | |
| telegram_pattern = r"^@[\w\d_]{5,}$" | |
| return ( | |
| re.match(email_pattern, contact) | |
| or re.match(phone_pattern, contact) | |
| or re.match(telegram_pattern, contact) | |
| ) | |
| async def input_contact(update: Update, context: ContextTypes.DEFAULT_TYPE): | |
| contact = update.message.text.strip() | |
| if not contact: | |
| await update.message.reply_text("Контакт обязателен. Пожалуйста, укажите, куда менеджеру написать.") | |
| return INPUT_CONTACT | |
| if not is_valid_contact(contact): | |
| await update.message.reply_text( | |
| "Пожалуйста, укажите корректный контакт: email, номер телефона (10-15 цифр, можно с +), или Telegram username (@username)." | |
| ) | |
| return INPUT_CONTACT | |
| context.user_data['current_contact'] = contact | |
| context.user_data['last_contact'] = contact | |
| return await add_upsell(update, context) |
| caption=caption, | ||
| parse_mode=ParseMode.HTML | ||
| ) | ||
| else: |
There was a problem hiding this comment.
issue (complexity): Рассмотрите возможность рефакторинга больших каскадов обработчиков и потоков файлов/контактов в отдельные модули с картами диспетчеризации и обработчиками диалогов.
You’ve done great work adding file-upload/contact flows and richer admin screens, but `bot.py` is now ~1 200 LOC with two huge `if/elif` cascades and even a duplicated `add_upsell()` definition. Two quick wins:
1) **Extract & dispatch admin callbacks**
Replace the big `admin_menu_handler` `if/elif` tree with a small dispatch map + per-action functions. E.g.:
```python
# admin.py
from telegram import InlineKeyboardMarkup, InlineKeyboardButton
from telegram.constants import ParseMode
async def list_orders(update, context):
query = update.callback_query
await query.answer()
buttons = []
for uid, orders in ORDERS.items():
for o in orders:
buttons.append([
InlineKeyboardButton(
f"#{o['order_id']} ({uid})",
callback_data=f"admin_order|{uid}|{o['order_id']}"
)
])
buttons.append([InlineKeyboardButton("Назад", callback_data="admin_menu")])
await query.edit_message_text(
"📋 Заказы:\n" + ("\n".join(f"#{o['order_id']} от {uid}: {o['status']}"
for uid, orders in ORDERS.items() for o in orders)
or "Пусто"),
reply_markup=InlineKeyboardMarkup(buttons)
)
return ADMIN_MENU
async def view_order(update, context, uid, oid):
query = update.callback_query
await query.answer()
order = next((o for o in ORDERS.get(uid, []) if str(o['order_id'])==oid), None)
text = build_order_details(uid, order) if order else "Заказ не найден."
kb = [[InlineKeyboardButton("⬅️ Назад", callback_data="admin_orders")]]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(kb), parse_mode=ParseMode.HTML)
return ADMIN_MENU
# … similarly: cancel_order(), delete_order()
ADMIN_ACTIONS = {
"admin_orders": list_orders,
"admin_order": view_order,
"admin_cancel": cancel_order,
"admin_delete": delete_order,
}
async def admin_menu_handler(update, context):
data = (await update.callback_query).data
key, *args = data.split("|")
handler = ADMIN_ACTIONS.get(key)
if handler:
return await handler(update, context, *args)
return await show_admin_menu(update, context)In your main file just import admin_menu_handler and remove the big cascade.
-
Move file-upload/contact steps into their own module
Right now you have 6 functions in a row for UPLOAD_FILES→INPUT_CONTACT. Bundle them into a miniConversationHandlerinfile_flow.pyand register it separately:# file_flow.py from telegram.ext import ( ConversationHandler, MessageHandler, CommandHandler, filters, CallbackQueryHandler ) states = { 'UPLOAD_FILES': [ # use your UPLOAD_FILES constant MessageHandler(filters.Document.ALL, handle_document_upload), MessageHandler(filters.PHOTO, handle_photo_upload), # … ], 'INPUT_CONTACT': [ MessageHandler(filters.TEXT & ~filters.COMMAND, input_contact), ], } file_upload_flow = ConversationHandler( entry_points=[CallbackQueryHandler(prompt_file_upload, pattern='^add_upsell$')], states=states, fallbacks=[ CommandHandler('skip', skip_files), CommandHandler('done', finish_files), ] )
Then in your setup:
from file_flow import file_upload_flow application.add_handler(file_upload_flow)
-
Remove the duplicate
add_upsell()– consolidate its logic into one place.
These steps will shrink bot.py by hundreds of lines, improve readability, and make each flow easier to test.
Original comment in English
issue (complexity): Consider refactoring large handler cascades and file/contact flows into separate modules with dispatch maps and conversation handlers.
You’ve done great work adding file-upload/contact flows and richer admin screens, but `bot.py` is now ~1 200 LOC with two huge `if/elif` cascades and even a duplicated `add_upsell()` definition. Two quick wins:
1) **Extract & dispatch admin callbacks**
Replace the big `admin_menu_handler` `if/elif` tree with a small dispatch map + per-action functions. E.g.:
```python
# admin.py
from telegram import InlineKeyboardMarkup, InlineKeyboardButton
from telegram.constants import ParseMode
async def list_orders(update, context):
query = update.callback_query
await query.answer()
buttons = []
for uid, orders in ORDERS.items():
for o in orders:
buttons.append([
InlineKeyboardButton(
f"#{o['order_id']} ({uid})",
callback_data=f"admin_order|{uid}|{o['order_id']}"
)
])
buttons.append([InlineKeyboardButton("Назад", callback_data="admin_menu")])
await query.edit_message_text(
"📋 Заказы:\n" + ("\n".join(f"#{o['order_id']} от {uid}: {o['status']}"
for uid, orders in ORDERS.items() for o in orders)
or "Пусто"),
reply_markup=InlineKeyboardMarkup(buttons)
)
return ADMIN_MENU
async def view_order(update, context, uid, oid):
query = update.callback_query
await query.answer()
order = next((o for o in ORDERS.get(uid, []) if str(o['order_id'])==oid), None)
text = build_order_details(uid, order) if order else "Заказ не найден."
kb = [[InlineKeyboardButton("⬅️ Назад", callback_data="admin_orders")]]
await query.edit_message_text(text, reply_markup=InlineKeyboardMarkup(kb), parse_mode=ParseMode.HTML)
return ADMIN_MENU
# … similarly: cancel_order(), delete_order()
ADMIN_ACTIONS = {
"admin_orders": list_orders,
"admin_order": view_order,
"admin_cancel": cancel_order,
"admin_delete": delete_order,
}
async def admin_menu_handler(update, context):
data = (await update.callback_query).data
key, *args = data.split("|")
handler = ADMIN_ACTIONS.get(key)
if handler:
return await handler(update, context, *args)
return await show_admin_menu(update, context)In your main file just import admin_menu_handler and remove the big cascade.
-
Move file-upload/contact steps into their own module
Right now you have 6 functions in a row for UPLOAD_FILES→INPUT_CONTACT. Bundle them into a miniConversationHandlerinfile_flow.pyand register it separately:# file_flow.py from telegram.ext import ( ConversationHandler, MessageHandler, CommandHandler, filters, CallbackQueryHandler ) states = { 'UPLOAD_FILES': [ # use your UPLOAD_FILES constant MessageHandler(filters.Document.ALL, handle_document_upload), MessageHandler(filters.PHOTO, handle_photo_upload), # … ], 'INPUT_CONTACT': [ MessageHandler(filters.TEXT & ~filters.COMMAND, input_contact), ], } file_upload_flow = ConversationHandler( entry_points=[CallbackQueryHandler(prompt_file_upload, pattern='^add_upsell$')], states=states, fallbacks=[ CommandHandler('skip', skip_files), CommandHandler('done', finish_files), ] )
Then in your setup:
from file_flow import file_upload_flow application.add_handler(file_upload_flow)
-
Remove the duplicate
add_upsell()– consolidate its logic into one place.
These steps will shrink bot.py by hundreds of lines, improve readability, and make each flow easier to test.
| f"<b>#{order['order_id']}</b> {escape(order_name)} — {topic}", | ||
| f"Срок: {order.get('deadline_days', 'N/A')} дней | Цена: {order.get('price', 0)} ₽", | ||
| f"Контакт: {contact_html}", | ||
| f"Требования: {requirements if requirements else 'Нет'}", |
There was a problem hiding this comment.
suggestion (code-quality): Замените if-выражение на or (or-if-exp-identity)
Suggested change
| f"Требования: {requirements if requirements else 'Нет'}", | |
| f"Требования: {requirements or 'Нет'}", |
Explanation
Здесь мы устанавливаем значение, если оно оценивается какTrue, в противном случаеиспользуя значение по умолчанию.
Случай 'После' немного легче читается и позволяет избежать дублирования
input_currency.
Это работает, потому что левая часть оценивается первой. Если она оценивается как
истинная, то currency будет установлено в это значение, а правая часть не будет
оцениваться. Если она оценивается как ложная, то правая часть будет оцениваться, и
currency будет установлено в DEFAULT_CURRENCY.
Original comment in English
suggestion (code-quality): Replace if-expression with or (or-if-exp-identity)
Suggested change
| f"Требования: {requirements if requirements else 'Нет'}", | |
| f"Требования: {requirements or 'Нет'}", |
Explanation
Here we find ourselves setting a value if it evaluates toTrue, and otherwiseusing a default.
The 'After' case is a bit easier to read and avoids the duplication of
input_currency.
It works because the left-hand side is evaluated first. If it evaluates to
true then currency will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
currency will be set to DEFAULT_CURRENCY.
| user_link = escape(f"tg://user?id={uid}", quote=True) | ||
| lines = [ | ||
| f"<b>Заказ #{order_id}</b>", | ||
| f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>", |
There was a problem hiding this comment.
suggestion (code-quality): Удалите ненужные приведения к int, str, float или bool (remove-unnecessary-cast)
Suggested change
| f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>", | |
| f'Пользователь: <a href=\"{user_link}\">{escape(uid)}</a>', |
Original comment in English
suggestion (code-quality): Remove unnecessary casts to int, str, float or bool (remove-unnecessary-cast)
Suggested change
| f"Пользователь: <a href=\"{user_link}\">{escape(str(uid))}</a>", | |
| f'Пользователь: <a href=\"{user_link}\">{escape(uid)}</a>', |
Comment on lines
+434
to
+435
| last_contact = context.user_data.get('last_contact') | ||
| if last_contact: |
There was a problem hiding this comment.
suggestion (code-quality): Используйте именованные выражения для упрощения присваивания и условного оператора (use-named-expression)
Suggested change
| last_contact = context.user_data.get('last_contact') | |
| if last_contact: | |
| if last_contact := context.user_data.get('last_contact'): |
Original comment in English
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
Suggested change
| last_contact = context.user_data.get('last_contact') | |
| if last_contact: | |
| if last_contact := context.user_data.get('last_contact'): |
| text += f"#{ord['order_id']} от {uid}: {ord['status']}\n" | ||
| elif data == 'admin_users': | ||
| text = "👥 Пользователи:\n" + "\n".join(f"ID: {uid}" for uid in ORDERS.keys()) | ||
| async def admin_menu_handler(update: Update, context: ContextTypes.DEFAULT_TYPE): |
There was a problem hiding this comment.
issue (code-quality): Низкое качество кода обнаружено в admin_menu_handler - 12% (low-code-quality)
Explanation
Оценка качества для этой функции ниже порогового значения качества в 25%.Эта оценка представляет собой комбинацию длины метода, когнитивной сложности и рабочей памяти.
Как вы можете это решить?
Возможно, стоит провести рефакторинг этой функции, чтобы сделать ее короче и читабельнее.
- Уменьшите длину функции, извлекая части функциональности в
отдельные функции. Это самое важное, что вы можете сделать — в идеале функция
должна быть менее 10 строк. - Уменьшите вложенность, возможно, путем введения защитных условий для раннего выхода.
- Убедитесь, что переменные имеют строго ограниченную область видимости, чтобы код, использующий связанные концепции,
находился вместе внутри функции, а не был разбросан.
Original comment in English
issue (code-quality): Low code quality found in admin_menu_handler - 12% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
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_68c98bc7f7b0832faa80ff9d9e92bc1e
Сводка от Sourcery
Обновить процесс оформления заказа, чтобы требовать контактную информацию клиента, добавить поддержку вложений файлов и улучшить возможности администратора по управлению заказами.
Новые функции:
Улучшения:
Original summary in English
Summary by Sourcery
Update the order flow to require client contact information, support file attachments, and enhance admin order management capabilities.
New Features:
Enhancements: