Improved: flow for app version support in case of embedded app - #145
ymaheshwari1 wants to merge 4 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c6c27d6de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const currentExpiration = embeddedAppStore?.getTokenExpiration || expirationTime.value; | ||
|
|
There was a problem hiding this comment.
When a merchant opens the standalone app on the same origin after an embedded session, the persisted embedded store still contains its token and expiration because only shopifyAppBridge is omitted from persistence. These lines therefore prefer those credentials even though commonUtil.isAppEmbedded() is false: an expired embedded token makes a valid cookie-backed standalone session appear unauthenticated, while a live one is paired with the store-first token/OMS helpers and can send requests using the previous embedded session. Select these values from the store only when the current page is actually embedded, or clear the embedded credentials when entering standalone mode.
Useful? React with 👍 / 👎.
patelanil
left a comment
patelanil
left a comment
There was a problem hiding this comment.
Log message format review
I reviewed the log/error messages added in this PR against our best practice: log-messages.md.
These added messages do not follow the [Entity] [Context] - [Action/Outcome/Issue] pattern:
App version "${pending}" is not served by this host; falling back to the root build for this session.
Suggest:App Version [Version: ${pending}] - Not served by this host; falling back to the root build for this session
Dynamic IDs belong in the [Context] block and the action text should stay constant, so messages group and count cleanly in Grafana/Loki during log analysis. Please align these before merge.
2 participants
Related Issues
Short Description and Why It's Useful
Screenshots of Visual Changes before/after (If There Are Any)
Is the changes contains any breaking change?
If there are any breaking change include those in the release notes file
Contribution and Currently Important Rules Acceptance