Письмо через 3 дня после покупки записи

[tough-dev-school]

Письмо через 3 дня после покупки записи

[grepto]

Посмотрел в проект в части триггеров

Что понял

1. Триггеры лежат в pmdaily-backend/src/triggers/
2. Триггер - это объект класса, который наследуется от базового класса BaseTrigger
3. Триггер содержит в себе условие срабатывания в .condition() и действие в .send(). Выполнение действия происходит за счет переопределения мэджик метода __call__ в BaseTrigger
4. Триггеры запускаются посредством задач celery в tasks.py. Я пока не нашел где задана частота дерганья задач и нужно ли прописывать названия регулярных задач, но пока мне это не нужно так как сперва я хочу разобраться с тестами и классом для нового триггера.

Что планирую делать

1. Написать тесты по аналогии с тестами для догонялки 30.12.2019:

• Проверка на возраст заказа
• Проверка на оплаченность заказа
• Проверка на содержимое заказа
• Проверка вызова метода отправки
• Проверка содержимого письма (контекста)
• Проверка на статус оплаты

2. Написать класс для нового триггера 06.01.2020
3. Написать celery задачи для работы триггера 06.01.2020

Указал дедлайны с запасом на затупы, чтения документации и дополнительные вопросы. Скорее всего, сдавать буду раньше.

Вопросы

1. Нужно ли учитывать дополнительные условия для покупателя (покупал ли он до этого курс, запись или бандл; слали ли ему другие письма по триггерам и пр.)?

[f213]

пока не нашел где задана частота дерганья задач

Расписание периодических тасков задаётся в settings.py

Нужно ли учитывать дополнительные условия для покупателя (покупал ли он до этого курс, запись или бандл; слали ли ему другие письма по триггерам и пр.)?

А как бы ты сам ответил?

[grepto]

А как бы ты сам ответил?

Сложно без статистики верный сценарий выбрать. Я бы предложил вариант - слать письмо, если в течении месяца других писем с этого триггера не было.

Однако, если ты потом еще решишь сделать триггер на покупку курса или бандла - те кто купят за один раз (несколькими заказами) курс и запись другого курса, получат 2 письма с одной сутью - тоже не очень хорошо. Но пока второго триггера не планируется - 1 письмо в месяц (30 дней)

[grepto]

Я могу новый фичебранч в этом репозитории делать и потом пулреквест или в своем форке делать, а потом из него пулреквестить?

[f213]

слать письмо, если в течении месяца других писем с этого триггера не было.

Это уже реализовано для всех триггеров. Как бы ты поступил с учётом этой вводной?

Я могу новый фичебранч в этом репозитории делать и потом пулреквест или в своем форке делать, а потом из него пулреквестить?

Нужно делать бранч тут, иначе у тебя скорее всего CI не будет работать.

[grepto]

Это уже реализовано для всех триггеров. Как бы ты поступил с учётом этой вводной?
Метод is_sent более строгий чем я описал. Когда я куплю запись в первый раз - я получу письмо. Если я куплю другую запись на тот же email я не получу письма независимо от того когда я это сделаю - завтра или через год.

Таким образом, если я правильно понял, ты сможешь получить обратную связь и напомнить про скачивание, только для первой купленной записи (исходим из предположения, что большинство пользователей покупают записи не вводя каждый раз новый email)

С учетом этого, condition должен быть примерно таким:

def condition(self):
    """Order should be paid and was created more then two days ago (safety)
    """
    return self.order.paid and \
        self.order.record and \
        self._is_created_recently() and \

_is_created_recently будет таким

def _is_created_recently(self) -> bool:
    return timezone.now() - self.PERIOD > self.order.created > timezone.now() - self.PERIOD * 3

[grepto]

Весь вчерашний день я провел в попытках запустить тесты: разбирался с требуемыми переменными окружения. Все тесты пока так и не смог запустить, но
~ pytest src/triggers/tests/started_purchase_trigger
в итоге выполнились

[f213]

Весь вчерашний день я провел в попытках запустить тесты:

Вообще CI делают именно для того, чтобы не парится с тестами — у нас всегда есть референсный набор условий, при которых проект работает. Все дефолтные переменные окружения можно посмотреть тут и тут. Кстати, вторая ссылка — это вообще источник знаний о том, в какой среде и как разворачивать проект: мы же это делаем на каждый коммит.

[f213]

