-
Notifications
You must be signed in to change notification settings - Fork 43
[HUM-164]: feat: add new http api client #3276
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
[HUM-164]: feat: add new http api client #3276
Conversation
|
@mpblocky is attempting to deploy a commit to the HUMAN Protocol Team on Vercel. A member of the Team first needs to authorize it. |
a0ea793 to
0926d51
Compare
|
@dnechay This one is ready. I know it's huge but I tried to commit every piece separately for easier review. LMK if You need something! |
|
@dnechay small fixes for my jobs needed, will let You know |
c9b54b4 to
01acc6f
Compare
|
@dnechay ready |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dnechay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP clients & auth handling lgtm as for now
Some comments below
- You don't have to create an instance of
HttpApiClientorAuthorizedHttpApiClientin every specific service, they all have the same base api url, so you can create these instances once and simply use them later w/o wrapping in any classes, e.g.
import { humanAppApiClient } from '@/api';
...
const result = await humanAppApiClient.post<SignatureData>(
apiPaths.web3.prepareSignature,
{
body: { ...data },
successSchema: prepareSignatureSuccessSchema,
}
);
return result;
- Deployment fails due to build errors, please check them
packages/apps/human-app/frontend/src/shared/services/signature.service.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/worker/hooks/use-get-oracles.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/signup/worker/hooks/use-sign-up-mutation.ts
Outdated
Show resolved
Hide resolved
.../human-app/frontend/src/modules/worker/email-verification/hooks/resend-email-verification.ts
Outdated
Show resolved
Hide resolved
...pps/human-app/frontend/src/modules/signup/operator/views/edit-existing-keys-success.page.tsx
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/signup/operator/hooks/use-web3-signup.ts
Outdated
Show resolved
Hide resolved
6990ba3 to
8c7cc37
Compare
|
@dnechay everything addressed |
dnechay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better!
Two comments from previous review are still actual: №1, №2 and some minor items to cleanup
- "Reset Password" functionality is not working due to absence of
h_captcha_tokenin payload - After registering address for worker, tokens are refreshed, but
Registerbutton is still there. Seems like some extra step need for updating user state when doingauthService.refreshAccessToken
packages/apps/human-app/frontend/src/modules/worker/profile/services/profile.service.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/operator/profile/services/profile.service.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/worker/hooks/use-get-oracles.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/signup/operator/hooks/use-web3-signup.ts
Outdated
Show resolved
Hide resolved
.../human-app/frontend/src/modules/worker/email-verification/hooks/resend-email-verification.ts
Outdated
Show resolved
Hide resolved
packages/apps/human-app/frontend/src/modules/homepage/services/homepage.service.ts
Outdated
Show resolved
Hide resolved
5d56be3 to
89c02d0
Compare
|
@dnechay fixed! |
dnechay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- Please resolve merge conflicts
- On example of
OperatorProfileService.enableOperator, could you please make this and similar methods where we expect "fire & forget" to look like
async enableOperator(signature: string) {
try {
await authorizedHumanAppApiClient.post(
apiPaths.enableOperator,
{
body: { signature },
}
);
} catch (error) {
...
}
so it's clear that it's not expected to handle any return value from it
- added new http api clients: authorized, unauthorized, auth service - implemented usage of it in refresh and sign in
2517a38 to
7505ece
Compare
Issue tracking
HUM-164
Context behind the change
How has this been tested?
Release plan
normal
Potential risks; What to monitor; Rollback plan
Monitor incorrect requests in case there was something that was omitted during testing