Cannot delete flatpages in admin

[benkonrath]

Here's the error:

IntegrityError at /admin/fluent_pages/page/1/delete/

update or delete on table "fluent_pages_urlnode" violates foreign key constraint "master_id_refs_id_10e5f581" on table "fluent_pages_htmlpage_translation"
DETAIL:  Key (id)=(1) is still referenced from table "fluent_pages_htmlpage_translation".

Here are the versions that I'm using:

git+https://github.com/edoburu/django-fluent-pages.git@eb7f28db695706adc2695d0b24a30a051ecade52#egg=django-fluent-pages
django-mptt==0.6.1
django-parler==1.2.1
django-polymorphic-tree==1.0.1
django-wysiwyg==0.7.0

Can you see something wrong with the versions of the dependencies I'm using?

[benkonrath]

I'm using this with Django 1.6.8.

[RealBigB]

Same problem here with a custom page type derived from fluent_pages.models.Page

IntegrityError at /admin/fluent_pages/page/2/delete/
update or delete on table "fluent_pages_urlnode" violates foreign key constraint
"master_id_refs_id_03dcaa72" on table "cms_basehtmlpage_translation"
DETAIL: Key (id)=(2) is still referenced from table "cms_basehtmlpage_translation".

using :
Django==1.6.5
django-fluent-pages==0.9b4
django-mptt==0.6.1
django-parler==1.2.1
django-polymorphic==0.6
django-polymorphic-tree==1.0.1

Note that I don't have the problem on another projet using fluent_pages 0.9b2 (django-parler==1.0).

[vdboor]

I got the same issue as well.
This is caused by the translation model on the HtmlPage model (created in f2f8319)

It's possible that the admin can't see the HtmlPageTranslation model, depending on which proxy class it looks at.

I'm considering to add an on_delete=models.CASCADE to the parler generated model.

[vinnyrose]

Proxy models are not inspected for fields, as fields are not expected to be defined there. The field has to be defined onto a concrete model.

[vdboor]

Ahh, thanks for that insight! This explains a lot.

[vdboor]

Looking at https://code.djangoproject.com/ticket/16128 and https://code.djangoproject.com/ticket/18491 there is a fix in Django 1.7. As quick workaround for Django 1.6 I've improved the delete() method in 959211c

This doesn't entirely fix it off course, but makes the site usable again!

[vdboor]

This workaround/fix is now released as v0.9c1, making the latest beta versions usable again!

[benkonrath]

Thanks. Should I close the issue?

[benkonrath]

@vdboor Can we close this issue?

[vdboor]

Sorry! Yes, lets close this and see when it comes up again.

[vinnyrose]

I'm in Django 1.8. I still have to use the following code to cascade delete a page and it's children. Even though the collector seems to be finding the translations on the delete confirmation page.

@receiver(pre_delete, sender=UrlNode)
def fix_delete(sender, instance, *args, **kwargs):
    model_class = instance.polymorphic_ctype.model_class()
    if issubclass(model_class, HtmlPage):
        HtmlPageTranslation = model_class.seo_translations.related.related_model
        HtmlPageTranslation.objects.filter(master=instance.pk).delete()

[jmurty]

We have experienced similar issues.

It is likely related to Django core issues with finding and processing reverse FK relationships on proxy models, see https://code.djangoproject.com/ticket/23076 and https://code.djangoproject.com/ticket/18012

The Django issue has recently been fixed on the master branch, though we haven't yet been able to fully confirm it due to deadlines and since it doesn't really help us for now (we are using Django 1.7).

Has your pre-delete work-around been reliable @vinnyrose? We might do something similar.

[vinnyrose]

It has been reliable in Django 1.6 and we tested today in Django 1.8 (skipped 1.7) with no problems. I don't know if 'related_model' is the correct attribute on the 'seo_translations' 'related' attribute, it might be 'model' in django 1.7.

[mrmachine]

Yep. This is an issue for any model (not only translations) that has FKs to one of fluent's proxy models (e.g. Page or HtmlPage).

@vdboor any thoughts on how we might be able to work-around this in fluent, given that the fix won't be in Django until 1.10 and 1.8 is an LTS release?

Monkey patching the collector to inspect related descriptors on proxy models?

Or a pre_delete signal handler that is configurable (via settings) to either cascade or set null specific models that might be related to a UrlNode proxy model, or a default cascade behaviour for unspecified models?

The former would hopefully also display the full list of collected objects on the admin delete page. The latter would just silently cascade or set null on delete.

As this is actually not specifically a problem with fluent, but it does affect fluent, it would be nice if the workaround was in fluent-utils and was general enough to be adapted for use with any proxy models that might have FKs pointing at them.

And should we re-open this issue, or make a new issue?

[jmurty]

FWIW we could provide a sample monkey-patch hack of Django's Collector that allows it to find and delete the proxy models' reverse FK relationships. As @mrmachine notes, this makes the pre-delete admin screen more accurate.

However we are wary about using this hack in production, so we are looking at less intrusive options.

[vdboor]

Yikes.

@mrmachine: I'd be ok with having a hack in fluent, consider it for "pre 1.9 Django compatibility". If the collector can be patched in a clean way, we might be able to ship it, but I'd rather not monkey patch Django silently from a thirdparty app.

[mrmachine]

@vdboor We have a working monkey patch to Django's collector that we are applying in our project's AppConfig.ready() method. I was thinking about adding the patch and an AppConfig mixing class to fluent-utils along with some notes in the documentation about how users can enable the patch in their project via the mixin. This way we would not be silently patching Django, but would have a solution ready for users who have FKs to fluent's proxy models.

[mrmachine]

Also, I think Django won't get this fix until the next release after 1.9, due to the timings of when it went in :(

[jpotterm]

I agree, monkey patching should definitely be opt-in. The pre_delete signal handler @vinnyrose posted has been working well for us in production so far.

[vinnyrose]

1.9 just went into beta, and the fix for these bugs was not merged in, so it won't make it in until 1.10 since these are not "release blocking". The AppConfig route sounds like a good idea, it fixes the pre-delete confirmation screen and sounds to be general enough to deal with any FK on_delete behavior. Unlike my pre_delete signal solution which only does CASCADE.

@mrmachine I would like to see the AppConfig solution if you can share it.

If it is possible, it would be convenient to have @mrmachine's mixin added to an alternative AppConfig in fluent_pages.apps itself i.e.:

class FluentAppCollectorPatchConfig(CollectorPatchMixin, FluentAppConfig):
    pass

Then users can specify the monkey-patching AppConfig in the INSTALLED_APPS settings, i.e. instead of "fluent_pages" they would put "fluent_pages.apps.FluentAppCollectorPatchConfig".

Perhaps for those that don't want to use the monkey patch the default FluentAppConfig can provide a ready method that sets up the pre_delete solution (the collector patch mixin will override the ready method)? It is possible to find all FK fields on a model, and then look at the on_delete behavior on those field and perform that action (i.e. no need for per-model-settings or a default behavior). We could cache the on_delete behaviors per model class per field at startup for better performance.