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

Optimize populating title cache for Page model. #7177

Merged
merged 9 commits into from
Nov 25, 2022

Conversation

zkne
Copy link
Contributor

@zkne zkne commented Dec 30, 2021

Description

Optimizes the way the title cache is populated.

Related resources

Checklist

  • I have opened this pull request against develop
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on Slack to find a “pr review buddy” who is going to review my pull request.

@stale
Copy link

stale bot commented Jun 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 30, 2022
@stale stale bot removed the stale label Jul 3, 2022
@stale
Copy link

stale bot commented Oct 1, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 1, 2022
@fsbraun fsbraun added 4.1 and removed stale labels Oct 1, 2022
@fsbraun
Copy link
Sponsor Member

fsbraun commented Oct 23, 2022

@zkne Thanks very much for this PR! Can you have a look at the failing tests? Indeed, it would be great to have the improved population of the page content cache.

@zkne
Copy link
Contributor Author

zkne commented Nov 7, 2022

@fsbraun thanks for reminding me of this PR. I will update it in the following days.

@zkne
Copy link
Contributor Author

zkne commented Nov 7, 2022

@fsbraun Can you please check the PR now, I've updated the tests.

test_editing_plugin_changes_page_modification_time_in_sitemap wasn't written correctly as you can see with the new assert I've added.

@fsbraun
Copy link
Sponsor Member

fsbraun commented Nov 7, 2022

@zkne This looks really good to me!

Copy link
Sponsor Member

@fsbraun fsbraun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tested this w/ versioning which monkey patches both PageContent.objects as well as Page.page_content_set. I do not see a side effect there.

@fsbraun fsbraun merged commit 497c3c6 into django-cms:develop-4 Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants