fix: respect keepUserData on app uninstall#64
Open
Ghaith (Gaitholabi) wants to merge 2 commits into
Open
Conversation
Shopware's app.deleted webhook carries a keepUserData flag. Honor it: keep the shop in the repository when true and delete it otherwise, forward the flag to BeforeShopDeletionEvent/ShopDeletedEvent via a keepUserData() accessor, and record it on the uninstall log line. Mirrors shopware/shopware#17466 (retain the app secret iff keepUserData). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghaith (Gaitholabi)
requested review from
Aydin Hassan (AydinHassan) and
Jonas Elfering (keulinho)
Ghaith (Gaitholabi)
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s app.deleted lifecycle handling to respect Shopware’s keepUserData uninstall flag, preserving the shop record (and thus credentials) when merchants choose to keep app data for a future re-install.
Changes:
- Parse
data.payload.keepUserDatafrom theapp.deletedwebhook body and conditionally skip shop deletion whentrue. - Extend
BeforeShopDeletionEventandShopDeletedEventwith akeepUserData()accessor (and constructor argument) to propagate the flag to listeners. - Add/adjust PHPUnit coverage for both keep/delete paths and include the flag in the structured uninstall log context.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/AppLifecycle.php |
Reads keepUserData from webhook payload, conditionally deletes the shop, forwards the flag to events, and logs it. |
src/Event/BeforeShopDeletionEvent.php |
Adds keepUserData state + accessor to the “before deletion” lifecycle event. |
src/Event/ShopDeletedEvent.php |
Adds keepUserData state + accessor to the “deleted” lifecycle event. |
tests/AppLifecycleTest.php |
Adds tests for keeping vs deleting based on the flag and updates uninstall logging expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+103
to
+109
| private function shouldKeepUserData(RequestInterface $request): bool | ||
| { | ||
| $body = \json_decode($request->getBody()->getContents(), true); | ||
| $request->getBody()->rewind(); | ||
|
|
||
| return \is_array($body) && ($body['data']['payload']['keepUserData'] ?? false) === true; | ||
| } |
shouldKeepUserData() decoded without JSON_THROW_ON_ERROR, so an invalid/empty payload became null, fell through to keepUserData=false, and deleted the shop. A JSON parse failure means the webhook body is corrupt: decode with JSON_THROW_ON_ERROR and rethrow as MalformedWebhookBodyException (the SDK's malformed-webhook error, as used throughout ContextResolver) instead of dropping the shop. A valid payload missing the key still defaults to false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghaith (Gaitholabi)
force-pushed
the
feat/respect-keep-user-data
branch
from
f40ff1d to
50b9a22
Compare
Jonas Elfering (keulinho)
approved these changes
Jun 17, 2026
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.
Why
Shopware's
AppDeletedEventcarries akeepUserDataflag in theapp.deletedwebhook payload (['keepUserData' => bool]). When a merchant uninstalls an app and opts to keep its data, Shopware retains the app data on its side — but the SDK ignored the flag and unconditionally deleted the shop from the repository, dropping the credentials a later re-install would restore from.What
AppLifecycle::delete()now readsdata.payload.keepUserDatafrom the webhook body (defaulting tofalse) and:true, deletes it otherwise;BeforeShopDeletionEventandShopDeletedEventthrough a newkeepUserData()accessor (mirrorsAppDeletedEvent::keepUserData()in core);Shop uninstalledlog line.Keeping the record is safe for re-install:
RegistrationService::register()already handles an existing shop via itsupdateShopbranch.This is the SDK-side mirror of shopware/shopware#17466, which makes Shopware retain the app secret iff
keepUserData— keeping both sides symmetric (retain credentials exactly when the data is kept).Compatibility
Additive only — an optional
bool $keepUserData = falseconstructor argument plus one method on each of the two deletion events. Existingnew ShopDeletedEvent($request, $shop)callers are unaffected.Tests
Full
phpunitsuite green;phpstanandphp-cs-fixerclean. AddstestUninstallKeepsShopWhenKeepUserDataIsTrueandtestUninstallDeletesShopWhenKeepUserDataIsFalse, and updates the existing uninstall-log assertion.🤖 Generated with Claude Code