Feat/cloud flows - #432
Conversation
This comment has been minimized.
This comment has been minimized.
tsvetelinpetrov
added
doc not required
There was a problem hiding this comment.
logo looks too small for retina. Is it feasible to use svg or add @2x version? if not quick to do then just ignore
| const stripped = cleanShellOutput(response || ''); | ||
| const [, match] = stripped.match(regex) ?? []; |
There was a problem hiding this comment.
is this safe for all other devices?
| * SPDX-License-Identifier: LicenseRef-Nordic-4-Clause | ||
| */ | ||
|
|
||
| // TODO: Replace this with a library that can handle ANSI removal. |
There was a problem hiding this comment.
if we can skip adding new dependency, better not to add :) or this regex is not complete?
| // Real: | ||
| const API_BASE = 'https://api.memfault.com/api/v0'; | ||
| const MYNORDIC_BASE = 'https://api.memfault.com/mynordic'; | ||
| // Mock: | ||
| // const API_BASE = 'http://127.0.0.1:8000'; | ||
| // const MYNORDIC_BASE = 'http://127.0.0.1:8000/mynordic'; | ||
|
|
||
| const POLL_INTERVAL_MS = 5000; |
There was a problem hiding this comment.
Using .env file or urls.ts / config.ts in the root is preferable. Otherwise it is hard to find configs / urls and manage them later.
There was a problem hiding this comment.
- I'd move interfaces to the
types.tsto improve future file readability & AI parsing - Guess too late to rewrite, but why not using RTK Query? I believe it should be already installed. Simplifies REST API manipulation a lot.
| const time = dateHeader ? Date.parse(dateHeader) : NaN; | ||
| if (Number.isNaN(time)) { | ||
| throw new Error( | ||
| 'Missing/unreadable server Date header (CORS: exposed?)', |
There was a problem hiding this comment.
Will this error appear in log or on the UI too? If on the UI, we need to provide more info. Maybe for the log and future debug we should add raw time value anyway.
| } | ||
| }; | ||
|
|
||
| interface TokenResponse { |
There was a problem hiding this comment.
please keep interfaces together :)
| projectSlug: string, | ||
| params: RegisterDeviceParams, | ||
| ): Promise<void> => { | ||
| const body: { |
There was a problem hiding this comment.
extract type / interface
| if ( | ||
| accessToken && | ||
| accessTokenExpiresAt && | ||
| Date.now() < accessTokenExpiresAt - TOKEN_EXPIRY_BUFFER_MS | ||
| ) { | ||
| return accessToken; | ||
| } |
There was a problem hiding this comment.
That is quite strange hack that is usually not needed. Easy to break by local device date change. Any chance we can remove this hack and manage tokens as usual?
|
|
||
| const TOKEN_EXPIRY_BUFFER_MS = 60_000; | ||
|
|
||
| export const getValidAccessToken = |
There was a problem hiding this comment.
All this function could be an API middleware..
| dispatch( | ||
| setAccessToken({ | ||
| accessToken: fresh, | ||
| expiresAt: Date.now() + expiresIn * 1000, |
There was a problem hiding this comment.
you never know. this isn't reliable. Cloud changes config and this will not work
| // 1. provisioning (/me) — precondition | ||
| await provisionMyNordicAccount(idTokenRes.data); | ||
|
|
||
| // 2. access token (/token) | ||
| const accessToken = await dispatch(getValidAccessToken()); | ||
|
|
||
| // 3. orgs/projects from /api/v0 | ||
| const organizations = await fetchOrganizations(accessToken); |
There was a problem hiding this comment.
| // 1. provisioning (/me) — precondition | |
| await provisionMyNordicAccount(idTokenRes.data); | |
| // 2. access token (/token) | |
| const accessToken = await dispatch(getValidAccessToken()); | |
| // 3. orgs/projects from /api/v0 | |
| const organizations = await fetchOrganizations(accessToken); | |
| await provisionMyNordicAccount(idTokenRes.data); | |
| const accessToken = await dispatch(getValidAccessToken()); | |
| const organizations = await fetchOrganizations(accessToken); |
| selectedOrgSlug: firstOrg.slug, | ||
| selectedProjectSlug: projects[0]?.slug, |
There was a problem hiding this comment.
if this sets values for the dropdown.. I do not think this should be stored in redux store at all.. Dropdown can decide what is default value by itself
2 participants
Comment to be added.
Uses those PRs: nordicsemi/pc-nrfconnect-shared#1142, nordicsemi/pc-nrfconnect-launcher#1313
Notes: