| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…_enabled() function
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label. Core Committers: Use this line as a base for the props when committing in SVN: Props alecgeatches, ingeniumed, zieladam, peterwilsoncc, tyxla. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Sorry, something went wrong.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Sorry, something went wrong.
There was a problem hiding this comment.
I have a few of suggestions.
My suggestion is to add the following to wp_functionality_constants() and modify the PR for the various implications
/**
* Whether real time collaboration is permitted to be enabled.
*
* @since 7.0.0
*/
if ( ! defined( 'WP_ALLOW_COLLABORATION' ) ) {
if ( false !== getenv( 'WP_ALLOW_COLLABORATION' ) ) {
// Environment variable is defined.
if ( 'true' === getenv( 'WP_ALLOW_COLLABORATION' ) ) {
define( 'WP_ALLOW_COLLABORATION', true );
} else {
define( 'WP_ALLOW_COLLABORATION', false );
}
} else {
// Environment variable is not defined, default to true.
define( 'WP_ALLOW_COLLABORATION', true );
}
}
Sorry, something went wrong.
Thanks for the feedback @peterwilsoncc. I've updated the code with that in mind. I did have to leave the defined check in collaboration.php as it was erroring out without that. Let me know what you think of the updated code 🙏🏾 |
Sorry, something went wrong.
|
Noting that it's now WP_ALLOW_COLLABORATION wherein: true - this means collaboration can be turned on It can be defined in a way to allow hosts to disallow the feature at an environment level. |
Sorry, something went wrong.
| * If the WP_ALLOW_COLLABORATION constant is false, | ||
| * collaboration is always disabled regardless of the database option. | ||
| * Otherwise, falls back to the 'wp_collaboration_enabled' option. |
There was a problem hiding this comment.
There's a potential edge case to verify here: if wp_is_collaboration_enabled() is called before WP_ALLOW_COLLABORATION is defined, RTC will also be disabled regardless of the database option.
Can we verify that this function can't be called before the constant is defined? If we can't verify this, or we found a way to call wp_is_collaboration_enabled() before the constant was declared, can we amend the docs to make this edge case clear?
Sorry, something went wrong.
There was a problem hiding this comment.
The constant is defined in wp_functionality_constants(); which is called in wp-settings.php (after require "collaboration.php") so the premature reference would have to happen at some point in the WordPress boot flow before that call (or via SHORTINIT). Two possible solutions I see are:
function wp_is_collaboration_enabled() {
if ( ! defined( 'WP_ALLOW_COLLABORATION' ) ) {
$env_value = getenv( 'WP_ALLOW_COLLABORATION' );
if ( false !== $env_value ) {
define( 'WP_ALLOW_COLLABORATION', 'true' === $env_value );
} else {
// Environment variable is not defined, default to true.
define( 'WP_ALLOW_COLLABORATION', true );
}
}
if ( ! WP_ALLOW_COLLABORATION ) {
return false
}
return (bool) get_option( 'wp_collaboration_enabled' );
}Which would be fine with me. There's a precedence of a similar behavior in kses.php:
Sorry, something went wrong.
There was a problem hiding this comment.
I suggest we keep this part as it is for beta6 and revisit it in RC1 cc @ellatrix
Sorry, something went wrong.
There was a problem hiding this comment.
Confirming that this makes sense to me 👍
Sorry, something went wrong.
There was a problem hiding this comment.
The constant is defined in wp_functionality_constants(); which is called in wp-settings.php (after require "collaboration.php") so the premature reference would have to happen at some point in the WordPress boot flow before that call (or via SHORTINIT). Two possible solutions I see are:
Thanks for catching this, I hadn't noticed the order of operations.
Of the two suggestions I think defining the constant in wp_is_collaboration_enabled() is the safest approach for RC1. @adamziel are you able to look in to this in #11322 (I'm at a mate's 50th today so won't have a chance, sorry).
Sorry, something went wrong.
There was a problem hiding this comment.
@peterwilsoncc I started a new PR for that here: #11333
Sorry, something went wrong.
| <input name="wp_collaboration_enabled" type="checkbox" id="wp_collaboration_enabled" value="1" <?php checked( '1', (bool) get_option( 'wp_collaboration_enabled' ) ); ?> /> | ||
| <?php _e( 'Enable real-time collaboration' ); ?> | ||
| </label> | ||
| <?php if ( ! WP_ALLOW_COLLABORATION ) : ?> |
There was a problem hiding this comment.
I think we should also have a defined() check before we actually use the constant, just in case someone is loading the writing settings admin page in an unconventional way.
Sorry, something went wrong.
There was a problem hiding this comment.
Something like
| <?php if ( ! WP_ALLOW_COLLABORATION ) : ?> | |
| <?php if ( ! defined( 'WP_ALLOW_COLLABORATION' ) || true === WP_ALLOW_COLLABORATION ) : ?> |
Sorry, something went wrong.
There was a problem hiding this comment.
I'll take the liberty of merging the suggestion. Feel free to rollback my commit if needed.
Sorry, something went wrong.
There was a problem hiding this comment.
@adamziel this seems wrong; did you mean:
<?php if ( ! defined( 'WP_ALLOW_COLLABORATION' ) || false === WP_ALLOW_COLLABORATION ) : ?>If we make it true we're inverting what we had there before, which looks unintentional.
Sorry, something went wrong.
There was a problem hiding this comment.
My intention was "if collaboration is allowed, allow the user to decide whether they want to use it". Yours reads to me like "If collaboration is implicitly allowed by not defining the constant OR the collaboration is explicitly not allowed, then allow the user to decide" which seems off to me.
Sorry, something went wrong.
There was a problem hiding this comment.
Ah no, you're right and I'm wrong. Good spot. I'll open a PR to correct this, CC @ellatrix
Sorry, something went wrong.
|
wp_functionality_constants() doesn't seem to be called during SHORTINIT, so any code path that refers to WP_ALLOW_COLLABORATION will fail with: Fatal error: Uncaught Error: Undefined constant "WP_ALLOW_COLLABORATION" in What would be the best way to document that? While the same is true for AUTOSAVE_INTERVAL and other constants defined in wp_functionality_constants(), WP_ALLOW_COLLABORATION seems to have some potential to pop up in different places around the PHP codebase. |
Sorry, something went wrong.
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
There was a problem hiding this comment.
Looks good to me and works as intended. Note: I've merged a few suggestions @tyxla and I proposed. I know it's not the typical workflow, but I took that liberty in the name of beta 6. I think they're good additions and I'm also happy for any of them to be reverted as needed.
Sorry, something went wrong.
This provides an easy way at config level to disable real-time collaboration. Developed in: #11311. See #64904. Props alecgeatches, ingeniumed, zieladam, peterwilsoncc, tyxla. git-svn-id: https://develop.svn.wordpress.org/trunk@62075 602fd350-edb4-49c9-b593-d223f7449a82
This provides an easy way at config level to disable real-time collaboration. Developed in: WordPress/wordpress-develop#11311. See #64904. Props alecgeatches, ingeniumed, zieladam, peterwilsoncc, tyxla. Built from https://develop.svn.wordpress.org/trunk@62075 git-svn-id: http://core.svn.wordpress.org/trunk@61357 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
Closing since it's been committed |
Sorry, something went wrong.
The WP_ALLOW_COLLABORATION constant was introduced in PR WordPress#11311 to support disabling RTC at the host level or at the wp-config.php level. A part of that PR was hiding the "Enable RTC" checkbox in the writing options when RTC is explicitly disallowed via the constant. In the process of reviewing the change and merging suggestions, I have used the wrong condition to hide that checkbox. This PR makes the condition right. Props to @tyxla for noticing! \# Testing instructions * Set the WP_ALLOW_COLLABORATION constant to false, go to writing options, confirm the checkbox is replaced by a message "Real-time collaboration has been disabled". * Set it to true, confirm the checkbox is there. * Remove the constnat, confirm the checkbox is there.
| Back | FazBrowse Home | New Git URL |
Update from @ingeniumed:
Trac ticket: https://core.trac.wordpress.org/ticket/64904
Screenshots
With
With
or no WP_ALLOW_COLLABORATION configuration at all, the default checkbox:
Use of AI Tools
AI assistance: Yes
Tool(s): Claude
Model(s): Opus 4.6
Used for: Implementation