janboddez / share-on-mastodon

Easily share WordPress posts on Mastodon.
https://jan.boddez.net/wordpress/share-on-mastodon
GNU General Public License v3.0
39 stars 5 forks source link

"Validation failed" when posting through JetPack with hook #107

Closed jsit closed 2 months ago

jsit commented 5 months ago

I have "Always share on Mastodon" checked, and I have this hook in my theme's functions.php:

add_filter( 'share_on_mastodon_status', function( $status, $post ) {
  if ( 'status' === get_post_format( $post ) ) {
    $status = wp_strip_all_tags( $post->post_content );
    return $status;
  } else {
    return null;
  }
}, 10, 2 );

This works fine when posting through WP Admin, and curiously it works when posting from MarsEdit, but when posting from the JetPack app, (a) the post doesn't get shared to Mastodon, and (b) the post shows this message in the edit screen:

Validation failed: Text can't be blank

Screenshot 2024-05-05 at 2 35 45 PM

Is this expected behavior, is it a bug, or am I doing something wrong?

janboddez commented 5 months ago

Yeah, you shouldn't return null. The return value has to be a non-empty string, as it is the text you're posting to Mastodon.

Maybe you want to disable federation for non-status posts? In that case you should probably use the share_on_mastodon_enabled filter. (I need to update the docs!)

janboddez commented 5 months ago

To disable the cross-posting of anything that isn't a status, you'd use an additional filter, something like (untested):

add_filter( 'share_on_mastodon_enabled', function( $is_enabled, $post_id ) {
  if ( 'status' !== get_post_format( $post_id ) ) {
    // Only cross-post statuses.
    $is_enabled = false;
  }

  return $is_enabled;
}, 10, 2 );

This should take precedence over the "Share Always" setting.

Edit: Judging from the code, the above will likely not work ... and I should fix that. But, in the meantime, you could instead disable "Share Always" and use:

add_filter( 'share_on_mastodon_enabled', function( $is_enabled, $post_id ) {
  if ( 'status' === get_post_format( $post_id ) ) {
    // **Always** cross-post statuses.
    $is_enabled = true;
  }

  return $is_enabled; // Unless enabled in WP-Admin, `$is_enabled` is `false` by default.
}, 10, 2 );

To override the Mastodon status text (but only for statuses):

add_filter( 'share_on_mastodon_status', function( $status, $post ) {
  if ( 'status' === get_post_format( $post ) ) {
    $status = wp_strip_all_tags( $post->post_content );
  }

  return $status;
}, 10, 2 );
janboddez commented 5 months ago

The return value has to be a non-empty string, as it is the text you're posting to Mastodon.

Guessing it could be blank if there was an image. I should check that.

janboddez commented 5 months ago

If the post is a "status" and there is text (i.e., a non-empty post content), then it may be a bug, though, and likely has to do with how WordPress' REST API works (i.e., it first saves the post, and only then sets the post format).

Edit: Just had a quick look at the code and this should work. That is, we should already be working around any such REST API weirdness.

You might be able to work around that by setting a "delay" (of only, I don't know, 10 or so seconds). (Still, $status should probably not be null, ever.)

jsit commented 5 months ago

If the post is a "status" and there is text (i.e., a non-empty post content), then it may be a bug, though, and likely has to do with how WordPress' REST API works (i.e., it first saves the post, and only then sets the post format).

Yeah, that's why this surprised me -- I didn't expect it to even get to the return null, since there is content, and when posting via the web UI, it works with this same hook in place.

janboddez commented 5 months ago

So ... you're certain the post format was set?

Guessing the app is doing something different then ...

Would you mind trying setting a delay? I think a couple seconds should be plenty, but it doesn't hurt to also try out, e.g., 60 seconds. (Note that WP's cron system is notoriously inaccurate, so posts could always come through later.)

janboddez commented 5 months ago

Guessing the app is doing something different then ...

I'd assume the app uses the REST API. Which means a post gets created at https://github.com/WordPress/WordPress/blob/bcff9a2fe449c6e304b327768f3d92dc74bc6533/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L647, which is what triggers the first run of https://github.com/janboddez/share-on-mastodon/blob/a4a4bc5fa852b9b2e900bbcd6f992875e3a43802/includes/class-post-handler.php#L114 except it quits almost instantly https://github.com/janboddez/share-on-mastodon/blob/a4a4bc5fa852b9b2e900bbcd6f992875e3a43802/includes/class-post-handler.php#L117-L124 and runs again at https://github.com/WordPress/WordPress/blob/bcff9a2fe449c6e304b327768f3d92dc74bc6533/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L778, well after the post format is set (which happens after the initial save, hence us delaying things to a later hook).

At least ... that's the theory.

But since the block editor uses the same API ... I don't get why it'd be any different?

The only way I can see this fail is if the first call (i.e., before the post format is stored) of our toot() method actually did get to run ... which should never happen, as the 0 === strpos( current_action(), 'save_' ) in https://github.com/janboddez/share-on-mastodon/blob/a4a4bc5fa852b9b2e900bbcd6f992875e3a43802/includes/class-post-handler.php#L117 is definitely true at that point.