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

Refactor/75: 코드래빗 리뷰 1차 반영 #76

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Conversation

cindycho0423
Copy link
Collaborator

@cindycho0423 cindycho0423 commented Dec 10, 2024

#️⃣ 이슈

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.

  • api 함수 공통로직 분리
  • 타입 분리
  • 자잘한 코드개선

📸 스크린샷

✅ 체크 리스트

  • 적절한 Title 작성
  • 적절한 Label 지정
  • Assignee 및 Reviewer 지정
  • 로컬 작동 확인
  • Merge 되는 브랜치 확인

👩‍💻 공유 포인트 및 논의 사항

Summary by CodeRabbit

  • 새로운 기능

    • API 요청을 위한 비동기 함수 makeApiRequest 추가.
    • 거래 관련 API 함수들이 makeApiRequest를 사용하도록 리팩토링됨.
    • 거래 테이블 및 버튼의 구조가 개선됨.
    • Tabs 컴포넌트에 접근성을 위한 areaLabel 속성 추가.
  • 버그 수정

    • 입력 값 처리 로직에서 0 값을 허용하도록 수정됨.
    • 거래 양식의 유효성 검사 로직이 변경되어 입찰 값이 0보다 커야 함.
  • 문서화

    • 여러 새로운 타입 정의가 추가되어 거래 관련 데이터의 구조가 명확해짐.

@cindycho0423 cindycho0423 linked an issue Dec 10, 2024 that may be closed by this pull request
2 tasks
@cindycho0423 cindycho0423 self-assigned this Dec 10, 2024
@cindycho0423 cindycho0423 requested a review from Han-wo December 10, 2024 11:36
Copy link

coderabbitai bot commented Dec 10, 2024

Walkthrough

이 풀 리퀘스트는 API 요청을 처리하기 위한 새로운 비동기 함수 makeApiRequest를 도입하고, 기존 거래 관련 API 요청 함수를 리팩토링하여 이 함수를 활용하도록 변경합니다. 이로 인해 거래 관련 함수는 보다 구체적인 응답 타입을 반환하며, 오류 처리는 중앙 집중식으로 관리됩니다. 또한, 여러 컴포넌트의 구조와 스타일이 개선되어 접근성과 사용자 경험이 향상되었습니다.

Changes

파일 경로 변경 요약
src/api/make-api-request.ts 새로운 비동기 함수 makeApiRequest 추가, API 요청을 위한 HTTP 메소드 및 엔드포인트 처리.
src/api/transaction/index.ts 거래 관련 API 요청 함수 리팩토링, 응답 타입을 구체화하고 makeApiRequest 사용.
src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-body.tsx EditTableBodyPropsdata 타입을 OrderHistory에서 LimitPriceOrderHistory로 변경.
src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-header.tsx <tr> 요소를 <thead>로 감싸 HTML 구조 개선.
src/app/search/[id]/_components/transaction-form/edit-cancel/index.tsx <tbody> 추가 및 버튼 스타일 변경.
src/app/search/[id]/_components/transaction-form/index.tsx Tabs 컴포넌트에 areaLabel prop 추가.
src/app/search/[id]/_components/transaction-form/trade/buy-form-buttons.tsx 버튼 스타일 변경 및 조건부 클래스 추가.
src/app/search/[id]/_components/transaction-form/trade/index.tsx OrderHistoryLimitPriceOrderHistory로 변경, 새로운 타입 추가.
src/app/search/[id]/_components/transaction-form/trade/order-field.tsx handleInputChange에서 0 이하 체크 제거.
src/app/search/[id]/_components/transaction-form/transaction-table.tsx 버튼 스타일 변경 및 조건부 클래스 추가.
src/app/search/[id]/types/index.ts 여러 거래 관련 인터페이스 삭제.
src/components/common/button/index.tsx ButtonPropsvariant prop 타입 변경.
src/components/common/tabs/index.tsx Tabs 컴포넌트에 areaLabel prop 추가.
src/types/transaction/index.ts 여러 새로운 거래 관련 인터페이스 추가.
src/validation/schema/transaction-form/index.tsx bidding 필드의 유효성 검사 로직 변경.

Assessment against linked issues

