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.

Sign up for GitHub

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

Harmonize module options #1158

Merged
merged 2 commits into from
May 1, 2022
Merged

Harmonize module options #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b69c128
Harmonize module options
herrvigg May 1, 2022
580ddb6
Clarify migration -> import vs rename
herrvigg May 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion admin/qtx_activation_hook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,6 @@ function qtranxf_clear_debug_log() {
}
}


function qtranxf_activation_hook() {
qtranxf_clear_debug_log();
if ( version_compare( PHP_VERSION, '5.4' ) < 0 ) {
Expand Down Expand Up @@ -785,6 +784,11 @@ function qtranxf_activation_hook() {
qtranxf_admin_notice_deactivate_plugin( 'qTranslate-X', 'qtranslate-x/qtranslate.php' );
}

// Migrate (rename/import) legacy options, temporary transitions during evolutions.
qtranxf_rename_legacy_option( 'qtranslate_modules', QTX_OPTIONS_MODULES_STATE );
qtranxf_import_legacy_option( 'acf_qtranslate', QTX_OPTIONS_MODULE_ACF, false );
qtranxf_import_legacy_option( 'qts_options', QTX_OPTIONS_MODULE_SLUGS, false );

$ts = time();
$next_thanks = get_option( 'qtranslate_next_thanks' );
$check_qtranslate_forks = $next_thanks === false;
Expand Down
36 changes: 36 additions & 0 deletions admin/qtx_admin_options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,39 @@ function qtranxf_admin_load_config() {

qtranxf_add_conf_filters();
}

/**
* Import legacy option by recopying its value if the new option doesn't already exist.
*
* @param string $old_name
* @param string $new_name
* @param bool|string $autoload as in update_option
*
* @return void
*/
function qtranxf_import_legacy_option( $old_name, $new_name, $autoload = null ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this function return update_option result (and true for other cases)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't found a case where I'd need it.

assert( strpos( $new_name, 'qtranslate_' ) === 0 );
if ( ! get_option( $new_name ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure new named option should always have the priority.
Maybe we could test also module activation state (test above AND module is active) though maybe involves a too remote scenario to complicate the function...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea of the import is a one-shot action at activation, the goal is that QT-XT cares of its own options (modules).

The normal scenario is that new options don't exist yet. If we can import a value such as ACF or slugs legacy settings (coming from original plugins) the new option is created from these values to ensure the transition from the old plugins. But from that point we want to remain with the QT-XT setup and ignore the legacy stuff once we have all set in QT-XT.

The most tricky case is when the QT-XT options are reset, we don't really want to import again but in that case we expect the admin to save a new set of options right after the reset. If this is not done, a new import can happen again.

This patch is only valid for the modules as it's a new feature in QT-XT but this will be expanded to all other options later once we cut completely the links with QT-X (not XT).

$old_value = get_option( $old_name );
if ( $old_value ) {
update_option( $new_name, $old_value, $autoload );
}
}
}

/**
* Rename a legacy option: import and cleanup.
* Only applies to `qtranslate` options, preserve external plugin options.
*
* @param string $old_name
* @param string $new_name
* @param bool|string $autoload as in update_option
*
* @return void
*/
function qtranxf_rename_legacy_option( $old_name, $new_name, $autoload = null ) {
assert( strpos( $new_name, 'qtranslate_' ) === 0 );
assert( strpos( $old_name, 'qtranslate_' ) === 0 );
qtranxf_import_legacy_option( $old_name, $new_name, $autoload );
delete_option( $old_name );
Copy link
Contributor

Choose a reason for hiding this comment

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

Better test qtranxf_import_legacy_option before doing this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With a rename the old option should be deleted in any case or if it it appears again.
If the new option exist, it should not be overwritten. But the old option should removed even if nothing was updated.

}
4 changes: 3 additions & 1 deletion admin/qtx_admin_options_update.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,9 @@ function qtranxf_reset_config() {
// internal private options not loaded by default
delete_option( 'qtranslate_next_update_mo' );
delete_option( 'qtranslate_next_thanks' );
delete_option( 'qtranslate_modules_state' );
delete_option( QTX_OPTIONS_MODULES_STATE );
delete_option( QTX_OPTIONS_MODULE_ACF );
delete_option( QTX_OPTIONS_MODULE_SLUGS );
Comment on lines +363 to +365
Copy link
Contributor

Choose a reason for hiding this comment

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

These would look better somewhere in modules source files to make things cleaner, maybe a new function to be called from here.
It would be better to segregate code which depends on module list from core files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also thought this, but this belongs more to general admin functionalities for the whole QT-XT plugin. For the same reason as for the option names, better keep those with the rest and not split them to avoid the risks of missing updates/deletions.


// obsolete options
delete_option( 'qtranslate_custom_pages' );
Expand Down
19 changes: 12 additions & 7 deletions modules/acf/src/plugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public function __construct() {
public function init() {
static $plugin_loaded = false;
if ( ! $plugin_loaded && $this->acf_enabled() ) {
// TODO: remove temporary import of legacy option for master, rely on plugin activation in next release.
require_once( QTRANSLATE_DIR . '/admin/qtx_admin_options.php' );
qtranxf_import_legacy_option( 'acf_qtranslate', QTX_OPTIONS_MODULE_ACF, false );

if ( $this->acf_major_version() === 5 ) {
require_once ACF_QTRANSLATE_PLUGIN_DIR . 'src/acf_5/acf.php';
$this->acf = new acf_qtranslate_acf_5( $this );
Expand Down Expand Up @@ -230,7 +234,7 @@ public function filter_qtranslate_admin_config( $config ) {
* @return mixed
*/
function get_plugin_setting( $name, $default = null ) {
$options = get_option( 'acf_qtranslate' );
$options = get_option( QTX_OPTIONS_MODULE_ACF );
if ( isset( $options[ $name ] ) ) {
return $options[ $name ];
}
Expand All @@ -242,7 +246,7 @@ function get_plugin_setting( $name, $default = null ) {
* Register settings and default fields
*/
function admin_init() {
register_setting( 'settings-qtranslate-acf', 'acf_qtranslate' );
register_setting( 'settings-qtranslate-acf', QTX_OPTIONS_MODULE_ACF );

// Define a placeholder for the fields without title or content.
add_settings_section(
Expand Down Expand Up @@ -289,8 +293,8 @@ function update_settings() {
if ( ! isset( $_POST['nonce_acf'] ) || ! wp_verify_nonce( $_POST['nonce_acf'], 'acf' ) ) {
return;
}
$options = isset( $_POST['acf_qtranslate'] ) ? $_POST['acf_qtranslate'] : null;
update_option( 'acf_qtranslate', $options );
$options = isset( $_POST[ QTX_OPTIONS_MODULE_ACF ] ) ? $_POST[ QTX_OPTIONS_MODULE_ACF ] : null;
update_option( QTX_OPTIONS_MODULE_ACF, $options, false );
}

/**
Expand All @@ -299,7 +303,7 @@ function update_settings() {
function render_setting_translate_standard_field_types() {
?>
<input type="checkbox"
name="acf_qtranslate[translate_standard_field_types]" <?php checked( $this->get_plugin_setting( 'translate_standard_field_types' ), 1 ); ?>
name="<?php echo QTX_OPTIONS_MODULE_ACF ?>[translate_standard_field_types]" <?php checked( $this->get_plugin_setting( 'translate_standard_field_types' ), 1 ); ?>
value="1">
<?php
}
Expand All @@ -310,7 +314,7 @@ function render_setting_translate_standard_field_types() {
function render_setting_show_language_tabs() {
?>
<input type="checkbox"
name="acf_qtranslate[show_language_tabs]" <?php checked( $this->get_plugin_setting( 'show_language_tabs' ), 1 ); ?>
name="<?php echo QTX_OPTIONS_MODULE_ACF ?>[show_language_tabs]" <?php checked( $this->get_plugin_setting( 'show_language_tabs' ), 1 ); ?>
value="1">
<?php
}
Expand All @@ -320,7 +324,8 @@ function render_setting_show_language_tabs() {
*/
function render_setting_show_on_pages() {
?>
<textarea name="acf_qtranslate[show_on_pages]" style="max-width:500px;width:100%;height:200px;padding-top:6px"
<textarea name="<?php echo QTX_OPTIONS_MODULE_ACF ?>[show_on_pages]"
style="max-width:500px;width:100%;height:200px;padding-top:6px"
placeholder="post.php"><?= esc_html( $this->get_plugin_setting( 'show_on_pages' ) ) ?></textarea><br>
<small>Enter each page on it's own line</small>
<?php
Expand Down
8 changes: 4 additions & 4 deletions modules/qtx_admin_module_manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static function register_hooks() {
* AND
* - if the `admin_enabled_modules` admin option is checked for that module.
*
* Update the 'qtranslate_modules_state' option, telling which module should be loaded.
* Update the QTX_OPTIONS_MODULES_STATE option, telling which module should be loaded.
* Note each module can enable hooks both for admin and front requests.
*
* @param callable $func_is_active callback to evaluate if a plugin is active
Expand All @@ -45,8 +45,8 @@ public static function update_modules_state( $func_is_active = 'is_plugin_active
$option_modules[ $module->id ] = $state;
}

$old_option_modules = get_option( 'qtranslate_modules_state' );
update_option( 'qtranslate_modules_state', $option_modules );
$old_option_modules = get_option( QTX_OPTIONS_MODULES_STATE );
update_option( QTX_OPTIONS_MODULES_STATE, $option_modules );

// Trigger info notices and potential loading only if changed.
if ( $old_option_modules != $option_modules ) {
Expand Down Expand Up @@ -132,7 +132,7 @@ public static function register_plugin_deactivated( $deactivated_plugin ) {
}

public static function admin_notices() {
$options_modules = get_option( 'qtranslate_modules_state', array() );
$options_modules = get_option( QTX_OPTIONS_MODULES_STATE, array() );
if ( empty( $options_modules ) ) {
$msg = '<p>' . sprintf( __( 'Modules state undefined in %s. Please deactivate it and reactivate it again from the plugins page.', 'qtranslate' ), 'qTranslate&#8209;XT' ) . '</p>';
$nonce = wp_create_nonce( 'deactivate-plugin_qtranslate-xt/qtranslate.php' );
Expand Down
2 changes: 1 addition & 1 deletion modules/qtx_admin_module_settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function has_settings() {
* @return QTX_Admin_Module_Settings[]
*/
public static function get_settings_modules() {
$states = get_option( 'qtranslate_modules_state', array() );
$states = get_option( QTX_OPTIONS_MODULES_STATE, array() );
$settings = array();
foreach ( QTX_Admin_Module::get_modules() as $module ) {
$state = isset( $states[ $module->id ] ) ? $states[ $module->id ] : QTX_MODULE_STATE_UNDEFINED;
Expand Down
4 changes: 2 additions & 2 deletions modules/qtx_module_loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class QTX_Module_Loader {
* @bool true if module active.
*/
public static function is_module_active( $module_id ) {
$modules_state = get_option( 'qtranslate_modules_state', array() );
$modules_state = get_option( QTX_OPTIONS_MODULES_STATE, array() );

return isset( $modules_state[ $module_id ] ) && $modules_state[ $module_id ] === QTX_MODULE_STATE_ACTIVE;
}
Expand All @@ -31,7 +31,7 @@ public static function is_module_active( $module_id ) {
* Note also the modules should be loaded before "qtranslate_init_language" is triggered.
*/
public static function load_active_modules() {
$modules_state = get_option( 'qtranslate_modules_state', array() );
$modules_state = get_option( QTX_OPTIONS_MODULES_STATE, array() );

foreach ( $modules_state as $module_id => $state ) {
if ( $state === QTX_MODULE_STATE_ACTIVE ) {
Expand Down
6 changes: 5 additions & 1 deletion modules/slugs/includes/class-qtranslate-slug.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ function init() {
return;
}

$this->options_buffer = get_option( QTS_OPTIONS_NAME, array() );
// TODO: remove temporary import of legacy option for master, rely on plugin activation in next release.
require_once( QTRANSLATE_DIR . '/admin/qtx_admin_options.php' );
qtranxf_import_legacy_option( 'qts_options', QTX_OPTIONS_MODULE_SLUGS, false );

$this->options_buffer = get_option( QTX_OPTIONS_MODULE_SLUGS, array() );
$this->permalink_structure = get_option( 'permalink_structure' );

if ( ! is_admin() ) {
Expand Down
2 changes: 1 addition & 1 deletion modules/slugs/includes/qtranslate-slug-admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function qts_multi_activate() {
function qts_uninstall() {
global $q_config, $wpdb;

delete_option( QTS_OPTIONS_NAME );
delete_option( QTX_OPTIONS_MODULE_SLUGS );

$meta_keys = array();
foreach ( $q_config['enabled_languages'] as $lang ) {
Expand Down
26 changes: 13 additions & 13 deletions modules/slugs/includes/qtranslate-slug-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function qts_show_form_field( $args = array() ) {
$choices = $args['choices'];
$class = $args['class'];

$options = $qtranslate_slug->options_buffer ? $qtranslate_slug->options_buffer : get_option( QTS_OPTIONS_NAME, array() );
$options = $qtranslate_slug->options_buffer ? $qtranslate_slug->options_buffer : get_option( QTX_OPTIONS_MODULE_SLUGS, array() );

// pass the standard value if the option is not yet set in the database
if ( ! isset( $options[ $id ] ) && $type != 'checkbox' ) {
Expand All @@ -67,7 +67,7 @@ function qts_show_form_field( $args = array() ) {
case 'text':
$options[ $id ] = stripslashes( $options[ $id ] );
$options[ $id ] = esc_attr( $options[ $id ] );
echo "<input class='regular-text$field_class' type='text' id='$id' name='" . QTS_OPTIONS_NAME . "[$id]' value='$options[$id]' />";
echo "<input class='regular-text$field_class' type='text' id='$id' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]' value='$options[$id]' />";
echo ( $desc != '' ) ? "<br /><p class='qtranxs-notes'>$desc</p>" : "";
break;

Expand All @@ -85,21 +85,21 @@ function qts_show_form_field( $args = array() ) {
} else {
$value = '';
}
echo "<label for=" . QTS_OPTIONS_NAME . "[$id|${item[1]}]>" . $item[0] . "</label> " .
"<input class='$field_class' type='text' id='$id|${item[1]}' name='" . QTS_OPTIONS_NAME . "[$id|${item[1]}]' value='" . urldecode( $value ) . "' /><br/>";
echo "<label for=" . QTX_OPTIONS_MODULE_SLUGS . "[$id|${item[1]}]>" . $item[0] . "</label> " .
"<input class='$field_class' type='text' id='$id|${item[1]}' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id|${item[1]}]' value='" . urldecode( $value ) . "' /><br/>";
}
echo ( $desc != '' ) ? "<p class='qtranxs-notes'>$desc</p>" : "";
break;

case 'textarea':
$options[ $id ] = stripslashes( $options[ $id ] );
$options[ $id ] = esc_html( $options[ $id ] );
echo "<textarea class='textarea$field_class' type='text' id='$id' name='" . QTS_OPTIONS_NAME . "[$id]' rows='5' cols='30'>$options[$id]</textarea>";
echo "<textarea class='textarea$field_class' type='text' id='$id' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]' rows='5' cols='30'>$options[$id]</textarea>";
echo ( $desc != '' ) ? "<br /><p class='qtranxs-notes'>$desc</p>" : "";
break;

case 'select':
echo "<select id='$id' class='select$field_class' name='" . QTS_OPTIONS_NAME . "[$id]'>";
echo "<select id='$id' class='select$field_class' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]'>";
foreach ( $choices as $item ) {
$value = esc_attr( $item );
$item = esc_html( $item );
Expand All @@ -112,7 +112,7 @@ function qts_show_form_field( $args = array() ) {
break;

case 'select2':
echo "<select id='$id' class='select$field_class' name='" . QTS_OPTIONS_NAME . "[$id]'>";
echo "<select id='$id' class='select$field_class' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]'>";
foreach ( $choices as $item ) {

$item = explode( "|", $item );
Expand All @@ -126,7 +126,7 @@ function qts_show_form_field( $args = array() ) {
break;

case 'checkbox':
echo "<input class='checkbox$field_class' type='checkbox' id='$id' name='" . QTS_OPTIONS_NAME . "[$id]' value='1' " . checked( $options[ $id ], 1, false ) . " />";
echo "<input class='checkbox$field_class' type='checkbox' id='$id' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]' value='1' " . checked( $options[ $id ], 1, false ) . " />";
echo ( $desc != '' ) ? "<br /><p class='qtranxs-notes'>$desc</p>" : "";
break;

Expand All @@ -144,7 +144,7 @@ function qts_show_form_field( $args = array() ) {
}
}

echo "<input class='checkbox$field_class' type='checkbox' id='$id|$item[1]' name='" . QTS_OPTIONS_NAME . "[$id|$item[1]]' value='1' $checked /> $item[0] <br/>";
echo "<input class='checkbox$field_class' type='checkbox' id='$id|$item[1]' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id|$item[1]]' value='1' $checked /> $item[0] <br/>";
}
echo ( $desc != '' ) ? "<br /><p class='qtranxs-notes'>$desc</p>" : "";
break;
Expand All @@ -162,7 +162,7 @@ function qts_show_form_field( $args = array() ) {
$checked = 'checked="checked"';
}

echo "<label for='$id|$item_value'><input class='radio$field_class' type='radio' id='$id|$item_value' name='" . QTS_OPTIONS_NAME . "[$id]' value='$item_value' $checked /> <strong>$item_key</strong>";
echo "<label for='$id|$item_value'><input class='radio$field_class' type='radio' id='$id|$item_value' name='" . QTX_OPTIONS_MODULE_SLUGS . "[$id]' value='$item_value' $checked /> <strong>$item_key</strong>";
if ( isset( $desc[ $index ] ) && ! empty( $desc[ $index ] ) ) {
echo ": " . $desc[ $index ];
}
Expand Down Expand Up @@ -347,8 +347,8 @@ function qts_validate_options( $input ) {
function qts_update_settings() {
global $qtranslate_slug;
$qts_settings = false;
if ( isset( $_POST[ QTS_OPTIONS_NAME ] ) ) {
$qts_settings = qts_validate_options( $_POST[ QTS_OPTIONS_NAME ] );
if ( isset( $_POST[ QTX_OPTIONS_MODULE_SLUGS ] ) ) {
$qts_settings = qts_validate_options( $_POST[ QTX_OPTIONS_MODULE_SLUGS ] );
}

if ( ! $qts_settings || empty( $qts_settings ) ) {
Expand All @@ -357,7 +357,7 @@ function qts_update_settings() {
if ( $qtranslate_slug->options_buffer == $qts_settings ) {
return;
}
update_option( QTS_OPTIONS_NAME, $qts_settings );
update_option( QTX_OPTIONS_MODULE_SLUGS, $qts_settings, false );
$qtranslate_slug->options_buffer = $qts_settings;
flush_rewrite_rules();
}
Expand Down
3 changes: 0 additions & 3 deletions modules/slugs/slugs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
if ( ! defined( "QTS_PREFIX" ) ) {
define( "QTS_PREFIX", '_qts_' );
}
if ( ! defined( "QTS_OPTIONS_NAME" ) ) {
define( "QTS_OPTIONS_NAME", 'qts_options' );
}
if ( ! defined( "QTS_META_PREFIX" ) ) {
define( "QTS_META_PREFIX", QTS_PREFIX . 'slug_' );
}
Expand Down
14 changes: 13 additions & 1 deletion qtranslate_options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,22 @@
// Language code format: ISO 639-1 (2 alpha), 639-2 or 639-3 (3 alpha)
define( 'QTX_LANG_CODE_FORMAT', '[a-z]{2,3}' );

/**
* Option names.
*/
const QTX_OPTIONS_MODULES_STATE = 'qtranslate_modules_state';
const QTX_OPTIONS_MODULE_ACF = 'qtranslate_module_acf';
const QTX_OPTIONS_MODULE_SLUGS = 'qtranslate_module_slugs';
Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

These definitions would look better in modules source files.
It would be better to segregate code which depends on module list from core files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is that all options definitions should be kept consistent in a unique place (the options names are used both for front and admin). Currently it's hard to to understand where a general option is set or read in the code because the values are spread everywhere and most of the settings are generated from intermediate strings (with qtranslate_ prefix). The modules should not be fundamentally different regarding the options interface. The other options should also converge to something more comprehensive.


/**
* @global array Global configuration, interpreted from settings and i18n configuration loaded from JSON.
*/
global $q_config;
global $qtranslate_options;

/**
* @global array Global options, mapped at a lower level to the settings.
*/
global $qtranslate_options;

/**
* array of default option values
Expand Down