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

Try/publicize api #10367

Closed
wants to merge 14 commits into from
Closed

Try/publicize api #10367

wants to merge 14 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Oct 22, 2018

Reboot of sorts of #9963, to help us develop the Calypso side. Essentially #9955 plus modules/publicize/class-jetpack-publicize-gutenberg.php. Don't merge.

c-shultz and others added 14 commits July 12, 2018 15:35
Connection data access is intermingled with HTML
generation. This moves connection data access
out of ui.php and into publicize-jetpack.php to
decouple it from the UI. This is a cleaner design
and allows the data to be accessed for other purposes.
Adding test coverage for:
 - done_sharing_post: testing flagged and published posts
 - get_services_connected: testing service list
 - get_filtered_connection_data: testing simple connection data with no filtering
Adding unit tests for three existing filters in Publicize:
 - 'wpas_submit_post?'
 - 'publicize_checkbox_global_default'
 - 'publicize_checkbox_default'

These filters were previously difficult to test since they were
being applied within HTML generation code. Now that HTML
generation is separated from the connection retrieving/filtering,
it is a good time to add unit tests.
A previous iteration had method 'get_filtered_connection_data' in
Publicize_UI, so the test and other references were calling it from
there. This updates those references now that the method has been moved
into Publicize class.
'disabled' parameter value was from 'get_filtered_connection_data'
return was an HTML string an an early iteration but is now a
boolean. This commit updates the associated test case to
check new data format.
HTML indenting was incorrect in
Publicize_UI::get_metabox_form_connected(). This
commit fixes the whitespace.
Updated Publicize form generation to use new boolean
parameter value to correctly disable checkboxes when
required.
Cleaning up some minor issues identified by phpcs in
get_filtered_connection_data. This function is legacy
code that has been moved over, so it continues to have
code style issues. More major refactoring would be
necessary to fix the rest...
Cleaning up low-hanging fruit on coding styles, including:
 - whitespace
 - comments
 - yoda conditions
'@SInCE' changing from 5.9.1 to 6.2.0.
'@SInCE' version number being removed from
Publicize_UI::get_metabox_form_connected() since
this method is not new in this PR.

props @kraftbj
Modifies Publicize_UI::get_filtered_connection_data()
to gracefully handle the case where there is no
current post and the post_id parameter is unset.
This condition is now handled by not disabling
any of the connection checkboxes.
Previously, this case would cause a PHP error.
Also adding test cases to exercise this case.

props @kraftbj
Reining in some especially long lines
in Publicize_UI::get_filtered_connection_data().
No functional changes intended.
@ockham ockham added [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack DO NOT MERGE don't merge it! labels Oct 22, 2018
@jeherve jeherve added the [Feature] Publicize Now Jetpack Social, auto-sharing label Oct 22, 2018
@jeherve jeherve added this to the 6.7 milestone Oct 22, 2018
@ockham
Copy link
Contributor Author

ockham commented Oct 23, 2018

Closing in favor of #10372.

@ockham ockham closed this Oct 23, 2018
@ockham ockham deleted the try/publicize-api branch October 23, 2018 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE don't merge it! [Feature] Publicize Now Jetpack Social, auto-sharing [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants