Skip to content

ECTO-2770 - ECTO-2770 - Report the real purchase result when the moda… - #193

Merged
Jannik (qysp) merged 1 commit into
epic/ECTO-2267from
bugfix/ECTO-2770-2
Aug 3, 2026
Merged

ECTO-2770 - ECTO-2770 - Report the real purchase result when the moda…#193
Jannik (qysp) merged 1 commit into
epic/ECTO-2267from
bugfix/ECTO-2770-2

Conversation

@qysp

Copy link
Copy Markdown
Collaborator

…l is closed while installing

  • It's possible to close the checkout modal while the installation is ongoing. This would've still triggered an unexpected refresh, even after navigating away.
  • After fixing that, I noticed that the buy box doesn't change to "Already owned". That's because closing the modal was emitting a "purchase failed" response to the iframe (and also Amplitude). I fixed that as well.

@chatgpt-codex-connector

This comment was marked as spam.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses incorrect purchase result reporting and unexpected reload behavior when the extension-store checkout modal is closed while a purchase/installation is still in progress, ensuring the UI/telemetry reflect the real purchase outcome and reloads happen only when appropriate.

Changes:

  • Introduces modal instance tracking (purchaseId) and adds a “reload required” notification instead of auto-reloading when the purchase outlives the modal.
  • Adjusts cancellation behavior so closing a submitted purchase modal doesn’t falsely report failure/cancellation.
  • Adds/extends unit tests to cover modal-closed-in-flight scenarios and updates snippets for the new notification text.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Resources/app/administration/src/module/sw-extension-store/store/extension-store-purchase-confirmation.store.ts Adds purchaseId, reload-required notification, and updated confirm/cancel behavior to avoid incorrect reload/result reporting.
src/Resources/app/administration/src/module/sw-extension-store/store/extension-store-purchase-confirmation.store.spec.ts Adds tests for in-flight modal closing, reload offer notification, and correct telemetry behavior.
src/Resources/app/administration/src/module/sw-extension-store/snippet/en.json Adds new snippet keys for “reload required” notification text.
src/Resources/app/administration/src/module/sw-extension-store/snippet/de.json Adds German translations for the new “reload required” notification text.
src/Resources/app/administration/src/module/sw-extension-store/service/extension-store-channel.service.spec.ts Updates router mock shape and adds a test ensuring purchase results are still published when the modal is closed during installation.
Suppressed comments (1)

src/Resources/app/administration/src/module/sw-extension-store/store/extension-store-purchase-confirmation.store.ts:232

  • purchaseId is documented as increasing with every opened modal, but openErrorModal() opens a modal without incrementing it. If a previous purchase finishes later, it can incorrectly treat the error modal as “its own” (same purchaseId + isOpen) and trigger reload/state updates unexpectedly.
            this.closeModal();

            this.onCancel = onCancel;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@qysp
Jannik (qysp) merged commit 0058e88 into epic/ECTO-2267 Aug 3, 2026
4 checks passed
@qysp
Jannik (qysp) deleted the bugfix/ECTO-2770-2 branch August 3, 2026 10:04
Kai Gossel (kaigossel) added a commit that referenced this pull request Aug 19, 2026
Kai Gossel (kaigossel) added a commit that referenced this pull request Aug 19, 2026
Kai Gossel (kaigossel) added a commit that referenced this pull request Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants