-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Blogging Prompts: Add wrapper component to interface between schedulers #18772
Conversation
You can test the changes in Jetpack from this Pull Request by:
|
You can test the changes in WordPress from this Pull Request by:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests pass and the code looks good to me!
The unit test failure is pretty weird. After spending a bit digging into this, it looks like there are other "live objects" subscribing to the account change notification after calling The core problem, as I understand, is that different objects hold different context managers but they share the same user defaults; which can lead to inconsistency that caused some stored values to be wiped. In this case, I think (cc: @mokagio & team) |
Refs #18761
This PR adds
ReminderScheduleCoordinator
as a wrapper component that can decide whether to useBloggingRemindersScheduler
orPromptRemindersScheduler
.The component will decide to use the Blogging Prompts scheduler when all of these are true:
BloggingPromptsSettings
),promptRemindersEnabled
property istrue
. This property tracks and gets updated whenever theInclude Prompts
switch is toggled in the reminder sheet.In other cases, the coordinator will fall back on the Blogging Reminders scheduler.
To Test
Ensure that the unit tests are passing.
Regression Notes
Potential unintended areas of impact
N/A. Feature is unreleased and component is not yet used.
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A. Feature is unreleased and component is not yet used.
What automated tests I added (or what prevented me from doing so)
Added unit tests.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.