-
Notifications
You must be signed in to change notification settings - Fork 3
Feat/101/login signup api #112
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
Conversation
Walkthroughμ΄ λ³κ²½ μ¬νμ Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the toolβs configuration or disable the tool if itβs a critical failure. π§ eslint
src/_apis/auth/auth-apis.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't determine the plugin "react-hooks" uniquely.
Please remove the "plugins" setting from either config or remove either plugin installation. If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
πstorybook: https://67206cc6ff9d7a05a3528ff8-qbyqdepanr.chromatic.com/ |
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.
Actionable comments posted: 3
π§Ή Outside diff range and nitpick comments (2)
src/_apis/auth/auth-apis.tsx (1)
5-16: νμμμ κ°μ μμλ‘ λΆλ¦¬νλ κ²μ΄ μ’μ΅λλ€νλμ½λ©λ νμμμ κ°(5000)μ μμλ‘ λΆλ¦¬νμ¬ μ¬μ¬μ©μ±κ³Ό μ μ§λ³΄μμ±μ ν₯μμν€λ κ²μ΄ μ’μ΅λλ€.
λ€μκ³Ό κ°μ΄ μμ νλ κ²μ μ μν©λλ€:
+const AUTH_REQUEST_TIMEOUT = 5000; export function signupUser(data: SignupRequest): Promise<{ data: SignupResponse }> { return fetchApi<{ data: SignupResponse; headers: Headers }>( '/auths/signup', { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(data), }, - 5000, + AUTH_REQUEST_TIMEOUT, true, ).then((response) => {src/utils/api.ts (1)
22-22: λ§€κ°λ³μ μ΄λ¦μ λ λͺ ννκ² κ°μ νλ©΄ μ’κ² μ΅λλ€.
isAuth보λ€λincludeHeadersλλreturnAuthHeadersμ κ°μ΄ λ§€κ°λ³μμ λͺ©μ μ λ λͺ ννκ² νννλ μ΄λ¦μ μ¬μ©νλ κ²μ΄ μ’κ² μ΅λλ€.- isAuth = false, + includeHeaders = false,
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
π Files selected for processing (2)
src/_apis/auth/auth-apis.tsx(1 hunks)src/utils/api.ts(2 hunks)
π Additional comments (2)
src/utils/api.ts (2)
Line range hint 6-9: νμ
μ£Όμ μΆκ°κ° μ μ ν©λλ€!
detail λ§€κ°λ³μμ λν νμ
μ£Όμμ΄ ν΄λμ€ μμ±κ³Ό μΌμΉνλ©°, νμ
μμ μ±μ ν₯μμν΅λλ€.
Line range hint 1-65: μΈμ¦ κ΄λ ¨ λ³κ²½μ¬νμ μν₯λλ₯Ό νμΈν΄μ£ΌμΈμ.
μ΄ λ³κ²½μ¬νμ΄ κΈ°μ‘΄ μΈμ¦ νλ¦μ λ―ΈμΉλ μν₯μ νμΈνκΈ° μν΄ λ€μ μ¬νλ€μ κ²μ¦ν΄μ£ΌμΈμ:
β Verification successful
μΈμ¦ κ΄λ ¨ λ³κ²½μ¬νμ΄ κΈ°μ‘΄ μΈμ¦ νλ¦μ μν₯μ λ―ΈμΉμ§ μμμ νμΈνμ΅λλ€.
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# μΈμ¦ κ΄λ ¨ API νΈμΆ ν¨ν΄ νμΈ
echo "Checking auth API call patterns..."
rg -l "fetchApi.*isAuth.*true" src/
# μλ¬ μ²λ¦¬ ν¨ν΄ νμΈ
echo "Checking error handling patterns..."
ast-grep --pattern 'catch (error) {
$$$
ApiError
$$$
}'
Length of output: 263
| return fetchApi<{ data: SignupResponse; headers: Headers }>( | ||
| '/auths/signup', | ||
| { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }).then((response) => { | ||
| 5000, | ||
| true, | ||
| ).then((response) => { |
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.
μλ¬ μ²λ¦¬ λ‘μ§μ΄ νμν©λλ€
λ‘κ·ΈμΈκ³Ό νμκ°μ ν¨μμμ λ€νΈμν¬ μ€λ₯λ μλ² μ€λ₯μ λν μ²λ¦¬κ° λλ½λμ΄ μμ΅λλ€. μ¬μ©μμκ² μ μ ν μλ¬ λ©μμ§λ₯Ό μ 곡νκΈ° μν΄ μλ¬ μ²λ¦¬λ₯Ό μΆκ°νλ κ²μ΄ μ’μ΅λλ€.
λ€μκ³Ό κ°μ μλ¬ μ²λ¦¬ λ‘μ§μ μΆκ°νλ κ²μ μ μν©λλ€:
export function signupUser(data: SignupRequest): Promise<{ data: SignupResponse }> {
return fetchApi<{ data: SignupResponse; headers: Headers }>(
'/auths/signup',
{
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(data),
},
5000,
true,
- ).then((response) => {
+ ).then((response) => {
const token = response.headers.get('Authorization');
+ if (!token) {
+ throw new Error('μΈμ¦ ν ν°μ λ°μ§ λͺ»νμ΅λλ€');
+ }
return { data: { token } };
+ }).catch((error) => {
+ if (error.status === 409) {
+ throw new Error('μ΄λ―Έ μ‘΄μ¬νλ μ¬μ©μμ
λλ€');
+ }
+ throw new Error('νμκ°μ
μ€ μ€λ₯κ° λ°μνμ΅λλ€');
});
}Also applies to: 23-34
π οΈ Refactor suggestion
μ€λ³΅ μ½λλ₯Ό μ νΈλ¦¬ν° ν¨μλ‘ λΆλ¦¬νλ κ²μ΄ μ’μ΅λλ€
signupUserμ loginUser ν¨μμ κ΅¬μ‘°κ° λ§€μ° μ μ¬ν©λλ€. μ½λ μ€λ³΅μ μ€μ΄κ³ μ μ§λ³΄μμ±μ λμ΄κΈ° μν΄ κ³΅ν΅ λ‘μ§μ λ³λμ μ νΈλ¦¬ν° ν¨μλ‘ λΆλ¦¬νλ κ²μ΄ μ’μ΅λλ€.
λ€μκ³Ό κ°μ΄ 리ν©ν λ§νλ κ²μ μ μν©λλ€:
+const handleAuthRequest = async <T>(
+ endpoint: string,
+ data: any
+): Promise<{ data: T }> => {
+ const response = await fetchApi<{ data: T; headers: Headers }>(
+ endpoint,
+ {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/json',
+ },
+ body: JSON.stringify(data),
+ },
+ 5000,
+ true
+ );
+
+ const token = response.headers.get('Authorization');
+ if (!token) {
+ throw new Error('μΈμ¦ ν ν°μ λ°μ§ λͺ»νμ΅λλ€');
+ }
+ return { data: { token } as T };
+};
export function signupUser(data: SignupRequest): Promise<{ data: SignupResponse }> {
- return fetchApi<{ data: SignupResponse; headers: Headers }>(
- '/auths/signup',
- {
- method: 'POST',
- headers: {
- 'Content-Type': 'application/json',
- },
- body: JSON.stringify(data),
- },
- 5000,
- true,
- ).then((response) => {
- const token = response.headers.get('Authorization');
- return { data: { token } };
- });
+ return handleAuthRequest<SignupResponse>('/auths/signup', data);
}
export function loginUser(data: LoginRequest): Promise<{ data: LoginResponse }> {
- return fetchApi<{ data: LoginResponse; headers: Headers }>(
- '/auths/login',
- {
- method: 'POST',
- headers: {
- 'Content-Type': 'application/json',
- },
- body: JSON.stringify(data),
- },
- 5000,
- true,
- ).then((response) => {
- const token = response.headers.get('Authorization');
- return { data: { token } };
- });
+ return handleAuthRequest<LoginResponse>('/auths/login', data);
}Also applies to: 23-34
| if (isAuth) return { data, headers: response.headers } as T; | ||
| return { data } as T; |
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.
λ°ν νμ μ²λ¦¬μ λν κ°μ μ΄ νμν©λλ€.
λ€μκ³Ό κ°μ λ¬Έμ μ λ€μ΄ μμ΅λλ€:
- μ‘°κ±΄λΆ λ°ν νμ μΌλ‘ μΈν΄ νμ μμ μ±μ΄ μ νλ μ μμ΅λλ€.
- μΈμ¦μ νμν νμ ν€λμ μ‘΄μ¬ μ¬λΆλ₯Ό νμΈνμ§ μμ΅λλ€.
λ€μκ³Ό κ°μ κ°μ μ μ μλ립λλ€:
interface AuthResponse<T> {
data: T;
headers: Headers;
}
interface DataResponse<T> {
data: T;
}
export async function fetchApi<T>(
url: string,
options: RequestInit = {},
timeout = 5000,
includeHeaders = false,
): Promise<AuthResponse<T> | DataResponse<T>> {
// ... existing code ...
if (includeHeaders) {
const requiredHeaders = ['authorization', 'content-type'];
const missingHeaders = requiredHeaders.filter(
header => !response.headers.has(header)
);
if (missingHeaders.length > 0) {
throw new ApiError(
500,
`Missing required headers: ${missingHeaders.join(', ')}`
);
}
return { data, headers: response.headers } as AuthResponse<T>;
}
return { data } as DataResponse<T>;
}
π Issue Ticket
Ticket
βοΈ Description
λ‘κ·ΈμΈ/νμκ°μ API μμ²μμλ§ return κ°μ ν€λκ° λ°μμ€λ κ²μΌλ‘ μμ
β Checklist
PR
Test
Summary by CodeRabbit
μ κ· κΈ°λ₯
λ²κ·Έ μμ