revert: "fix(settings): preserve settings on deactivation" (#1750) - #1771
Closed
NabDevs wants to merge 2 commits into
Closed
revert: "fix(settings): preserve settings on deactivation" (#1750)#1771NabDevs wants to merge 2 commits into
NabDevs wants to merge 2 commits into
Conversation
* Restore carrier settings * fix: narrow settings restore * fix: preserve settings on deactivation
Reverts squash commit 4bbd2ed. The change was merged prematurely and still needs to be tested. Temporary revert to keep main releasable. Re-land after testing by reverting this revert or re-applying the branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
|
✅ Build successful! You can find a link to the downloadable artifact below.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the previously merged “preserve settings on deactivation” change (#1750) to restore the prior deactivation/uninstall behavior and remove the one-off “restore v6 options” repair migration that was merged before it was fully tested.
Changes:
- Reintroduces running the PDK uninstall flow on WordPress plugin deactivation (via
register_deactivation_hook()). - Removes the one-time v6 options restore migration and its unit test coverage.
- Simplifies/removes several WordPress test mocks that were only needed for the removed migration, and updates snapshots accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| woocommerce-myparcel.php | Re-adds a deactivation hook and an uninstall() callback that calls Installer::uninstall(). |
| src/Service/WpInstallerService.php | Restores an override to avoid updating the installed version when the provided version is null. |
| config/pdk.php | Removes the migrationDirectory config that was used for timestamped migration discovery. |
| src/Migration/2026_08_04_101714_restore_v6_options.php | Deletes the one-off repair migration introduced in #1750. |
| tests/Unit/Migration/RestoreV6OptionsTest.php | Deletes unit tests for the removed repair migration. |
| tests/Unit/EntryTest.php | Updates the deactivation-related expectation (but currently in a way that doesn’t actually assert the hook exists). |
| tests/Mock/WordPressOptions.php | Simplifies mocked get_option() behavior. |
| tests/Mock/MockWpdb.php | Removes option-query helpers that were only used by the deleted repair migration tests. |
| tests/mock_wp_functions.php | Removes the delete_option() mock (previously used by the deleted repair migration). |
| tests/snapshots/*.json | Updates snapshots to include the deactivation hook registration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
56
| it('runs uninstall on deactivate', function () { | ||
| MockWpActions::execute('deactivate_woocommerce-myparcel'); | ||
|
|
||
| expect(MockWpActions::toArray())->not->toHaveKey('deactivate_woocommerce-myparcel') | ||
| ->and(WordPressOptions::getOption('_myparcelcom_carrier')) | ||
| ->toBe(['POSTNL' => ['enabled' => true]]); | ||
| expect(MockWpActions::get('deactivate_woocommerce-myparcel'))->toBe([]); | ||
| }); |
Contributor
Author
|
Overbodig: main is één commit teruggezet (history rewrite i.p.v. revert) — #1750 komt na het testen via een nieuwe PR terug. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #1750 (squash commit 4bbd2ed).
The change was merged prematurely — it still needs to be tested. This temporary revert keeps
mainreleasable so the next release does not ship the untested change.Re-landing after testing: restore the original branch (
feat/restore-carrier-settings-reactivation) and either revert this revert or re-apply the commits on a fresh branch (the original commits remain inmain's history, so a plain re-merge will report "nothing to merge").