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

DUPP-52 improve gettext usage #255

Merged
merged 2 commits into from
Jun 14, 2022
Merged

DUPP-52 improve gettext usage #255

merged 2 commits into from
Jun 14, 2022

Conversation

enricobattocchi
Copy link
Member

@enricobattocchi enricobattocchi commented Jun 10, 2022

Context

  • We want to improve how some labels are rewritten for the R&R feature using gettext, to avoid performance issues.

Summary

This PR can be summarized in the following changelog entry:

  • Improves the impact of the plugin on the performance of the site by avoiding useless calls on the gettext filter.

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

We should check that the labels are still rewritten, and that the number to the affecting methods is much lower.

Regression test

  • install the Classic Editor plugin
  • Choose a published post and create a R&R copy of that
  • while editing the R&R copy in the Classic Editor, check that the button says "Republish" and the field where you set the date says "Republish on:" (the wrong margin is being fixed in DUPP-572 Apply core class to get margins right #254)
    Screenshot 2022-06-10 at 09-49-52 Edit Post “Digitized didactic strategy” ‹ Basic — WordPress
  • change the date to a future date and check that the button now says "Schedule republish"
  • save as draft and on reload check that the button is still "Schedule republish"
  • check the same for pages and custom post types.

Check that we hook only in the edit screen

  • install Query Monitor
  • in QM, open Languages > Hooks in use:
    Screenshot 2022-06-10 at 10-10-48 Edit Post “Open-source 3rdgeneration protocol” ‹ Basic — WordPress
  • visit different pages in the backend and see that Yoast\W\D\U\Classic_Editor->change_republish_strings_classic_editor() and Yoast\W\D\U\Classic_Editor->change_schedule_strings_classic_editor() are hooked only on the edit post/page screens

Check the performance improvement

this is not easy to check because it's visible in environments with plenty of content, but a possible way could be:

  • set breakpoints on Yoast\W\D\U\Classic_Editor->change_republish_strings_classic_editor() and Yoast\W\D\U\Classic_Editor->change_schedule_strings_classic_editor()
  • see that you hit the breakpoints only in the edit screen
  • see that when you hit the breakpoints, you bail out early when the text domain/context or the strings are not the expected ones, so no call to get_post() is done in that case.

Another idea:

  • install the Code Profiler plugin
  • Run checks for Dashboard and for Posts screen as admin with and without this patch, and see that the times change clearly (e.g. 0.043s vs 0.126s on the Posts page, and 0.022s vs 0.08s on the Dashboard)

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Fixes #185 DUPP-52

@pls78
Copy link
Member

pls78 commented Jun 14, 2022

CR&ACC: ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the use of the gettext filter.
2 participants