Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to use plain ForeignKeys with InlinePanels in modeladmin #24

Open
awhileback opened this issue Jun 3, 2021 · 2 comments
Open
Labels
type:Enhancement New feature or request

Comments

@awhileback
Copy link

awhileback commented Jun 3, 2021

Found a bug? Please fill out the sections below. 馃憤

Issue Summary

Imagine a drip email marketing functionality, in which non-developers are able to build query sets from dropdown menus and append them as inline panels on an email form. That's what this is ;). Of course, people tend to forget things, and the first thing that needs to be included in an email marketing rule by default which the content author would otherwise forget is to exclude people who have unsubscribed. So this is an attempt to automatically append "is_subscribed = True" to the first inline rule of a marketing email form.

But, the conditional default modeladmin inline property seems to be duplicated under certain circumstances. When defining a custom CreateView with an inline form pre-filled, the form is properly appended upon create. But upon edit of the just-created item, the item seems to have been duplicated, there are then two identical entries.

After removing one of the duplicates and saving the item, the duplicate doesn't come back a second time after going back and forth between the edit/list page, so this seems to be specific to the initial create + save, perhaps, in the CreateView that was specified.

Steps to Reproduce

  1. Install wagtail
  2. Install Django drip campaigns (straight from git is fine)
  3. Migrate, integrate the drip admin into wagtail via wagtail_hooks like so....

