Add check to is_wordcamp_closed function

[pkevan]

When a get_wordcamp_post() cannot find a post, it return false, which then breaks the check against post_status - this stops that happening, and should also allow testing of tickets (when there is no post on central.wordcamp.org).

cc @StevenDufresne.

[pkevan]

e.g.

E_NOTICE
Trying to get property 'post_status' of non-object

[StevenDufresne]

Thanks for opening up this PR. Returning false could imply that the camp is open which would be unexpected. Alternatively, we could return null but checking for null would be an annoying interface.

Ideally, we don't ever call this function when a post doesn't exist and I don't know why, excluding misconfigured environments, we would ever be in that context. However, my ignorance of the WordCamp network and codebase is definitely at play here.

With that in mind, what do you think about returning true if there's no post set? If we don't have a post it's more logical that the camp has closed (or never existed).

[pkevan]

Returning false could imply that the camp is open which would be unexpected.

Yes, I realised this when creating the fix, but we would somehow need to support sites which don't have these posts (mostly for testing), so perhaps we add a sandbox or other test to cover that? We could also just adjust the code comment to state why it is returning and the scenarios?

[StevenDufresne]

What about we do something like what symfony does in: ParameterBag

 public function get(string $key, mixed $default = null): mixed
    {
        return \array_key_exists($key, $this->parameters) ? $this->parameters[$key] : $default;
    }

Allow consumers to pass the default, so:

/**
 * Determines if the WordCamp is closed.
 *
 * @param bool $default Optional. Default value to return if no WordCamp post exists. Default true.
 * @return bool True if WordCamp is closed, false otherwise.
 */
public function is_wordcamp_closed( bool $default = true ) {
	$wordcamp = get_wordcamp_post();

	// Return default if no WordCamp post exists.
	if ( false === $wordcamp ) {
		return $default;
	}

	return 'wcpt-closed' === $wordcamp->post_status;
}

It's not completely ideal, but it would allow the consumer to dictate the outcome should the WordCamp not exist and that feels more predictable to me.

[pkevan]

What about we do something like what symfony does in: ParameterBag

 public function get(string $key, mixed $default = null): mixed
    {
        return \array_key_exists($key, $this->parameters) ? $this->parameters[$key] : $default;
    }

Allow consumers to pass the default, so:

/**
 * Determines if the WordCamp is closed.
 *
 * @param bool $default Optional. Default value to return if no WordCamp post exists. Default true.
 * @return bool True if WordCamp is closed, false otherwise.
 */
public function is_wordcamp_closed( bool $default = true ) {
	$wordcamp = get_wordcamp_post();

	// Return default if no WordCamp post exists.
	if ( false === $wordcamp ) {
		return $default;
	}

	return 'wcpt-closed' === $wordcamp->post_status;
}

It's not completely ideal, but it would allow the consumer to dictate the outcome should the WordCamp not exist and that feels more predictable to me.

I'm not sure it's much clearer, but I can see the value of doing something like this.

Shouldn't the default be false though?

[dd32]

Instead of fixing the function, why not fix the cause, that the post can't be found?

[dd32]

Instead of fixing the function, why not fix the cause, that the post can't be found?

After looking at it... yeah some sites it can be fixed, the metadata is missing... but on others..

This mostly happens when a WordCamp has multiple sites (ie. /2024/ and /2024-fr/).