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.

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

Allow support for multiple sites concurrently w/ commerce-sdk-react #911

Merged
merged 22 commits into from
Jan 26, 2023

Conversation

bendvc
Copy link
Collaborator

@bendvc bendvc commented Jan 19, 2023

image

Prefix your cookies with milk

Description

To be in alignment with plugin_slas this PR ensures that the storage keys we use during the authentication process are prefixed with the id of the ecomm back-end we are connecting to. This means that you can jump from one site to another without loosing your guest or registered session, but more importantly the cookie keys will be the same format as what plugin_slas expects, so there will be no issues navigating from PWA to SFRA experiences.

Changes

  • Make BaseStorage an abstract class instead of an interface to add prefixing functionality.
  • Remove clearing of storage as it's not in use anymore.
  • Add new storage class for server side storage instead of using Maps directly (solves typing issue).

How to Test-Drive This PR

  • In the test-commerce-sdk-react project execute the command npm start.
  • Goto the slas login helpers page.
  • Open your inspector and goto the application --> cookies tab and look at the cookies being created.
  • You should see the cc-nx-g cookie prefixed with the currently selected site.
  • Try changing the site, you should see an additional cookie created with the newly selected site's id prefix to it.
  • Now try logging in as a registered user, the guest cookie (<prefix>_cc-nx-g) will be replaces with the registered cooke (<prefix>_cc-nx)

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

@bendvc bendvc requested a review from a team as a code owner January 19, 2023 18:39
@echessman echessman requested a review from kevinxh January 20, 2023 17:56
@kevinxh
Copy link
Collaborator

kevinxh commented Jan 20, 2023

We probably want some sort of test that make sure the multi-site feature is working.

Particularly two test cases

  1. I switch site, and things still works
  2. I switch site, and switch back, i can use the old token

Any idea where these kind of tests belong to?

}
abstract set(key: string, value: string, options?: unknown): void
abstract get(key: string): string
abstract delete(key: string): void
Copy link
Contributor

Choose a reason for hiding this comment

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

We could potentially return boolean ("was deleted?") to fully conform with the Map interface. Not important if we don't, though.

packages/commerce-sdk-react/src/auth/storage.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.test.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.ts Outdated Show resolved Hide resolved
set(key: string, value: string, options?: unknown): void
get(key: string): string
delete(key: string): void
export type StorageTypes = 'cookie' | 'local'
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 We could add static type: StorageType to each implementation. That could potentially enable some fancy type magic. Not sure we have a use case for fancy type magic here, but maybe someone downstream will...

packages/commerce-sdk-react/src/auth/storage.test.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.test.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.test.ts Outdated Show resolved Hide resolved
packages/commerce-sdk-react/src/auth/storage.test.ts Outdated Show resolved Hide resolved
@@ -41,15 +41,13 @@ type AuthDataKeys =
type AuthDataMap = Record<
Copy link
Contributor

Choose a reason for hiding this comment

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

Not relevant to this PR, but I was curious if there was a way to use TokenResponse so that we don't need to manually list the keys. Turns out there is!

// Helpers
type RemoveNeverValues<T> = {
    // Key remapping, fancy stuff! https://devblogs.microsoft.com/typescript/announcing-typescript-4-1/#key-remapping-in-mapped-types
    [K in keyof T as T[K] extends never ? never: K]: T[K]
}
type StringIndexNever<T> = {
    [K in keyof T]: string extends K ? never : T[K]
}
type KnownKeys<T> = keyof RemoveNeverValues<StringIndexNever<T>>

// Implementation
type TokenResponseKeys = KnownKeys<ShopperLoginTypes.TokenResponse>
type AuthDataKeys =
    | Exclude<TokenResponseKeys, 'refresh_token'>
    | 'idp_access_token'
    | 'refresh_token_guest'
    | 'refresh_token_registered'
    | 'site_id'

Copy link
Collaborator

Choose a reason for hiding this comment

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

@wjhsf This is nice 👌, this either close to (if not exactly) what I was mentioning in the typescript chat we had with @kevinxh last week

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks promising to my TS n00b eyes. Maybe you can contribute this is a separate PR? @wjhsf