(You will of course not have an is_subscribed boolean field, change to some other django user model boolean field to see the result, you'll also not have the django-summernote editor widget unless you install it. Otherwise I think these should work as described.)

from drip.models import Drip, SentDrip, QuerySetRule
from wagtail.contrib.modeladmin.views import CreateView, EditView

class DripAdminCreateView(CreateView):
    def get_instance(self):
        instance = super().get_instance()
        # default values for first query rules InlinePanel to only match subscribed users
        if instance.queryset_rules.exists():
            pass
        else:
            instance.queryset_rules = [
                QuerySetRule(method_type='filter', field_name='is_subscribed', lookup_type='exact', rule_type='and', field_value=1, sort_order=0)
            ]
        return instance

class DripAdmin(ModelAdmin):
    model = Drip
    menu_icon = 'fa-tint'
    menu_label = 'Drip Email'
    list_display = ('id', 'name', 'subject_template', 'enabled', 'lastchanged')
    create_view_class = DripAdminCreateView

    panels = [
        MultiFieldPanel(
            [
                FieldPanel('name', widget=SubjectField),
                FieldPanel('enabled'),
                FieldPanel('from_email'),
                FieldPanel('from_email_name', widget=SubjectField),
                FieldPanel('subject_template', widget=SubjectField),
                FieldPanel('body_html_template', widget=SummernoteInplaceWidget),
                InlinePanel('queryset_rules', label='Query Set Rules'),
            ],
            _('Drip Admin'),
        )
    ]

class SentDripAdmin(ModelAdmin):
    model = SentDrip
    menu_icon = 'fa-envelope-open'
    menu_label = 'Drip Email Logs'
    list_display = ('id', 'drip', 'subject', 'user', 'date')
    permission_helper_class = DripLogPermissionHelper

    panels = [
        MultiFieldPanel(
            [
                FieldPanel('subject', widget=SubjectField),
                FieldPanel('body', widget=SummernoteInplaceWidget),
                FieldPanel('from_email', widget=SubjectField),
                FieldPanel('from_email_name', widget=SubjectField),
            ],
            _('Sent Drip Admin'),
        )
    ]

class EmailGroup(ModelAdminGroup):
    menu_label = 'Email'
    menu_icon = 'fa-envelope'
    menu_order = 500
    items = (DripAdmin, SentDripAdmin)

modeladmin_register(EmailGroup)

To further test this I put some print statements in my conditionals along with an EditView specification and got the following...

class DripAdminEditView(EditView):
    def get_instance(self):
        instance = super().get_instance()
        if instance.queryset_rules.exists():
            print('true')
        else:
            print('false')
        return instance

Upon edit, I get...

true
true
true
true
[11/May/2021 02:09:02] "GET /admin/drip/drip/edit/10/ HTTP/1.1" 200 51466

Upon create, with a print of ('false') added to the default inline...

class DripAdminCreateView(CreateView):
    def get_instance(self):
        instance = super().get_instance()
        # default values for first query rules InlinePanel to only match subscribed users
        if instance.queryset_rules.exists():
            print('true')
        else:
            instance.queryset_rules = [
                    QuerySetRule(method_type='filter', field_name='is_subscribed', lookup_type='exact', field_value=1, sort_order=0)
            ]
            print('false')
        
        return instance

I got...

false
false
false
false
[11/May/2021 02:10:07] "GET /admin/drip/drip/create/ HTTP/1.1" 200 42503

So the conditionals seem to be working as expected.

This doesn't appear to be some weird browser caching behavior, the database shows two identical entries for the first inline, here's an SQL dump of one after 'create':

INSERT INTO "main"."drip_querysetrule" ("id", "sort_order", "date", "last_changed", "method_type", "field_name", "lookup_type", "field_value", "drip_id") VALUES ('32', '0', '2021-05-11 07:00:01.843932', '2021-05-11 07:00:01.940603', 'filter', 'is_subscribed', 'exact', '1', '10');
INSERT INTO "main"."drip_querysetrule" ("id", "sort_order", "date", "last_changed", "method_type", "field_name", "lookup_type", "field_value", "drip_id") VALUES ('33', '0', '2021-05-11 07:00:01.985938', '2021-05-11 07:00:01.985988', 'filter', 'is_subscribed', 'exact', '1', '10');
INSERT INTO "main"."drip_querysetrule" ("id", "sort_order", "date", "last_changed", "method_type", "field_name", "lookup_type", "field_value", "drip_id") VALUES ('34', '1', '2021-05-11 07:00:02.045635', '2021-05-11 07:00:02.045719', 'filter', 'last_login', 'gt', 'now-1 day', '10');

Technical details

  • Python version: 3.8.5
  • Django version: 3.1
  • Wagtail version: 2.11.7
  • Browser version: Safari 14
@awhileback awhileback added the type:Bug Something isn't working label Jun 3, 2021
@gasman
Copy link
Contributor

gasman commented Jun 4, 2021

Hi @awhileback,
I think this is because the queryset_rules relation in django-drip-campaigns - being not specifically built for Wagtail - is a ForeignKey rather than a ParentalKey. InlinePanels are only designed for ParentalKey relations - these have different behaviours regarding how forms are processed and the order in which models are saved, and these differences are key to various bits of Wagtail functionality like previewing and revision history.

Having said that, those aspects don't apply to ModelAdmin, and since ModelAdmin is intended to work with plain Django models, I think it's reasonable to expect plain foreign keys to work there. So while I don't think this is an actual bug, I'd say it's a legitimate feature request.

@gasman gasman added type:Enhancement New feature or request and removed type:Bug Something isn't working labels Jun 4, 2021
@gasman gasman changed the title modeladmin CreateView set to conditionally pre-populate an inline panel produces duplicate inlines upon create, but not thereafter. Ability to use plain ForeignKeys with InlinePanels in modeladmin Jun 4, 2021
@awhileback
Copy link
Author

awhileback commented Jun 4, 2021

Hey @gasman, thanks for the reply, now that you mention it I remember why there was a need to fork the drip app. Sorry, it's been awhile since I got this working initially a couple of years ago, I've just come back to it for the purpose of upgrading versions.

I did sub-class the InlinePanel model and change it to a ParentalKey to get it to work. That was done before I posted the bug report.

https://github.com/awhileback/django-drip-campaigns/blob/e708f3d55bef4bca1ac167d43223f028da852fd3/drip/models.py#L365

@laymonage laymonage transferred this issue from wagtail/wagtail Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants