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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-create alias pages after page is completely saved #454

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

kaedroho
Copy link
Contributor

@kaedroho kaedroho commented Sep 16, 2021

Fixes: #346
Fixes: #391
Also provides a better fix for #245
CC: MozillaFoundation/foundation.mozilla.org#7356 @TheoChevalier

When you tried to publish a page that contains new inline panel items, Django would crash with an integrity error.

This behaviour is caused by:

  • Wagtail Localize calls .copy_for_translation to create the alias pages during the post_save signal that was triggered when the source page is published
  • At this point, the source page itself is saved but its inline panel items are not
  • .copy_for_translation calls .copy_child_relation in django-modelcluster to copy the inline panel objects to the new alias
  • .copy_child_relation does not copy the objects, but instead just reassigns them to the new page. This relies on the fact that the page being copied wouldn't be saved again in this request
  • After calling .copy_child_relation, the .copy_for_translation then sets all the locale fields of the inline panel objects to the target language. But as they are actually the same Python objects as the source page, this inadvertently updates them on the source page as well
  • After the post_save signal is finished, Django then attempts to save the inline panel objects on the source page. This leads to the integrity error because their locale is set to the target language instead of the source, and there is a unique constraint on that field which raises a conflict with the already saved inline panel objects on the alias page.

The workaround to this I have implemented here is to create the alias pages using the after_create_page hook instead of the post_save signal. This is always run after the many to many objects are already saved avoiding the underlying bug in django-modelcluster.

@kaedroho kaedroho force-pushed the create-aliases-after-complete-save branch from 36e276f to a51f446 Compare September 17, 2021 10:21
@codecov-commenter
Copy link

Codecov Report

Merging #454 (a51f446) into main (1ba776c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #454      +/-   ##
==========================================
- Coverage   93.10%   93.09%   -0.01%     
==========================================
  Files          35       35              
  Lines        2812     2808       -4     
  Branches      445      442       -3     
==========================================
- Hits         2618     2614       -4     
  Misses         98       98              
  Partials       96       96              
Impacted Files Coverage Δ
wagtail_localize/synctree.py 93.50% <100.00%> (-0.33%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ba776c...a51f446. Read the comment docs.

@kaedroho kaedroho merged commit 6ebe0ba into main Sep 17, 2021
@kaedroho kaedroho deleted the create-aliases-after-complete-save branch September 17, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrity error on form fields when automatic sync is turned on Inlines trigger error upon save
2 participants