LPgenerator / django-db-mailer

Django module to easily send emails/sms/tts/push using django templates stored on database and managed through the Django Admin
https://github.com/LPgenerator/django-db-mailer
GNU General Public License v2.0
256 stars 80 forks source link

Ошибка при создание шаблона #2

Closed sergio-bershadsky closed 9 years ago

sergio-bershadsky commented 9 years ago

Если ранее был создан шаблон с slug=None то класс MailTemplateAdmin ломается до след. перезапуска. Ошибка кроется скорей всего тут, позже покурю и пришлю пул реквест:

    def get_form(self, request, obj=None, **kwargs):
        if obj is not None:
            self.prepopulated_fields = {}
            if defaults.READ_ONLY_ENABLED:
                self.readonly_fields = ['slug', 'context_note']
        else:
            self.prepopulated_fields = {'slug': ('name',)}

        return super(MailTemplateAdmin, self).get_form(
            request, obj=None, **kwargs)

Вообще сама идея что-то менять в рантайме у ModelAdmin крайне опасная затея, так как в райнтайме используется всегда один экземпляр данного класса. Класс инстанциируется один раз и больше новых экземпляров не будет.

gotlium commented 9 years ago

Я объясню почему в рантайме. Если посмотреть внимательно настройки, то можно увидеть что эти поля отключаются для редактировании после того как запись создана. Для чего? Если вдруг по своей глупости Вася или Петя, что работают в другом отделе вдруг случайно решат изменить slug и сломают критически важный шаблон для отправки почты. В итоге страдают все. В настройках имеется соответствующая константа. Жду пул реквеста :) Спасибо.

PS - Кстати в модели можно валидировать при clean или валидатор к полю добавить. Что бы нельзя было вытворить нечто подобное в консольке питона.

sergio-bershadsky commented 9 years ago

Зачем это понятно. Это надо решать на уровне отображение формы. Завтра пришлю. А то все ломается.

gotlium commented 9 years ago

Честно говоря пока еще подобных проблем не наблюдалось ни в одном проекте. Sentry на этот счет тоже молчит. Иначе бы починил уже. Каким образом создавался slug со значением None?

sergio-bershadsky commented 9 years ago

да вот странно как-то я его ввел свой слаг, нажал сохранить и сохранилось с None я еще не разбирал, ясно одно что потом ломается класс админ-модели.

sergio-bershadsky commented 9 years ago
Traceback:
File "***/python2.7/site-packages/django/core/handlers/base.py" in get_response
  111.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "***/python2.7/site-packages/django/contrib/admin/options.py" in wrapper
  583.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "***/python2.7/site-packages/django/utils/decorators.py" in _wrapped_view
  105.                     response = view_func(request, *args, **kwargs)
File "***/python2.7/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  52.         response = view_func(request, *args, **kwargs)
File "***/python2.7/site-packages/django/contrib/admin/sites.py" in inner
  206.             return view(request, *args, **kwargs)
File "***/python2.7/site-packages/django/contrib/admin/options.py" in add_view
  1453.         return self.changeform_view(request, None, form_url, extra_context)
File "***/python2.7/site-packages/django/utils/decorators.py" in _wrapper
  29.             return bound_func(*args, **kwargs)
File "***/python2.7/site-packages/django/utils/decorators.py" in _wrapped_view
  105.                     response = view_func(request, *args, **kwargs)
File "***/python2.7/site-packages/django/utils/decorators.py" in bound_func
  25.                 return func.__get__(self, type(self))(*args2, **kwargs2)
File "***/python2.7/site-packages/django/db/transaction.py" in inner
  394.                 return func(*args, **kwargs)
File "***/python2.7/site-packages/django/contrib/admin/options.py" in changeform_view
  1427.             model_admin=self)
File "***/python2.7/site-packages/django/contrib/admin/helpers.py" in __init__
  36.         } for field_name, dependencies in prepopulated_fields.items()]
File "***/python2.7/site-packages/django/forms/forms.py" in __getitem__
  147.                 "Key %r not found in '%s'" % (name, self.__class__.__name__))

Exception Type: KeyError at /admin/dbmail/mailtemplate/add/
Exception Value: u"Key 'slug' not found in 'MailTemplateForm'"
gotlium commented 9 years ago

Смог воспроизвести ошибку. Помню что в самом начале использовал get_prepopulated_fields и get_readonly_fields, но отказался, из-за того что в версиях ниже 1.7 эти методы не поддерживаются. Сейчас поправил ошибку. Проверяйте.

sergio-bershadsky commented 9 years ago

Надо было править начиная со 140 строки, пул реквест отправил. Только травис ругается почему-то

sergio-bershadsky commented 9 years ago

В любом случае, так как было, подменять нельзя. У кого ниже 1.7 пусть ручками вбивают слаг.

gotlium commented 9 years ago

Надо было ответвляться от development. Лучше сразу покрыть этот момент тестами. Раз уже наткнулись, что бы в дальнейшем не натыкаться на те же грабли. Ошибку воспроизводил таким образом:

  1. Создал новый шаблон
  2. Перешел к редактированию ранее созданного шаблона
  3. Удалил созданный шаблон
  4. Нажал добавить новый шаблон Смысл в том, что прежде я при редактировании он добавляет переменные readonly, а после форма при добавлении не может получить 2 поля, включая slug. Кстати можно этот же тест применить к MailGroup. Там так же реализовано подобное.
sergio-bershadsky commented 9 years ago

Так какие дальнейшия действия? Мне перепулить? Или сам поправишь ?

gotlium commented 9 years ago

Давай лучше перепулишь в ветку development. За одно напиuи тестики в файлике test_cases.py. Будем карпеть над той веткой. После вольем. Кстати новые фитчи что ты хочешь, лучше писать так же с development в отдельной ветке.

sergio-bershadsky commented 9 years ago

5 мин

sergio-bershadsky commented 9 years ago

У меня есть сомнения что тест кейс покроет этот случай, нудно делать LiveTest чтобы удостовериться что в цикле тестирования используется один и тот же экземпляр админ-модели

gotlium commented 9 years ago

Это можно легко проверить используя старый код и новый. Сразу выявиться) Главное что бы один клиент был. Если не получится, то бог с ним. Главное функционал покрыть. Если писать плагины.

sergio-bershadsky commented 9 years ago

Тревис все равно ругается, добавил в ридми инфомарцию по дев версии

gotlium commented 9 years ago

Странно что он ругается. Мой билд проходил без проблем. Локально пробовал tox запустить в корне проекта?

sergio-bershadsky commented 9 years ago

Нет не пробовал но если ты посмотришь лог оно сломалось раньше моего пула

gotlium commented 9 years ago

Зря ты ридми тронул. У нас есть файл для этого CONTRIBUTING.rst. Поправь этот момент. Так же лучше убрать Development installation, так как есть Local demo installation. Так же описанный в документации http://django-db-mailer.readthedocs.org/dev_install.html и вот еще http://django-db-mailer.readthedocs.org/contributing.html. Пробегись по документации

sergio-bershadsky commented 9 years ago

Ок, но позже - работать надо

gotlium commented 9 years ago

Оки. Не спешим.

gotlium commented 9 years ago

Стоит ветку локально подтянуть development. Было очень много новых коммитов. Если есть возможность, погоняй функционал плагинов, который ты так хотел:)