Skip to content

Commit 8a627a3

Browse files
committed
fix: make the manual retry actually retry the migration
With the overwrite guard in place, "Retry migration" did nothing whenever settings were already stored: the step skipped its work, the migration counted as succeeded and the notice disappeared, which is indistinguishable from "Keep the current settings" and not what the button promised. A retry the user asked for now clears `antispam_bee_options` first, so the legacy settings are migrated again for real. The guard keeps protecting the automatic retries, which must never discard a configuration nobody asked them to touch. The notice wording follows the state it is describing: with settings stored the action reads "Discard the current settings and migrate again" and says what it replaces, and the introduction no longer claims the plugin runs on its defaults when it does not.
1 parent 6635e4d commit 8a627a3

3 files changed

Lines changed: 91 additions & 10 deletions

File tree

src/Admin/MigrationFailureNotice.php

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,13 @@ public static function render(): void {
6767
esc_html__( 'Antispam Bee could not migrate your settings.', 'antispam-bee' )
6868
);
6969

70-
printf(
71-
'<p>%s</p>',
72-
esc_html__( 'The plugin is currently running with its default settings. Your previous settings have not been lost — they are still stored in the database and will be applied as soon as the migration succeeds.', 'antispam-bee' )
73-
);
70+
if ( PluginUpdate::has_stored_settings() ) {
71+
$intro = __( 'The plugin is running with the settings currently stored. Your previous settings have not been lost — they are still in the database, unmigrated.', 'antispam-bee' );
72+
} else {
73+
$intro = __( 'The plugin is currently running with its default settings. Your previous settings have not been lost — they are still stored in the database and will be applied as soon as the migration succeeds.', 'antispam-bee' );
74+
}
75+
76+
printf( '<p>%s</p>', esc_html( $intro ) );
7477

