oko-377 #122
oko-377 #122
Conversation
blacktoast
force-pushed
the
retto/OKO-377
branch
from
39aaa16 to
d4c14c6
Compare
blacktoast
requested review from
chihunmanse and
eldenpark
and removed request for
chihunmanse
|
@blacktoast Is this review ready? |
|
Yes. |
blacktoast
force-pushed
the
retto/OKO-377
branch
from
c01ac62 to
537347f
Compare
| VALUES ( | ||
| $1, $2, $3, | ||
| $4, $5 | ||
| $4, $5, COALESCE($6, 'system') |
There was a problem hiding this comment.
Can we decide the "theme" value in the TS scope?
There was a problem hiding this comment.
In the code below, system is assigned when the value is null in the TS scope.
I just added it just in case, Should it be removed?
try {
const values = [
customer.customer_id,
customer.label,
customer.status,
customer.url?.length ? customer.url : null,
customer.logo_url?.length ? customer.logo_url : null,
customer.theme ?? "system",
];
const res = await db.query<Customer>(query, values);
There was a problem hiding this comment.
By the schema definition, PG will automatically insert "system" if nothing is given. Plus, even if there is no "default" value set up in the database side, I'd rather make the system explicitly "fail" if an invalid value (such as undefined) is given.
So for now, I think we can replace w/ just $6
| if (updates.theme !== undefined) { | ||
| updateFields.push(`theme = $${paramIndex}`); | ||
| values.push(updates.theme); | ||
| paramIndex++; |
There was a problem hiding this comment.
Please try not to use the unary operator. Instead use "+= 1"
| import { useAppState } from "@oko-wallet-attached/store/app"; | ||
|
|
||
| export function useSetThemeInCallback(providerType: OAuthProvider) { | ||
| const { setTheme, getTheme } = useAppState(); |
There was a problem hiding this comment.
Where is "setTheme" used?
- Since the callback is executed after the attached function is already running on the host, there is no need to call setTheme here.
|
@blacktoast Just so we know, I'm quite against the idea of having "attached-api", a separate package. This is a functionality related to "user", so later we may merge this "attached-api" |
Pull Request
CONTRIBUTING.mdand followed the guidelines.Summary
Themewhen editing customer information in the CT Dashboard@oko-wallet-attached-api/to enable theme loading via hostOrigin in attached.determineThemeto retrieve the theme from the server via the API and apply the changes.Links (Issue References, etc, if there's any)