Objective Addressed Explanation
코드래빗 리뷰 반영하기 (#75)

Possibly related PRs

Suggested labels

✨ FEAT, 🛠️ FIX

Suggested reviewers

  • Han-wo

🐇 변화의 바람이 불어와
API 요청이 새롭게 태어나
거래의 길이 더 명확해져
코드의 흐름이 더욱 부드러워져
모두 함께 기뻐하리라! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (4)
src/app/search/[id]/_components/transaction-form/trade/buy-form-buttons.tsx (1)

32-37: 사용자 정의 버튼 스타일 적용 시 접근성 고려 제안

variant"custom"으로 변경하고 orderType에 따라 버튼의 배경색을 동적으로 적용하여 UI의 일관성을 높였습니다. 다만, 설정한 배경색이 접근성 표준(예: WCAG 대비 비율)을 충족하는지 확인하여 사용자 경험을 향상시키는 것을 권장합니다.

src/types/transaction/index.ts (1)

6-13: 중복되는 속성들의 재사용을 위한 기본 인터페이스 추출을 제안드립니다.

LimitPriceOrderHistoryTradeHistory는 많은 공통 속성을 공유하고 있습니다. 다음과 같이 기본 인터페이스를 만들어 재사용하는 것을 고려해보세요:

interface BaseTradeHistory {
  buyPrice: number;
  remainCount: number;
  stockCount: number;
  stockName: string;
}

interface LimitPriceOrderHistory extends BaseTradeHistory {
  OrderId: number;
  type: string;
}

interface TradeHistory extends BaseTradeHistory {
  id: number;
  buyOrder: string;
}

Also applies to: 15-22

src/app/search/[id]/_components/transaction-form/transaction-table.tsx (1)

18-18: Props 타입에 대한 JSDoc 문서화가 필요합니다.

TransactionTableProps 인터페이스에 대한 문서화를 추가하면 컴포넌트의 사용성이 향상될 것 같습니다.

다음과 같이 문서를 추가해보세요:

+/**
+ * @interface TransactionTableProps
+ * @property {('red' | 'green' | 'blue')} [color='red'] - 테이블 헤더와 버튼의 색상
+ * @property {boolean} [isSubmit=true] - 제출 버튼 표시 여부
+ * @property {Object} submittedData - 거래 데이터
+ * @property {Function} [onClickGoBack] - 뒤로가기 버튼 클릭 핸들러
+ * @property {Function} [onClickConfirm] - 확인 버튼 클릭 핸들러
+ */
src/app/search/[id]/_components/transaction-form/edit-cancel/index.tsx (1)

144-146: 정렬 로직 최적화가 필요합니다

매 렌더링마다 정렬이 수행되고 있습니다. 성능 최적화를 위해 useMemo를 사용하는 것이 좋습니다.

다음과 같이 수정하는 것을 제안합니다:

- [...limitOrderData]
-   .sort((a, b) => b.OrderId - a.OrderId)
-   .map((data) => (
+ {useMemo(() => 
+   [...limitOrderData]
+     .sort((a, b) => b.OrderId - a.OrderId)
+     .map((data) => (
+   )), [limitOrderData])}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e55fb9f and e723f57.

📒 Files selected for processing (15)
  • src/api/make-api-request.ts (1 hunks)
  • src/api/transaction/index.ts (2 hunks)
  • src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-body.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-header.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/edit-cancel/index.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/index.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/trade/buy-form-buttons.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/trade/index.tsx (2 hunks)
  • src/app/search/[id]/_components/transaction-form/trade/order-field.tsx (1 hunks)
  • src/app/search/[id]/_components/transaction-form/transaction-table.tsx (2 hunks)
  • src/app/search/[id]/types/index.ts (0 hunks)
  • src/components/common/button/index.tsx (2 hunks)
  • src/components/common/tabs/index.tsx (2 hunks)
  • src/types/transaction/index.ts (1 hunks)
  • src/validation/schema/transaction-form/index.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • src/app/search/[id]/types/index.ts
✅ Files skipped from review due to trivial changes (1)
  • src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-header.tsx
🔇 Additional comments (13)
src/api/transaction/index.ts (4)

14-14: API 요청 로직의 중앙 집중화로 코드 간결화 및 유지보수성 향상

makeApiRequest 함수를 도입하여 API 요청 로직을 중앙에서 관리함으로써 각 함수의 코드가 간결해졌습니다. 이는 유지보수성과 가독성을 높이는 좋은 접근입니다.


20-21: 반환 타입 구체화로 타입 안정성 향상

기존에 Promise<string>을 반환하던 것을 구체적인 응답 타입으로 변경하여 타입 안정성이 향상되었습니다. 이는 좋은 개선입니다.

Also applies to: 28-29, 36-37, 44-45


10-10: 사용되지 않거나 제거된 타입의 재사용 여부 확인 필요

TradeHistory 타입이 다른 곳에서 제거되었거나 변경되었을 수 있습니다. 그러나 이 파일에서는 TradeHistory를 반환 타입으로 사용하고 있습니다. 해당 타입이 유효한지 확인이 필요합니다.

다음 스크립트를 실행하여 TradeHistory 타입의 정의 및 사용 위치를 확인하세요:

Also applies to: 97-99

✅ Verification successful

Based on the results, I can generate the final response as the verification is complete. The TradeHistory type is properly defined in src/types/transaction/index.ts and is actively used in the codebase:

TradeHistory 타입이 정상적으로 정의되어 있고 올바르게 사용되고 있습니다

  • src/types/transaction/index.ts에 인터페이스가 명확하게 정의되어 있습니다
  • getTradeHistory API 함수에서 반환 타입으로 사용되고 있습니다
  • 트랜잭션 히스토리 컴포넌트(history.tsx)에서 실제로 해당 API를 호출하여 사용하고 있습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: TradeHistory 타입의 정의 및 사용 위치를 파악합니다.

# TradeHistory 타입 정의 검색
rg -A 5 'interface TradeHistory' --type ts

# TradeHistory 타입 사용 위치 검색
rg -A 2 'TradeHistory' --type ts

Length of output: 1734


52-55: 함수 반환 타입 변경에 따른 코드베이스 영향 확인 필요

getHistorygetTrade 함수의 반환 타입이 변경되었습니다. 이로 인해 해당 함수를 호출하는 부분에서 타입 호환성 문제가 발생할 수 있으므로, 모든 호출부가 업데이트되었는지 확인해야 합니다.

다음 스크립트를 실행하여 해당 함수의 호출부를 확인하세요:

Also applies to: 62-65

src/validation/schema/transaction-form/index.tsx (1)

16-16: 호가의 유효성 검사 강화

bidding 필드의 유효성 검사가 .min(0)에서 .gt(0)로 변경되어, 호가가 0보다 커야 함을 정확히 검증합니다. 올바른 수정입니다.

src/api/make-api-request.ts (1)

1-40: API 요청 로직의 중앙 집중화를 통한 재사용성 향상

makeApiRequest 함수를 도입하여 API 요청 로직을 중앙에서 관리하고 재사용성을 높였습니다. 이는 코드의 일관성과 유지보수성에 큰 도움이 됩니다.

src/app/search/[id]/_components/transaction-form/edit-cancel/edit-table-body.tsx (1)

2-2: 타입 변경이 적절하게 이루어졌습니다.

OrderHistory에서 더 구체적인 LimitPriceOrderHistory 타입으로의 변경은 타입 안정성을 향상시킵니다.

Also applies to: 7-7

src/app/search/[id]/_components/transaction-form/trade/order-field.tsx (1)

66-69: ⚠️ Potential issue

numValue가 0인 경우에 대한 유효성 검사가 필요합니다.

0 값에 대한 검사가 제거되었습니다. 이는 주문 수량이나 입찰가가 0인 경우를 허용할 수 있어 비즈니스 로직에 문제가 될 수 있습니다.

다음 스크립트를 실행하여 관련 유효성 검사 스키마를 확인하겠습니다:

src/app/search/[id]/_components/transaction-form/index.tsx (1)

51-51: 접근성 개선이 잘 이루어졌습니다!

Tabs 컴포넌트에 areaLabel을 추가하여 스크린 리더 사용자를 위한 접근성이 향상되었습니다.

src/app/search/[id]/_components/transaction-form/transaction-table.tsx (1)

103-108: 버튼 스타일링이 개선되었습니다!

색상별 조건부 스타일링이 잘 구현되었고, hover 상태도 적절히 처리되었습니다.

src/app/search/[id]/_components/transaction-form/edit-cancel/index.tsx (2)

142-168: 테이블 구조가 개선되었습니다

tbody 요소를 추가하여 테이블의 시맨틱 구조가 향상되었습니다.


174-184: 버튼 스타일링이 일관되게 적용되었습니다

커스텀 variant를 사용하여 버튼 스타일링이 명확하게 구현되었습니다.

src/app/search/[id]/_components/transaction-form/trade/index.tsx (1)

18-21: 타입 정의가 개선되었습니다

LimitPriceOrderHistoryModifyTradeFormData 타입을 사용하여 타입 안정성이 향상되었습니다.

Also applies to: 38-39

@@ -3,7 +3,7 @@ import { ComponentProps, ReactNode } from "react";
import cn from "@/utils/cn";

interface ButtonProps extends ComponentProps<"button"> {
variant?: "primary" | "secondary" | "outline" | "outline-gray" | "red";
variant?: "primary" | "secondary" | "outline" | "outline-gray" | "custom";
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

커스텀 버튼 스타일 정의가 불완전합니다.

custom 버튼 variant에 대한 스타일링에서 다음 사항을 확인해주세요:

  • 배경색이 정의되어 있지 않습니다
  • hover 상태에 대한 스타일이 누락되었습니다

다음과 같이 스타일을 보완하는 것을 제안드립니다:

-          "text-white !rounded-4 px-24 py-9":
+          "bg-[#FF0000] text-white !rounded-4 px-24 py-9 hover:bg-[#DD0000]":
             variant === "custom" && !isDisabled,

Also applies to: 34-35

Comment on lines +73 to +75
export interface ModifyTradeFormData extends TradeAtLimitPriceFormDataType {
orderId: string | number | undefined;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

ModifyTradeFormData의 orderId 타입을 더 명확하게 정의해주세요.

orderIdstring | number | undefined 타입을 가지는 것은 타입 안정성을 저해할 수 있습니다. 가능하다면 단일 타입으로 통일하는 것이 좋습니다.

export interface ModifyTradeFormData extends TradeAtLimitPriceFormDataType {
  orderId: string; // 또는 number로 통일
}

Comment on lines +56 to +62
export interface TradeAtMarketPriceFormDataType {
token: string | null;
data: {
stockName: string;
quantity: number;
};
}
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

token 타입에서 null 제거가 필요합니다.

makeApiRequest 함수의 구현을 보면 토큰이 없을 경우(!token) "로그인이 필요합니다" 에러를 발생시킵니다. 따라서 TradeAtMarketPriceFormDataTypeTradeAtLimitPriceFormDataType 인터페이스에서 tokennull을 허용하는 것은 실제 구현과 일치하지 않습니다. 아래 위치의 타입 정의를 수정해야 합니다:

  • src/types/transaction/index.tsTradeAtMarketPriceFormDataType 인터페이스
  • src/types/transaction/index.tsTradeAtLimitPriceFormDataType 인터페이스
🔗 Analysis chain

토큰 타입에 대한 재검토가 필요합니다.

TradeAtMarketPriceFormDataTypeTradeAtLimitPriceFormDataType에서 tokennull을 허용하는 것이 의도된 것인지 확인해주세요. API 요청 시 토큰이 필수라면 null을 제거하는 것이 안전합니다.

Also applies to: 64-71


Let me check how the token is actually used in API calls and form submissions.


Let me check how the makeApiRequest function handles the token to understand if null is actually allowed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# API 호출에서 토큰 사용 패턴 확인
rg -A 2 "token: " src/api

Length of output: 1374


Script:

#!/bin/bash
# Check token usage in API calls and form submissions
rg -A 5 "TradeAtMarketPriceFormDataType|TradeAtLimitPriceFormDataType" src/

Length of output: 2858


Script:

#!/bin/bash
# Check makeApiRequest implementation
cat src/api/make-api-request.ts

Length of output: 977

@@ -31,9 +31,11 @@ const useTabsContext = () => {
function Tabs({
children,
defaultValue,
areaLabel,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

ARIA 레이블 속성명이 잘못되었습니다

area-label 속성은 올바른 HTML 접근성 속성이 아닙니다. 대신 aria-label을 사용해야 합니다.

다음과 같이 수정해주세요:

- <TabsContext.Provider value={contextValue} area-label={areaLabel}>
+ <TabsContext.Provider value={contextValue} aria-label={areaLabel}>

Also applies to: 38-38, 48-48

@cindycho0423 cindycho0423 added the ♻️REFACTOR Refactor code label Dec 10, 2024
@cindycho0423 cindycho0423 merged commit 3adc1f1 into develop Dec 10, 2024
4 of 5 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 27, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♻️REFACTOR Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: 코드래빗 리뷰 반영하기
1 participant