7578
if ( '' !== $state['message'] ) {
7679
printf(
@@ -79,17 +82,32 @@ public static function render(): void {
7982
);
8083
}
8184

85+
/*
86+
* The wording has to follow what the button actually does. Once settings are
87+
* stored — because the user gave up and configured the plugin by hand — a retry
88+
* can only get the old settings back by replacing them, and saying "retry" while
89+
* quietly leaving them in place would be a lie: the migration would skip its work
90+
* and simply report success.
91+
*/
92+
if ( PluginUpdate::has_stored_settings() ) {
93+
$retry_label = __( 'Discard the current settings and migrate again', 'antispam-bee' );
94+
$explanation = __( 'Migrating again replaces the settings currently stored with the result of migrating your previous ones. Choose “Keep the current settings” to stop retrying and leave your settings exactly as they are.', 'antispam-bee' );
95+
} else {
96+
$retry_label = __( 'Migrate again', 'antispam-bee' );
97+
$explanation = __( 'Choose “Keep the current settings” to stop retrying for good and configure the plugin yourself.', 'antispam-bee' );
98+
}
99+
82100
printf(
83101
'<p><a class="button button-primary" href="%s">%s</a> <a class="button" href="%s">%s</a></p>',
84102
esc_url( $retry_url ),
85-
esc_html__( 'Retry migration', 'antispam-bee' ),
103+
esc_html( $retry_label ),
86104
esc_url( $dismiss_url ),
87105
esc_html__( 'Keep the current settings', 'antispam-bee' )
88106
);
89107

90108
printf(
91109
'<p class="description">%s</p>',
92-
esc_html__( 'Retrying never overwrites settings you have already saved. Choose “Keep the current settings” to stop retrying for good and configure the plugin yourself.', 'antispam-bee' )
110+
esc_html( $explanation )
93111
);
94112

95113
echo '</div>';
@@ -104,7 +122,7 @@ public static function render(): void {
104122
public static function handle_retry(): void {
105123
self::authorize( self::RETRY_ACTION );
106124

107-
delete_option( PluginUpdate::FAILURE_OPTION_NAME );
125+
PluginUpdate::reset_for_retry();
108126

109127
self::redirect_back();
110128
}

src/Handlers/PluginUpdate.php

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,31 @@ private static function maybe_update_database(): void {
184184
update_option( self::DB_VERSION_OPTION_NAME, self::get_plugin_version() );
185185
}
186186

187+
/**
188+
* Whether the plugin already has settings of its own stored.
189+
*
190+
* @return bool Whether `antispam_bee_options` holds a configuration.
191+
*/
192+
public static function has_stored_settings(): bool {
193+
$stored = get_option( Settings::OPTION_NAME, null );
194+
195+
return is_array( $stored ) && ! empty( $stored );
196+
}
197+
198+
/**
199+
* Throw away the stored settings so the next run migrates the legacy ones again.
200+
*
201+
* The automatic retries deliberately never touch a stored configuration — see the
202+
* guard in the 3.0.0 step. A retry the user asked for is the opposite situation:
203+
* they are looking at a notice that says their old settings were not migrated and
204+
* are choosing to have them back, so the settings standing in the way have to go,
205+
* or the retry would silently do nothing at all.
206+
*/
207+
public static function reset_for_retry(): void {
208+
delete_option( Settings::OPTION_NAME );
209+
delete_option( self::FAILURE_OPTION_NAME );
210+
}
211+
187212
/**
188213
* Record the database as migrated without running the migration.
189214
*
@@ -299,10 +324,10 @@ protected static function run_migration_steps( string $version_from_db ): void {
299324
* over either would throw away their configuration. The write below is the step's
300325
* last action, so a run cut short left nothing behind and a present array is always
301326
* one of those two. A stored value that is not an array is corrupt, not a
302-
* configuration, and is replaced.
327+
* configuration, and is replaced. A retry the user explicitly asked for clears
328+
* the option first, so this guard only ever holds back the automatic retries.
303329
*/
304-
$existing_options = get_option( Settings::OPTION_NAME, null );
305-
if ( is_array( $existing_options ) && ! empty( $existing_options ) ) {
330+
if ( self::has_stored_settings() ) {
306331
return;
307332
}
308333

tests/Unit/Handlers/PluginUpdateTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,44 @@ public function test_existing_options_are_not_overwritten(): void {
438438
);
439439
}
440440

441+
/**
442+
* A retry the user asked for clears the stored settings, so the migration really runs.
443+
*
444+
* Without this the guard that protects the automatic retries would make the manual one
445+
* skip its work and report success, which is the opposite of what the button promises.
446+
*
447+
* @return void
448+
*/
449+
public function test_resetting_for_a_retry_clears_the_stored_settings(): void {
450+
$this->stub_options(
451+
[
452+
'antispam_bee' => [ 'regexp_check' => 1 ],
453+
'antispam_bee_options' => [ 'comment' => [ 'rule_asb_regexp_active' => '' ] ],
454+
'antispambee_db_version' => '1.02',
455+
'antispambee_db_update_failures' => [
456+
'version' => '3.0.0-beta.1',
457+
'attempts' => PluginUpdate::MAX_UPDATE_ATTEMPTS,
458+
'message' => 'Migration exploded',
459+
'time' => 1,
460+
],
461+
]
462+
);
463+
464+
PluginUpdate::reset_for_retry();
465+
466+
$this->assertContains( Settings::OPTION_NAME, $this->deleted_options );
467+
$this->assertContains( PluginUpdate::FAILURE_OPTION_NAME, $this->deleted_options );
468+
469+
PluginUpdate::maybe_run_plugin_updated_logic();
470+
471+
$this->assertSame(
472+
'on',
473+
$this->written_options[ Settings::OPTION_NAME ]['comment']['rule_asb_regexp_active'],
474+
'The legacy settings are migrated again instead of being skipped.'
475+
);
476+
$this->assertSame( '3.0.0-beta.1', $this->written_options[ PluginUpdate::DB_VERSION_OPTION_NAME ] );
477+
}
478+
441479
/**
442480
* Marking the database as migrated stops the retries without touching the settings.
443481
*

0 commit comments

Comments
 (0)