Таким образом, если я правильно понял, ты сможешь получить обратную связь и напомнить про скачивание, только для первой купленной записи

Именно так — более сложный триггер имеет смысл делать только после того, как мы убедимся, что юзеры готовы отвечать на это письмо. Иначе мы рискуем оказаться в ситуации, когда отрефакторим триггеры, напишем кучу кода, а потом выяснится, что всем этим письма пофиг.

[grepto]

Я закоммитил новый триггер, задачу и первые тесты в новую ветку.

У меня сомнение в корректности реализации метода _is_created_recently. Сейчас он возвращает True для заказов созданных в период от 3 до 5 дней назад.

В ситуации с триггером-догонялкой для неоплативших заказ, допустимый период ограничен одним днем, так как значение константы PERIOD, в том случае, установлено в 1 и это позволяет получить ограничение в 2 дня, просто умножив таймдельту.

С учетом настроек выборки подходящих заказов, нужно ли делать так чтобы _is_created_recently возвращал True для заказов которые старше текущего времени строго на три дня?

[f213]

Я закоммитил новый триггер, задачу и первые тесты в новую ветку.

Сделай из неё пожалуйста пулл-реквест, чтобы я мог комментить код.

С учетом настроек выборки подходящих заказов, нужно ли делать так чтобы _is_created_recently возвращал True для заказов которые старше текущего времени строго на три дня?

А зачем? Чем от этого будет лучше пользователям или нам?

[f213]

Выкатил в прод, прописал правильный шаблон. Запилил тестовый заказ, мне через 3 дня должно упасть письмо. Дополнительно, прослежу за мейлджетом, что уходит недавно купившим юзерам.

Немного с опозданием, но я подумал ещё про код — меня смущает название RecordPurchaseTrigger, оно слишком общее. К примеру, если мы потом придумаем ещё какую-нибудь догонялку для тех, кто купил запись, имея название RecordPurchaseTrigger будет сложно придумать что-нибудь ещё говорящее.

Давай назовём триггер RecordFeedbackTrigger?

[grepto]

Давай назовём триггер RecordFeedbackTrigger?

Не проблема. Я переназову. После мёржа ПР для этого изменения новый делать? В новой ветке и новым пул-реквестом делать?

[f213]

В новой ветке и новым пул-реквестом делать?

Я бы сделал именно так. Но это неважно — главное, чтобы у нас с тобой было отдельное место, где я мог бы посмотреть и покомментировать код. Как ты его сделаешь — тебе решать :-)

[f213]

@grepto я нашёл причину, почему твой код не работает.

В триггере мы обрабатываем заказы возрастом от трех до шести дней, а в таске выбираем заказы возрастом ДО трёх дней.

Избежать такой фигни в будущем можно либо через интеграционные тесты (до чего мы ещё не доросли), либо сделав единый базовый триггер — так мы добьёмся, что возраст заказа, с которым работает триггер, будет задаваться в одном месте — в самом триггере, а не в двух, как сейчас.

[grepto]

@grepto я нашёл причину, почему твой код не работает.

@f213 я поправил возраст выбираемых заказов в таске. Но чтобы не было каши из пулреквестов в одной ветке, хочу подождать с этим фиксом пока ты не смержишь ПР #27 (и прод не упадет). После этого я сделаю новый ПР для исправления даты в задаче. Ок?

Избежать такой фигни в будущем можно либо через интеграционные тесты (до чего мы ещё не доросли), либо сделав единый базовый триггер — так мы добьёмся, что возраст заказа, с которым работает триггер, будет задаваться в одном месте — в самом триггере, а не в двух, как сейчас.

Предположу что ты имел ввиду единую базовую таску, так как базовый тригер уже есть и внем вроде как не нужно задавать общий возраст заказов, так как требования к срокам отправки того или иного письма могут быть разные. Если так - я сделал отдельную ишью #24 для единого таска. Займусь когда на проде нормально взлетит тригер после покупки.

[f213]

Смёрджил

[f213]

Кстати, чтобы в таких случаях не было каши — можно просто отнаследовать новую ветку от той ветки, что ты сделал для пуллреквеста, и всё будет ок. Гитхаб в интерфейсе это разберёт и покажет только релевантные изменения

[f213]

Я наконец-то проверил — сейчас триггер на проде работает ок:

На кривой сабж не смотри — я его поправил в шаблоне. Думаю, эту задачу можно закрывать, а разработку — продолжать.

[f213]

Проверил, на проде всё отлично!