Refactoring send_notification and change_subscription#76
Refactoring send_notification and change_subscription#76JediMode wants to merge 3 commits intoProCharity:developfrom
Conversation
| logger.info(f"Messages: Passed invalid <has_mailing> parameter. Passed: {has_mailing}") | ||
| return make_response(jsonify(result=f"Неверно указан параметр <has_mailing>. " | ||
| f"Сообщение не отправлено."), 400) | ||
| notifier = TelegramNotification() |
There was a problem hiding this comment.
Писал прошлый раз по этому, но давай ещё раз переименуем, в название класса и переменной довай добавим слово сервис.
|
|
||
| # TODO refactoring https://github.com/python-telegram-bot/python-telegram-bot/wiki/Avoiding-flood-limits | ||
| def send_notification(self, message): | ||
| def send_notification(self, mailing_type, message): |
There was a problem hiding this comment.
Другой порядок, сперва сообщение, потом тип. сообщение важнее чем тип.!
| class TelegramNotificationRequest(RequestBase): | ||
| message: StrictStr = Field() | ||
| mailing_type: MailingType = MailingType.subscribed | ||
| mailing_number: MailingNumber = MailingNumber.subscribed |
There was a problem hiding this comment.
Это по документации если при входе попадет значение не из трех значений 'subscribed', 'unsubscribed', 'all', то выдаст ошибку
https://docs.pydantic.dev/usage/types/#enums-and-choices
There was a problem hiding this comment.
не, я про саму переменную, зачем нам нужна mailing_number? Как её использовать и кому?
| return make_response(jsonify(result=f"Неверно указан параметр <has_mailing>. " | ||
| f"Сообщение не отправлено."), 400) | ||
| notifier = TelegramNotification() | ||
| notifier.send_notification(mailing_type=notifications, message=message.message) |
There was a problem hiding this comment.
А тут что происходит? mailing_type=notifications ты в поле ожидающее получить enum передаёшь объект с различнми полями. Подозреваю, ЭТО НЕ РАБОТАЕТ! Перед отправкой на следующее ревью, тебе нужно прогнать 5 кейсов тестирования различного положения флага и убедиться, что всё работает.
И в месте где проверяешь тип, if mailing_type == MailingType.subscribed.value:, везде добавь else, а в самом конце, если это не одно из трёх значений, бросай исключение! Как раз по этому у тебя и отработало всё без ошибок.
2 participants
Изменил send_notification и change_subscription.