Fix for issue #1555 and #1620: Page path rewriting when published page becomes the HOME page #1647
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g when published page becomes the HOME page
* when CMS_MODERATOR = True:
if the draft page is unpublished, the public version is also unpublished
* when determining the home page, ignore publication_date and publication_end_date
this ensures that the home page path rewriting logic happens even if the page will only become available
somewhere in the future
* change the post_save_page signal handler so that it also updates the:
** paths(titles) of the currently saved page
** if the currently saved page becomes the new home page, then update the paths(titles) of the previous home page
** if the currently saved page used to be the home page, then update the paths(titles) of the new home page
* add pre/post delete signal handlers that: if the currently deleted page was the home page
then update the path of the new home page
* Added unit tests for all the above functionality
This ensures that:
* we will always have AT MOST one page with the '' (empty) path
* no matter what publishing method is used (tick the publish/unpublish checkbox in the page hierarchy,
tick the 'is published' checkbox from the 'change page' view OR click 'publish' while doing frontend editing)
the generated paths are consistent
|
I will also try to provide a fix for these two bugs on the develop branch. However, since the publishing functionality changed quite a lot, the fix might need to be slightly different. |
| else: | ||
| if home_page.pk == instance.pk: | ||
| for title in Title.objects.filter(path=''): | ||
| if title not in instance.title_set.all(): |
There was a problem hiding this comment.
i'm not 100% sure, but doesn't this execute the query n times (n being the amount of titles with path='')?
Same goes for below. I think you should explicitly execute the instance.title_set.all() query by casting it to a list just after the if statement above.
…nstance's site (otherise titles of home pages from all other sites are returned)
|
Changes Unknown when pulling 22faaa1 on pbs:issue_1555 into * on divio:support/2.3.x*. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for #1555 and #1620 on the 2.3.x branch
if the draft page is unpublished, the public version is also unpublished
this ensures that the home page path rewriting logic happens even if the page will only become available
somewhere in the future
** paths(titles) of the currently saved page
** if the currently saved page becomes the new home page, then update the paths(titles) of the previous home page
** if the currently saved page used to be the home page, then update the paths(titles) of the new home page
then update the path of the new home page
This ensures that: