Removed an incorrect blocking of plugin cleanup on cancellation #537
Conversation
Codecov Report
@@ Coverage Diff @@
## support/4.0.x #537 +/- ##
==============================================
Coverage 66.66% 66.66%
==============================================
Files 13 13
Lines 387 387
Branches 40 40
==============================================
Hits 258 258
Misses 115 115
Partials 14 14
Continue to review full report at Codecov.
|
|
@vxsx Hi Vadim, This PR fixes an issue that is common and breaks being able to edit a page in the CMS for Django cms v4. An issue occurs because the new plugin architecture no longer uses treebeard and has it's own custom tree implementation, the cancel operation doesn't clean up when the user clicks cancel. The steps to reproduce the issue:
I can provide two sites for demo, one with the fix and one without, I can send these via the normal slack channels when you are ready to view, if of course you have the time to see the issue and then this fix in action. :-) |
…ause corruption issues for a page
…table error message
…nk about cleaning ghosts
…xt-ckeditor into bugfix/broken-plugin-cancellation
… and isort failures
…e any in the future
…ct that we are catching an IntegrityError error
| """ | ||
| If any "ghost" plugins are left behind by a failed cancellation | ||
| the creation of a new plugin can be blocked by the fact that a new plugin is | ||
| trying to use the same position, to fix this we can clean any existing orphns |
| trying to use the same position, to fix this we can clean any existing orphns | ||
| and recalculate the placeholder plugins positions | ||
| """ | ||
| has_ophans = False |
There was a problem hiding this comment.
Can we change this to orphans as well?
tests/test_plugin.py
Outdated
| with transaction.atomic(): | ||
| readd_response = self.client.get(endpoint) | ||
|
|
||
| # FIXME: Breaks because the code catches and integrity and then allows one to be thrown again! |
There was a problem hiding this comment.
If this issue still exists can we fix it before merging?
There was a problem hiding this comment.
Yup, it won't be going over until this test is fixed, it's blocking the pr.
…n it should reuse the ghost plugins position
…the add_view method
|
@Aiky30 looks sane to me |
This PR aims to fix an issue where cancelling a new text plugin before it is ever saved leaves the page in a corrupt state due to models being created that are not cleaned up.
The JS uses a count of the structure mode items for positions which does not include any "ghost" cms_plugins which skews the number of plugins.
The following PR aimed to resolve this issue but it also contains other changes that were rightfully rejected due to using the wrong Jquery package: #515
The error that this PR fixes:
IntegrityError at /en/admin/cms/placeholder/add-plugin/ duplicate key value violates unique constraint "cms_cmsplugin_placeholder_id_language_position_bad60ebe_uniq" DETAIL: Key (placeholder_id, language, "position")=(9, en, 1) already exists.