Social: Default per-network mode from connection templates#48721
Social: Default per-network mode from connection templates#48721gmjuhasz wants to merge 1 commit into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
This PR updates Jetpack Social’s editor behavior to automatically default into per-network customization mode when Message Templates are enabled and at least one connected account has a non-empty custom template, and persists that default in post meta so it survives reloads.
Changes:
- Add template-based defaulting logic to
usePerNetworkCustomization, persisting_wpas_customize_per_network: truewhen the default applies. - Track “default disabled for this editor session” state so users can toggle back to “Same for all” without immediately re-defaulting.
- Extend hook test coverage for the new defaulting and session-disable behavior, plus add a changelog entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/packages/publicize/changelog/social-482-per-network-template-default | Adds a changelog entry describing the behavioral fix. |
| projects/packages/publicize/_inc/hooks/use-per-network-customization/index.ts | Implements template-driven default per-network mode and persists the meta flag when applicable. |
| projects/packages/publicize/_inc/hooks/use-per-network-customization/test/index.test.ts | Adds tests validating defaulting behavior, blank-template behavior, and session-level disable. |
| Significance: patch | ||
| Type: fixed | ||
|
|
||
| Social: Default per-network customization to on only when a custom connection template exists. |
Code Coverage SummaryCoverage changed in 1 file.
|
manzoorwanijk
left a comment
There was a problem hiding this comment.
There is a use-case which doesn't work as expected. Please see the inline comment.
| const isEnabled = useSelect( select => { | ||
| const isMetaEnabled = useSelect( select => { | ||
| const meta = select( editorStore ).getEditedPostAttribute( 'meta' ); | ||
|
|
||
| return Boolean( meta?.[ CUSTOMIZE_PER_NETWORK_KEY ] ); | ||
| }, [] ); | ||
| const shouldUseTemplateDefault = | ||
| templatesEnabled && hasConnectionTemplate && ! isMetaEnabled && ! templateDefaultDisabled; | ||
| const isEnabled = isMetaEnabled || shouldUseTemplateDefault; |
There was a problem hiding this comment.
This logic here is a bit problematic. If I try this combination:
- Set the template for a connection
- Switch to global mode
- Save the post and reload
- Open the preview mode
The toggle switches to per-network mode and you cannot switch back to global mode.
I think it should be
const isMetaEnabled = useSelect( select => {
const meta = select( editorStore ).getEditedPostAttribute( 'meta' );
return meta?.[ CUSTOMIZE_PER_NETWORK_KEY ];
}, [] );
const shouldUseTemplateDefault =
isMetaEnabled ?? ( templatesEnabled && hasConnectionTemplate && ! templateDefaultDisabled );
const isEnabled = isMetaEnabled ?? shouldUseTemplateDefault;But, we could organize that code better inside a useMemo using a bail-early approach.
That said, do you think we should pass the default value from the backend?
There was a problem hiding this comment.
Good catch, after someone explicitly switches back to global, reload should not default them back into per-network. I think the cleanest fix is to keep the default logic client-side, but add a small persisted “user has explicitly chosen this toggle” flag so we can distinguish default false from user-saved false.
On the backend, that default is static, while this behavior depends on current connection templates. Making it dynamic there is awkward and broader than this issue.
What do you think?
There was a problem hiding this comment.
I agree. The main idea here is to respect isMetaEnabled
Fixes SOCIAL-482
Proposed changes
_wpas_customize_per_network: truewhen this default applies, so a post shared/reloaded after defaulting remains in per-network mode._wpas_customize_per_networkis a boolean, so an explicit savedfalsecannot be distinguished from the defaultfalseon reload. A separate override flag could preserve "turned off despite custom template" across reloads, but this PR avoids that extra state because it does not seem worth it currently.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
true; verify Customize each is selected by default.pnpm --filter @automattic/jetpack-publicize test -- _inc/hooks/use-per-network-customization/test/index.test.ts.pnpm --filter @automattic/jetpack-publicize test.pnpm --filter @automattic/jetpack-publicize typecheck.