@bendvc bendvc added the wip Work in process label Jan 24, 2023
- Make `onClient` a util so I can mock it.
- Make MemoryStorage use global map by default to maintain server behaviour
- Add test for validating above behaviour
@bendvc bendvc added ready for review PR is ready to be reviewed and removed wip Work in process labels Jan 25, 2023
Comment on lines +249 to +251
// Set mock value back to expected.
// @ts-expect-error read-only property
utils.onClient = () => true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe jest has util functions to restore mocks, I think it's jest.restoreAllMocks() (or maybe jest.resetAllMocks()).

We could do that in afterEach to DRY it.

afterEach(() => {
  jest.restoreAllMocks()
})

packages/commerce-sdk-react/src/auth/storage.ts Outdated Show resolved Hide resolved
constructor(options?: MemoryStorageOptions) {
super(options)

this.map = options?.sharedContext ? globalMap : new Map()
Copy link
Contributor

Choose a reason for hiding this comment

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

Re: #911 (comment)

Our use case is to use a shared context. Is our use case common enough that it should be the default, or should we default to isolated contexts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I chose the default to be isolated maps, as I think it's the most natural. Is it weird that our only use of this is not the default.. maybe.. but I think that our use case of wanting global storage from lambda run to another is a little wweird.

@kevinxh
Copy link
Collaborator

kevinxh commented Jan 26, 2023

What do you think if we have a test case that looks something like below, to ensure the multisite feature works...

warning: pseudo code 😛

test('Multi-site support', () => {
  const {rerender} = render(<CommerceApiProvider siteId={'en'} />)
  
  // expect storage keys are pre-fixed with siteId EN, pseudocode
  expect(storage.keys).has('en-access_token')

  // re-render the provider with a different site id
  rerender(<CommerceApiProvider siteId={'fr'} />)

  // old access token still exists and value do not change
  expect(storage.keys).has('en-access_token')

  // new access token for FR
  expect(storage.keys).has('fr-access_token')
})

bendvc and others added 2 commits January 26, 2023 13:44
@bendvc bendvc merged commit bdb7309 into develop Jan 26, 2023
@bendvc bendvc deleted the namespace-auth-storage branch January 26, 2023 22:11
@bendvc bendvc restored the namespace-auth-storage branch February 8, 2023 19:22
alexvuong added a commit that referenced this pull request Feb 13, 2023
* New year, new look! (#876)

* New year, new look!

* Use JS to compute current year.

* Small bump to max deps allowed.

Needed to so that non-code changes will pass CI.
Hitting the limit can be addressed later.

* Update from 2.5.0 (#881)

* Starting release process for 2.5.0

* allow commerce react sdk to release

* bump version to 2.5.0 (#879)

* bump version to 2.5.0

* bump max packages

* Begin development on 2.6.0

* Set commerce-sdk-react package to private (#882)

* test-commerce: comment out not-implemented customer hooks (#877)

* Change padding (#899)

* [Snyk] Security upgrade eslint-import-resolver-webpack from 0.10.0 to 0.13.1 (#887)

* fix: packages/internal-lib-build/package.json & packages/internal-lib-build/package-lock.json to reduce vulnerabilities

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-DEBUG-3227433

* upgrade eslint-webpack-plugin-import, regen lock file

Co-authored-by: Alex Vuong <alex.vuong@salesforce.com>
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

* Cache SLAS callback using request processor (#884)

* cache callback in request processor

* fix import path

* cache callback for a year

* use native URLSearchParams

* revert use native URLSearchParams

* Rotate fingerprint ssh for deploy commerce sdk doc (#905)

* Feature/megamenu fixes (#875)

* [W-11996527] WIP - commit megamenu spike results / findings to feature branch

* resolve search error

* move default variables to constants file

* cleanup code

* add aria-live to menu categories

* lint

* implement aria live and busy

* fix failing tests

* update mock categories data, implement aria busy for listmenu

* update mock data and fix list-menu test

* lint

* resolve unmounted component error

* temporary testing server errors fix

* set spinner opacity to 0 and increase max package value

* lint

* remove mock data from app code, clean up

* allow padding of categories with loaded: true to bypass initial useEffect API call

* fix last failing test

* Update list-menu/index.jsx

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

* fix aria-busy implementation and cleanup

* add more test cases - when there is no root categories

* remove mit license comment

* add depndency array to useeffect

* globally mock getcategory api call

* WIP

* fix tests WIP

* remove all instances of setupmockserver in test files

* globally mock istokenvalid AND many other cleanup items

* refactor tree walking code

* add locale checks and time to localstorage

* WIP removing tree walking logic

* remove tree walking code, update setroot and extract stale time

* add catch to promise all

* lint

* namespace constants, remove unnecessary try catch...

* final cleanup

* fix lint

Co-authored-by: Brian Feister <bfeister@salesforce.com>
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
Co-authored-by: Kevin He <kevin.he@salesforce.com>

* Mega menu fixes (#910)

* Remove unnecessary catch clause.

* Update copyright.

* Don't return error when category is expected.

* Fix bug preventing cache invalidation.

* Set fetchTime when data is fetched, rather than every page load.

* Update comment.

* Convert promise chain to async/await for readability.

* Guard against missing category items.

* Move comment inside code block.

* Change useEffect back to promise chain, not async/await.

useEffect expects the callback to return nothing or a function;
returning a promise could break things.

* GitHub Actions (#854)

* Add test workflow

* Use actions/checkout v3

* Don't use CI image

* Add step install npm dependencies

* Add setup action

* Set DEVELOP and RELEASE env variables

* Add required shell property

* Update action.yml

* Update action.yml

* Adding test matrix

* cleanup

* Split setup windows and ubuntu

* Update test.yml

* Update test.yml

* Add Lighthouse and smoketest scripts

* Not using actions/cache for now

* Add generated matrix

* Add missing runs-on prop to generated matrix

* Add Setup Node step to generated matrix

* Add Setup machine to generated matrix

* Add cron schedule

* Add timeout-minutes 5

* Move timeout to generate project step

* Add Run tests on generated projects

* Use dynamic generated-project folder

* Run tests on test-project and retail-react-app-demo

* Add Push Bundle step

* Skip flaky test

* Disable fail-fast strategy

* Use env variables

* Re-arrange env

* Add step before push bundle

* cleanup

* cleanup

* Use temp test-env-3

* testing slack notifs

* testing

* add publish to npm step

* fix indent

* python-dev does not exist anymore

* use python2

* increase max packages

* test slack notifs

* add snyk cli and datadog step

* update mrt user credentials

* testing slack with pwa kit channel

* syntax

* fix conditionals

* test push bundle

* add push bundle step for generated

* syntax

* fix syntax error

* update slack payload

* run steps in container

* testing

* refactor

* syntax

* sudo container error

* testing

* update

* add pip

* use different docker

* no container

* container

* testing

* add user to container

* fix

* syntax add shell

* syntax errors

* remove container, use act

* syntax errors

* add snyk audit and other syntax stuff

* extract steps to own actions

* add inputs for actions

* add shell for steps in actions with run

* project cannot be generated in action file

* updated snyk token, uncommenting code

* Fix typo.

* Add missing appkey property.

* Use snake_case names for legibility.

* Restore missing clean check

* Fix skipped conversion to snake_case.

* Trim trailing whitespace.

* Extract conditionals to vars and clean up vars.

* Change env IS_TESTABLE_TEMPLATE to more clear IS_TEMPLATE_FROM_RETAIL_REACT_APP

* Fix YAML breaking conditional.

* Try explicitly checking value.

* Try explicitly checking true/false string values.

* Try string comparisons.

* Fix bad YAML.

* Replace " with '

* Get ready for the prime time!

* Fail fast!

* Update TODOs.

* Clean up npm version management.

* Add TODO to merge workflows.

* Update step names.

* End files with newline.

* Run on pull_request to support forked repos.

* Only run on push for develop/release.

We can assume all other branches will eventually have a PR.

* Only push to MRT when actually desired.

* Get that JavaScript nonsense outta here!

* Check DEVELOP in step conditional, not in action execution.

* Add some TODOs.

* Too many newlines!

Co-authored-by: yunakim714 <yunakim@salesforce.com>
Co-authored-by: yunakim714 <84923642+yunakim714@users.noreply.github.com>
Co-authored-by: Will Harney <wharney@salesforce.com>
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

* GitHub Actions fixes (#915)

* Update is not fork check

* Add single quote to cron expression

* Add cron docs

* Remove Circle CI config, fix IS_NOT_FORK (#921)

* Fix develop branch name (#923)

* Fix wrong proptypes (#924)

* fix wrong proptypes

* Update packages/template-retail-react-app/app/components/recommended-products/index.jsx

Co-authored-by: Vincent Marta <vmarta@salesforce.com>

Co-authored-by: Brian Feister <47546998+bfeister@users.noreply.github.com>
Co-authored-by: Vincent Marta <vmarta@salesforce.com>

* Update createOrder to send SLAS USID (#920)

* Update createOrder to send SLAS USID

* Modify some tests to include setting the header

Co-authored-by: echessman <37908171+echessman@users.noreply.github.com>

* Upgrade prettier to v2 (#926)

* Upgrade to prettier v2 for modern TypeScript support.

* Change trailing comma to prettier v1 default.

Minimizes changes during v2 upgrade.

* Apply prettier v2 formatting changes.

Yaaay touching all of the files!

* Set end of line to LF.

* Remove unnecessary map statement (#929)

* fix: packages/pwa-kit-runtime/package.json & packages/pwa-kit-runtime/package-lock.json to reduce vulnerabilities (#935)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-UAPARSERJS-3244450

Co-authored-by: snyk-bot <snyk-bot@snyk.io>

* Update `develop` with v2.6.0 (#939)

* Starting release process for 2.6.0

* 🚢 Release v2.6.0 (#937)

* Update changelog files

* Set `commerce-sdk-react-preview` as public

* Version 2.6.0

* Begin development on 2.7.0

* Clean changelog files

* Allow support for multiple sites concurrently w/ `commerce-sdk-react` (#911)

* Namespace storage keys using the current siteId.

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

* @W-11920099 Re-write 'npm push' in Typescript, warn if Node deprecated (#763)

* Rewrite ancient scripts in Typescript.
* Show warnings on push when using a deprecated Node version.
* Automatically select a `.mobify` credentials file based on `--cloud-origin`.

* Fix broken CI – confusing path to package.json in the 'dist' directory! (#946)

* Reliably look up project and pwa-kit-dev package.json in scripts

* fix: handle special characters in `boldString` util (#942)

+ Add a new util for escaping special regex characters
+ Apply new util to `boldString`
+ Add and update util tests

Co-authored-by: Brian Feister <47546998+bfeister@users.noreply.github.com>

* Replace isomorphic jest mocks with msw handlers (#944)

* replace most manual mocks

* more replacements

* more replacements in addresses test

* lint

* resolve flaky cart test

* remove some jest mocks

* replace all isomorphic mocks

* cleanup

* fix auth modal mocks

* remove timers from auth hooks test, fix password test

* remove timer from create account test

* add timeout

* cleanup

---------

Co-authored-by: Brian Feister <bfeister@salesforce.com>

* Remove the PersistentCache functionality (#949)

* Initial pass at PersistentCache

* Drop test coverage

* Change test

* Update customer baskets cache when there is a basket mutation (#945)

* update customer baskets cache on basket mutations

* Fix layout shift for mega menu (#952)

* remove custom styling

* remove theme in theme file

* spinners in drawer menu should be visible

* Serialize category data once only (#953)

* serialize data only once

* remove console log

---------

Co-authored-by: Brian Feister <47546998+bfeister@users.noreply.github.com>

* chore: update pwa-kit-dev eslint config (#950)

+ Bump `eslint-plugin-react` to latest version
+ Auto-detect React version

Co-authored-by: Adam Raya <adamraya@users.noreply.github.com>

* resolving testing/merge errors

* Add Shopper Experience hooks (#958)

* Initial commit

* Update license

* Update test project to use bjnl_dev

* Fix usePage hook and Refactor test page

* Add usePages hook

* Add usePages pdp and plp test cases

* Clean up

* Update Changelog & Restore `zzrf-001` config

---------

Co-authored-by: Ben Chypak <bchypak@salesforce.com>

* add changelog

* Apply changes from commerce react sdk in feature branch (#964)

* apply change from commerce react sdk in feature branch

* Remove wrong changlog

* keep the same as develop

* linting

* fix formatMessage

* bump sizes temporarily

* fix product list tests

* fix header tests

* fix add to cart modal

* skip tests and low test coverate temporarily

* linting

---------

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
Co-authored-by: Alex Vuong <52219283+alexvuong@users.noreply.github.com>
Co-authored-by: vcua-mobify <47404250+vcua-mobify@users.noreply.github.com>
Co-authored-by: Vincent Marta <vmarta@salesforce.com>
Co-authored-by: Pavel <65617653+GoodNightBuddy@users.noreply.github.com>
Co-authored-by: Snyk bot <snyk-bot@snyk.io>
Co-authored-by: Alex Vuong <alex.vuong@salesforce.com>
Co-authored-by: yunakim714 <84923642+yunakim714@users.noreply.github.com>
Co-authored-by: Brian Feister <bfeister@salesforce.com>
Co-authored-by: Adam Raya <adamraya@users.noreply.github.com>
Co-authored-by: yunakim714 <yunakim@salesforce.com>
Co-authored-by: Will Harney <wharney@salesforce.com>
Co-authored-by: Brian Feister <47546998+bfeister@users.noreply.github.com>
Co-authored-by: echessman <37908171+echessman@users.noreply.github.com>
Co-authored-by: CC ProdSec <65211003+cc-prodsec@users.noreply.github.com>
Co-authored-by: Ben Chypak <bchypak@mobify.com>
Co-authored-by: Oliver Brook <o.brook@salesforce.com>
Co-authored-by: Brad Adams <hi@breadadams.com>
Co-authored-by: Kieran Haberstock <80915722+kieran-sf@users.noreply.github.com>
Co-authored-by: Ben Chypak <bchypak@salesforce.com>
bfeister added a commit that referenced this pull request Feb 16, 2023
* develop: (52 commits)
  fix localstorage key by add siteId prefix (#988)
  [Shopper Experience] `ImageTile` component (#967)
  Skip Snyk CLI (#983)
  fix(retail-react-app): recent searches (#969)
  [Feature] Shopper Experience / Page Designer Components (#963)
  fix(template-retail-react-app): improved handling of blocked einstein requests (#970)
  remove getResetPasswordToken from not implemented list (#973)
  Apply changes from commerce react sdk in feature branch (#964)
  Add Shopper Experience hooks (#958)
  chore: update pwa-kit-dev eslint config (#950)
  Serialize category data once only (#953)
  Fix layout shift for mega menu (#952)
  Update customer baskets cache when there is a basket mutation (#945)
  Remove the PersistentCache functionality (#949)
  Replace isomorphic jest mocks with msw handlers (#944)
  fix: handle special characters in `boldString` util (#942)
  Fix broken CI – confusing path to package.json in the 'dist' directory! (#946)
  @W-11920099 Re-write 'npm push' in Typescript, warn if Node deprecated (#763)
  Allow support for multiple sites concurrently w/ `commerce-sdk-react` (#911)
  Update `develop` with v2.6.0 (#939)
  ...

# Conflicts:
#	lerna.json
#	package-lock.json
#	package.json
#	packages/commerce-sdk-react/CHANGELOG.md
#	packages/commerce-sdk-react/package-lock.json
#	packages/commerce-sdk-react/package.json
#	packages/internal-lib-build/package-lock.json
#	packages/internal-lib-build/package.json
#	packages/pwa-kit-create-app/package-lock.json
#	packages/pwa-kit-create-app/package.json
#	packages/pwa-kit-dev/CHANGELOG.md
#	packages/pwa-kit-dev/bin/pwa-kit-dev.js
#	packages/pwa-kit-dev/package-lock.json
#	packages/pwa-kit-dev/package.json
#	packages/pwa-kit-react-sdk/CHANGELOG.md
#	packages/pwa-kit-react-sdk/package-lock.json
#	packages/pwa-kit-react-sdk/package.json
#	packages/pwa-kit-react-sdk/src/ssr/browser/main.jsx
#	packages/pwa-kit-runtime/CHANGELOG.md
#	packages/pwa-kit-runtime/package-lock.json
#	packages/pwa-kit-runtime/package.json
#	packages/template-express-minimal/package-lock.json
#	packages/template-express-minimal/package.json
#	packages/template-retail-react-app/app/commerce-api/mock-data.js
#	packages/template-retail-react-app/package-lock.json
#	packages/template-retail-react-app/package.json
#	packages/template-typescript-minimal/package-lock.json
#	packages/template-typescript-minimal/package.json
#	packages/test-commerce-sdk-react/package-lock.json
#	packages/test-commerce-sdk-react/package.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants