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

Introduce the class PLL_Options #1237

Closed
wants to merge 17 commits into from
Closed

Introduce the class PLL_Options #1237

wants to merge 17 commits into from

Conversation

Screenfeed
Copy link
Contributor

@Screenfeed Screenfeed commented Mar 17, 2023

Closes https://github.com/polylang/polylang-pro/issues/1160
Fixes #1236

This PR introduces a new class PLL_Options to manage Polylang's options.
This is requirement if someday we want to use a dependency Injection container.

Key points

  • This class implements ArrayAccess, meaning that values can still be accessed like with an array.
  • It also implements Iterator (and Countable), meaning that we can still use foreach.
  • It also implements JsonSerializable, meaning that using json_encode() will encode the options, not the class (this requires JSON to be available on the server though, which is always the case).
  • The options are automatically stored into the dabase on shutdown if they have been modified. Note: the system is not perfect but enough for what we do: changing twice a value to revert it to its initial value is still counted as "modified", and will trigger an update. For example:
var_dump( $options['default_language'] ); // 'en'
$options['default_language'] = 'fr'; // An update will be triggered.
$options['default_language'] = 'en'; // An update will still be triggered.
  • switch_to_blog() is handled: the class stores the options of each necessary blog. It starts with the current blog by storing its options locally, then on the switch_block hook it will switch to the options of this blog (but fetch them first if not already stored locally) and consider them as "current options" (but still keep the options of the previous blog locally).
  • Options are white-listed: it is not possible to add unknown options (not listed in PLL_Options::DEFAULTS). Ex:
$options['foo'] = 4;
var_dump( $options['foo'] ); // null
  • Options are always defined: it is not possible to unset them from the list, they are set to their default value instead. Ex:
var_dump( $options['default_language'] ); // 'en'
$options['default_language'] = 'fr';
unset( $options['default_language'] );
var_dump( $options['default_language'] ); // 'en'
  • Automatic cast + limited sanitization/validation: while values sent to the class should still be sanitized and validated, the class handles a part of this work. This is done when fetching the values from the database, and when values are updated locally in the class. This ensures we're working with the right types all along.
    • Invalid values are rejected:
var_dump( $options['default_language'] ); // 'en'
$options['default_language'] = 4; // Not a string.
var_dump( $options['default_language'] ); // 'en'
    • Values are casted:
$options['hide_default'] = 1;
var_dump( $options['hide_default'] ); // true
$options['force_lang'] = '2';
var_dump( $options['force_lang'] ); // 2 (int)
    • Values are partially sanitized and partially validated:
$options['domains'] = array(
	''   => 'example.org',
	'en' => '',
	'fr' => 'poof',
);
var_dump( $options['domains'] ); // `array( 'fr' => 'poof' )`: note that the domain is invalid but still stored.
    • For media and nav_menus, we only make sure that the array keys are non falsy strings.
  • Using array_merge() on options will trigger a fatal error, so the method PLL_Options::merge() has been created:
$this->options->merge(
	array( 'hide_default' => true ),
	array( 'browser' => true, 'force_lang' => 2 ),
	array( 'media_support' => true )
);
  • PLL_Options::get_reset_options() is used when $options['version'] is empty (plugin init, plugin activation). PLL_Install::get_default_options() is a wrapper of this method. The word "reset" is used here instead of "default":
    • "default" is the "zero state" of the values: $options['hide_default'] = false;.
    • "reset" are the values on install: $options['hide_default'] = true;.
    • We could consider other wordings, like get_install_options() for example.

PHPStan

This allows some type checks on our options. For example, PHPStan knows that $options['default_lang'] is a string (that can be an empty string).
That's why some things have been removed in 75998cb.

Other

  • All update_option( 'polylang', $options ) have been removed.
  • In PLL_Install/PLL_Install_Base we're still using get_option() and update_option() because PLL() is not yet available in most cases.
  • All $options class properties have been tagged PLL_Options instead of array (this allowed better PHPStan checks).

Polylang Pro and Polylang for WooCommerce

  • Both plugins must be updated.

@Screenfeed Screenfeed added this to the 3.5 milestone Mar 17, 2023
@Screenfeed Screenfeed self-assigned this Mar 17, 2023
@Screenfeed Screenfeed linked an issue Mar 17, 2023 that may be closed by this pull request
@Screenfeed Screenfeed marked this pull request as draft March 17, 2023 15:58
@Hug0-Drelon Hug0-Drelon self-requested a review March 20, 2023 13:19
Base automatically changed from 3.4-dev to master March 23, 2023 09:51
'sync' => array(),
'taxonomies' => array(),
'uninstall' => false,
'version' => '',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #1267 in mind, I believe that this should default to POLYLANG_VERSION.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do that so we can detect the first install.
I'll double-check if it's still necessary.

@Screenfeed
Copy link
Contributor Author

We have a language_taxonomies entry since #1242.

@Screenfeed
Copy link
Contributor Author

We have a language_taxonomies entry since #1242.

Done in 979f3ac.

@Screenfeed
Copy link
Contributor Author

The test failing on multisite is fixed in #1319.

@Screenfeed Screenfeed marked this pull request as ready for review July 17, 2023 12:45
@Chouby Chouby modified the milestones: 3.5, 3.6 Aug 22, 2023
@Chouby Chouby modified the milestones: 3.6, 3.7 Mar 1, 2024
@Screenfeed
Copy link
Contributor Author

New entries:

3rd_party_taxonomies
language_taxonomies
machine_translation_enabled
machine_translation_services
hide_language_from_content_option

@Screenfeed
Copy link
Contributor Author

Replaced by #1488.

@Screenfeed Screenfeed closed this Jun 24, 2024
@Screenfeed Screenfeed deleted the options-class branch June 24, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The class PLL_OLT_Manager should not unset its properties
2 participants