-
Notifications
You must be signed in to change notification settings - Fork 27
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot delete flatpages in admin #47
Comments
I'm using this with Django 1.6.8. |
Same problem here with a custom page type derived from
using : Note that I don't have the problem on another projet using fluent_pages 0.9b2 (django-parler==1.0). |
I got the same issue as well. It's possible that the admin can't see the I'm considering to add an |
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. |
Ahh, thanks for that insight! This explains a lot. |
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 This doesn't entirely fix it off course, but makes the site usable again! |
This workaround/fix is now released as v0.9c1, making the latest beta versions usable again! |
Thanks. Should I close the issue? |
@vdboor Can we close this issue? |
Sorry! Yes, lets close this and see when it comes up again. |
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.
|
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. |
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. |
Yep. This is an issue for any model (not only translations) that has FKs to one of fluent's proxy models (e.g. @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 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 And should we re-open this issue, or make a new issue? |
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. |
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. |
@vdboor We have a working monkey patch to Django's collector that we are applying in our project's |
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 :( |
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. |
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 @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
Then users can specify the monkey-patching AppConfig in the Perhaps for those that don't want to use the monkey patch the default |
Here's the error:
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?
The text was updated successfully, but these errors were encountered: