Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add logout redirect option to Customer Account Client #1871

Merged
merged 6 commits into from
Mar 21, 2024
Merged

add logout redirect option to Customer Account Client #1871

merged 6 commits into from
Mar 21, 2024

Conversation

michenly
Copy link
Contributor

@michenly michenly commented Mar 19, 2024 •
edited
Loading

WHY are these changes introduced?

Fixes #1809

WHAT is this pull request doing?

HOW to test your changes?

in templates/skeleton/app/routes/account_.logout.tsx

- return context.customerAccount.logout();
+ return context.customerAccount.logout({
    postLogoutRedirectUri: '/collections/all',
  });

also ensure this url is included in Customer Account application setup as logout url

  1. login & logout of customer account. See redirect occur after logout to /collections/all

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@shopify Shopify
Copy link
Contributor

shopify bot commented Mar 19, 2024 •
edited
Loading

Oxygen deployed a preview of your mc-logout-redirect branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
third-party-queries-caching ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM
custom-cart-method ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM
vite ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM
subscriptions ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM
skeleton ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM
optimistic-cart-ui ✅ Successful (Logs) Preview deployment Inspect deployment March 20, 202410:07 PM

Learn more about Hydrogen's GitHub integration.

@michenly michenly force-pushed the mc-logout-redirect branch 3 times, most recently from 8185cd7 to ffc7016 Compare March 19, 2024 16:18
michenly
michenly commented Mar 19, 2024
View reviewed changes
packages/hydrogen/src/customer/types.ts
@@ -144,7 +148,7 @@ export type CustomerAccountForDocs = {
* `en`, `fr`, `cs`, `da`, `de`, `es`, `fi`, `it`, `ja`, `ko`, `nb`, `nl`, `pl`, `pt-BR`, `pt-PT`,
* `sv`, `th`, `tr`, `vi`, `zh-CN`, `zh-TW`. If supplied any other language code, it will default to `en`.
* */
login: (options?: LoginOptions) => Promise<Response>;
login?: (options?: LoginOptions) => Promise<Response>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

login were made optional in #1682 for docs but reverted.

@michenly michenly self-assigned this Mar 19, 2024
@michenly michenly requested a review from a team March 19, 2024 21:31
@michenly michenly marked this pull request as ready for review March 19, 2024 21:31
@michenly
Copy link
Contributor Author

Making one last update to ensure all redirect links are within the app

michenly
michenly commented Mar 20, 2024
View reviewed changes
packages/hydrogen/src/customer/customer.ts
Comment on lines +97 to +101
const redirectUri = ensureLocalRedirectUrl({
requestUrl: request.url,
defaultUrl: DEFAULT_AUTH_URL,
redirectUrl: authUrl,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure authUrl is not cross site

michenly
michenly commented Mar 20, 2024
View reviewed changes
packages/hydrogen/src/utils/get-redirect-url.ts
* @param defaultUrl - The default URL to redirect to.
* @param redirectUrl - Relative or absolute URL of redirect. If the absolute URL is cross domain return undefined.
* */
export function ensureLocalRedirectUrl({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wizardlyhel hopefully this util make sense

@michenly michenly mentioned this pull request Mar 21, 2024
Closed
@michenly michenly merged commit ca1dcbb into main Mar 21, 2024
13 checks passed
@michenly michenly deleted the mc-logout-redirect branch March 21, 2024 21:27
@shopify-github-actions-access shopify-github-actions-access bot mentioned this pull request Mar 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wizardlyhel wizardlyhel wizardlyhel approved these changes

Assignees

@michenly michenly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add post logout redirect option in customerAccount.logout
2 participants
@michenly @wizardlyhel