Sync source page privacy settings with translated page #496
Conversation
Codecov Report
@@ Coverage Diff @@
## main #496 +/- ##
==========================================
+ Coverage 92.82% 92.92% +0.09%
==========================================
Files 36 36
Lines 2983 3023 +40
Branches 467 477 +10
==========================================
+ Hits 2769 2809 +40
Misses 114 114
Partials 100 100
Continue to review full report at Codecov.
|
| if list( | ||
| original_restriction.groups.values_list("pk", flat=True) | ||
| ) != list(translation_restriction.groups.values_list("pk", flat=True)): | ||
| translation_restriction.groups.set( | ||
| original_restriction.groups.all() | ||
| ) |
There was a problem hiding this comment.
Are wemissing
should_save = True
in this last if block?
There was a problem hiding this comment.
Also, just for my own understanding, is the reason why we're doing this
should_save = False
if this != that:
this = that
should_save = True
if should_save:
this.save()pattern rather than just
this = that
this.save()for performance reasons? (i.e: we're trying to minimise round trips to the DB)
There was a problem hiding this comment.
it certainly is missing should_save = True.
The reason for this is that 2 or more of the conditions can be true.
e.g. when changing form the 'login' restriction to 'password', both the restriction_type and password attributes will have changed.
so we want a single save
There was a problem hiding this comment.
on missing should_save, we are not - 0741ae4#diff-a9c40e8134c27fd95a23649e865005e8a957dc24db0d423122655a565221d614R555-R565 - as groups are M2M
This PR fixes #317
it synchronizes the source page
PageViewRestrictionwith the translated pages on translation creation and sync.Scenarios covered:
Note: if the source page privacy settings change, the translated page(s) privacy settings will only get updated after "sync translations"