Allow cron to update post_status

[dd32]

Recently it's been discovered that WordCamp events have been moving from Scheduled to Needs Scheduling automatically. This is confusing, because there's no automation that would result in that.

This is a long winded explanation, as I was confused while trying to work it out.

This happened due to c914a9f
Specifically, this:

- $post = get_post( $post_data_raw['post_ID'] );
+ $post = get_post( $post_data_raw['ID'] );

WordCamps are moved to closed by a cron job.
That cron job obviously runs without a user context. (This shows up as ‘WordCamp Bot’ in the logs, as that’s the text for “no user recorded”)

wordcamp.org/public_html/wp-content/plugins/wcpt/wcpt-wordcamp/wordcamp-admin.php

Lines 1129 to 1166 in b8dea08

	/**
	* Set WordCamp posts to the Closed status after the event is over
	*/
	public function close_wordcamps_after_event() {
	$scheduled_wordcamps = get_posts(
	array(
	'post_type' => WCPT_POST_TYPE_ID,
	'post_status' => 'wcpt-scheduled',
	'posts_per_page' => -1,
	)
	);
	foreach ( $scheduled_wordcamps as $wordcamp ) {
	$start_date = get_post_meta( $wordcamp->ID, 'Start Date (YYYY-mm-dd)', true );
	$end_date = get_post_meta( $wordcamp->ID, 'End Date (YYYY-mm-dd)', true );
	if ( empty( $start_date ) ) {
	continue;
	}
	if ( empty( $end_date ) ) {
	$end_date = $start_date;
	}
	$end_date_at_midnight = strtotime( '23:59', $end_date ); // $end_date is the date at time 00:00, but the event isn't over until 23:59
	if ( $end_date_at_midnight > time() ) {
	continue;
	}
	wp_update_post(
	array(
	'ID' => $wordcamp->ID,
	'post_status' => 'wcpt-closed',
	)
	);
	}
	}

Due to the above code change; that function now works.. it previously only worked when within a wp-admin context, as get_post( null ) would return the proper post, but during cron would return nothing. After the above notice fix, it now returns the proper post in both contexts.

Next; The current user (which is not set) can’t modify the post, as such, when the cron tries to close it, the post_status is reset to the current status of wcpt-scheduled
That’s fine; That should mean zero changes happen, right? Kind of.

wordcamp.org/public_html/wp-content/plugins/wcpt/wcpt-wordcamp/wordcamp-admin.php

Lines 827 to 831 in b8dea08

	// Only WordCamp Wranglers can change WordCamp statuses.
	if ( ! current_user_can( 'wordcamp_wrangle_wordcamps' ) ) {
	$post_data['post_status'] = $post->post_status;
	}

When a post is updated, and it’s being moved to scheduled it checks to see if the appropriate post_meta is set, and if not, pushes it back to needs-scheduling.
That checks $_POST and not the post object, because the post_meta might not yet exist.
When running as cron, $_POST won’t be set with the post meta, so it fails, and it get pushed back to needs-scheduling.

wordcamp.org/public_html/wp-content/plugins/wcpt/wcpt-wordcamp/wordcamp-admin.php

Lines 845 to 893 in b8dea08

	/**
	* Prevent WordCamp posts from being set to pending or published until all the required fields are completed.
	*
	* @param array $post_data
	* @param array $post_data_raw
	* @return array
	*/
	public function require_complete_meta_to_publish_wordcamp( $post_data, $post_data_raw ) {
	if ( WCPT_POST_TYPE_ID != $post_data['post_type'] ) {
	return $post_data;
	}
	// The ID of the last site that was created before this rule went into effect, so that we don't apply the rule retroactively.
	$min_site_id = apply_filters( 'wcpt_require_complete_meta_min_site_id', '2416297' );
	$required_needs_site_fields = $this->get_required_fields( 'needs-site', $post_data_raw['ID'] );
	$required_scheduled_fields = $this->get_required_fields( 'scheduled', $post_data_raw['ID'] );
	// Needs Site.
	if ( 'wcpt-needs-site' == $post_data['post_status'] && absint( $post_data_raw['ID'] ) > $min_site_id ) {
	foreach ( $required_needs_site_fields as $field ) {
	// phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce check would have done in `metabox_save`.
	$value = $_POST[ wcpt_key_to_str( $field, 'wcpt_' ) ];
	if ( empty( $value ) || 'null' == $value ) {
	$post_data['post_status'] = 'wcpt-needs-email';
	$this->active_admin_notices[] = 1;
	break;
	}
	}
	}
	// Scheduled.
	if ( 'wcpt-scheduled' == $post_data['post_status'] && isset( $post_data_raw['ID'] ) && absint( $post_data_raw['ID'] ) > $min_site_id ) {
	foreach ( $required_scheduled_fields as $field ) {
	// phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce check would have done in `metabox_save`.
	$value = $_POST[ wcpt_key_to_str( $field, 'wcpt_' ) ];
	if ( empty( $value ) || 'null' == $value ) {
	$post_data['post_status'] = 'wcpt-needs-schedule';
	$this->active_admin_notices[] = 3;
	break;
	}
	}
	}
	return $post_data;
	}

And that’s how it’s been going from wcpt-scheduled -> wcpt-needs-scheduling, because it’s actually been going scheduled -> (closed -> scheduled -> needs-scheduling)

---

This is a long-winded way to say; The code that enforces the post_status, just needs to acknowledge that during cron, a WordCamp post may change status, and that a user is not required to be present for that.

The alternative option, is that we could wp_set_current_user( wordcamp ) during the close wordcamp routines, but I think checking for cron is enough here, as this isn't the primary authentication, this is just a just-in-case validation check.

[dd32]

@pkevan What's your thoughts here? Do you think it's safe to just skip this cap check for cron? Or should I look at the other option of running the closure cron as a wordcamp user?

My thought is that there's too much of a possibility of other code being affected by this, that I don't know about, so I'd rather just skip the cap check.

[pkevan]

Interesting find :)

I don't have a strong preference to caps check during a cron, but if we are only expecting it to run in cron context we could have it check the constant that cron sets (which isn't perfect) since the function could be run in code by accident.

It feels like require_complete_meta_to_publish_wordcamp should be improved, as i'm personally not a fan of mystery or magic functions which can run and result in this behaviour. we can probably avoid this using a nonce or similar non-caps context checking.

[dd32]

It feels like require_complete_meta_to_publish_wordcamp should be improved

I agree, ideally, there shouldn't be a need for there to be any need for such low-level hooks like wp_insert_post_data - these feel like should be on something higher-level, but it doesn't seem like they really exist.. edit_post() for example doesn't have any hooks of use in it, and likewise the rest-api endpoint only really has rest_pre_insert_{$this->post_type}.

[dd32]

I've changed this slightly; instead of checking cron, checking that it's a logged in user.

This is "weird" in that it's not checking a cap for a logged out user, but it's relying upon the fact that a logged out user should never be able to update a post object.. and that any logged out is expected to be processed.

This is similar to the previous behaviour.

The ?? '' is added just to avoid PHP Notices. That code-branch shouldn't be problematic now, but it might need to be changed in future.