-
Notifications
You must be signed in to change notification settings - Fork 2
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
169 결재 글 열람결재기능 포함 #172
base: main
Are you sure you want to change the base?
The head ref may contain hidden characters: "169-\uACB0\uC7AC-\uAE00-\uC5F4\uB78C\uACB0\uC7AC\uAE30\uB2A5-\uD3EC\uD568"
169 결재 글 열람결재기능 포함 #172
Conversation
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.
결재 게시글 관련 코드 작업하느라 수고하셨습니다. 많은 파일 작업하시느라 미처 발견 못하셨거나 조금 더 보완할 수 있는 부분들 코멘트 남겨두었습니다. 검토 및 반영해주시고, 필요한 경우 편하게 상의 나누시죠!
src/api/registerComment.ts
Outdated
} else if (pathname.includes("/tasks")) { | ||
apiURL = `${BASE_URL}/projects/${projectId}/approvals/${taskId}/comments`; | ||
} else if (pathname.includes("/approvals")) { | ||
apiURL = `${BASE_URL}/projects/${projectId}/approvals/${approvalId}/comments`; |
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.
${BASE_URL} 부분은 있어도 상관 없지만 일관된 코드 작성을 위해 지워주시면 좋을 것 같습니다. 참고 부탁드립니다.
(위에 BASE_URL 변수 선언문도 같은 맥락입니다.)
- src > api > axiosInstance.ts 에서 baseURL 을 지정해줘서 API 요청 보낼 때마다 기본 URL 로 자동 설정됩니다.
src/api/registerComment.ts
Outdated
} else if (pathname.includes("/tasks")) { | ||
apiURL = `${BASE_URL}/projects/${projectId}/approvals/${taskId}/comments`; | ||
} else if (pathname.includes("/approvals")) { | ||
apiURL = `${BASE_URL}/projects/${projectId}/approvals/${approvalId}/comments`; |
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.
변수명 apiPath 또는 apiPathParameter 로 변경하면 어떨까요? API 호출 경로를 뜻하는 점에서 변경 고려해보시면 좋을 것 같습니다.
- URL 도 뜻하신 부분 바로 이해하긴 했습니다만, 주소창에 입력하는 도메인 주소명을 뜻해서요
responseData = await registerComment( | ||
Number(projectId), | ||
requestData, | ||
undefined, // questionId는 undefined로 전달 | ||
Number(taskId), | ||
Number(approvalId), | ||
parentId ? Number(parentId) : undefined, | ||
); | ||
} |
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.
return
문 내부commentText
코드 보완사항 제안
- 사유: 현재는 빈 댓글을 입력해도 API 요청이 실행됨
- 보완사항: commentText 가 비어있는 경우 버튼 비활성화, 또는 유효성 검사 추가
- 코드:
<Button
mt={2}
colorScheme="blue"
onClick={handleSave}
isDisabled={!commentText.trim()} // 댓글이 비어있으면 버튼 비활성화
>
댓글 작성
</Button>
- Textarea 글자 수 제한 추가
- 사유: 글자 수가 너무 길 경우 API 요청이 실패할 가능성이 있다고 합니다.
- 보완사항: maxLength 속성을 활용하여 글자 수를 100자로 제한하면 어떨까요?
- 코드:
<Textarea
placeholder="댓글을 입력하세요."
onChange={(e) => setCommentText(e.target.value)}
value={commentText}
maxLength={500} // 최대 500자 제한
/>
- 댓글 등록 API 호출 실패 시, 사용자 피드백 보충
- 사유: 현재는 console.log("댓글 등록 실패 : ", error); 만 출력됨
- 보완사항: Toast 컴포넌트를 활용하여 에러메시지 화면 출력
- 코드:
catch (error) {
console.log("댓글 등록 실패 : ", error);
const errorMessage = error.response?.data?.message || error.message || "댓글 등록에 실패하였습니다.";
showToast({
title: "댓글 등록 실패",
description: errorMessage,
type: "error",
duration: 3000,
error: errorMessage,
});
}
import React, { useEffect, useRef, useState } from "react"; | ||
import { Box, Flex, Button, Image, Text } from "@chakra-ui/react"; | ||
import { bringSignApi, sendSignApi } from "@/src/api/signature"; | ||
import { useParams } from "next/navigation"; |
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.
import 문 순서 맞춰서 작성해주세요.
import React, { useEffect, useRef, useState } from "react";
import { useParams } from "next/navigation";
import { Box, Flex, Button, Image, Text } from "@chakra-ui/react";
import { bringSignApi, sendSignApi } from "@/src/api/signature";
import axiosInstance from "@/src/api/axiosInstance";
import SignaturePad from "signature_pad";
import DropDownInfoTop from "@/src/components/common/DropDownInfoTop";
} | ||
}; | ||
|
||
const confirmArrroval = async () => { |
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.
Swagger 상으로는 결재 승인 API 호출 시 projectId 와 approvalId 를 파라미터로 넘겨줘야 하는 거 같은데 정상 작동하나요?
혹 아직 미구현 상태라면 주석 처리 또는 설명 함께 남겨주시고,
구현한다고 하면 다음과 같이 작성하면 될 것 같습니다. 참고 바랍니다.
const confirmArrroval = async () => {
try {
const response = await axiosInstance.post(
`projects/${projectId}/approvals/${approvalId}/confirm`,
{ projectId, approvalId } // ✅ API 요청 본문(body)으로 전달
);
alert("결재가 승인되었습니다.");
} catch (error) {
alert("승인 요청에 실패했습니다.");
console.error(error);
}
};
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.
그거 수정되어야 합니다.
파라미터 없이 넘겨주는식으로 수정될 예정이에요
} | ||
}; | ||
|
||
const rejectArrroval = async () => { |
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.
아래 결재 승인 API 와 마찬가지로 파라미터 넘겨줄 필요 없는지 검토 부탁드립니다.
참고하실 수 있도록 파라미터 넘겨주는 코드 첨부드립니다.
const rejectArrroval = async () => {
try {
await axiosInstance.post(
`projects/${projectId}/approvals/${approvalId}/reject`,
{ projectId, approvalId } // ✅ API 요청 본문(body)으로 전달
);
alert("결재가 반려되었습니다.");
} catch (error) {
alert("반려 요청에 실패했습니다.");
console.error(error);
}
};
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.
아직 api 완성되지 않아서..
try { | ||
const response = await axiosInstance.post( | ||
`projects/${projectId}/approvals/${approvalId}/reject`, | ||
); |
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.
승인/반려 요청이 성공 또는 실패할 경우 사용자 피드백(알림 표시) 코드를 추가해주시면 좋겠습니다.
catch (error) {
const errorMessage = error.response?.data?.message || error.message || "결재 승인에 실패하였습니다.";
showToast({
title: "요청 실패",
description: errorMessage,
type: "error",
duration: 3000,
error: errorMessage,
});
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.
아직 승인/반려 api 가 완성되지 않아 완성되면 작업하겠습니다
import { Box, VStack, Flex } from "@chakra-ui/react"; | ||
import { useParams } from "next/navigation"; | ||
import { Box, VStack, Flex, Text } from "@chakra-ui/react"; | ||
import { useParams, useRouter } from "next/navigation"; |
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.
이번에 PR 올리신 코드들 전부 import 문 순서 검토 후 조정 부탁드립니다.
import "./edit.css"; | ||
|
||
export default function ProjectApprovalsNewPage() { | ||
const { projectId } = useParams(); | ||
const router = useRouter(); | ||
const [title, setTitle] = useState<string>(""); | ||
|
||
const [category, setCategory] = useState<string>(""); | ||
console.log(category); | ||
const resolvedProjectId = Array.isArray(projectId) |
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.
projectId
가 배열일 가능성을 고려하여 resolvedProjectId
로 처리하지만, Next.js에서는 보통 projectId
는 string
으로만 반환되므로 Array
부분은 불필요할 수 있다고 합니다. 참고 바랍니다.
- 참고사항(코드):
// 배열일 경우 첫 번째 요소 사용, 없으면 빈 문자열 처리
const resolvedProjectId = typeof projectId === "string" ? projectId : projectId?.[0] ?? "";
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.
이거 제가 한 부분이 아니라 모르겠습니다. (상호님이 한거 그대로 가져다 씀)
); | ||
|
||
useEffect(() => { |
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.
useEffect
처럼 상태 관리를 하는 경우 '어떤 코드이고 왜 필요한지` 주석으로 남겨주실 수 있을까요?
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.
기본값 설정을 안해줬을 때 첫 렌더링 때 undefined 상태인데 ui 상으로는 undefined 가 아닌 것처럼 보여 그냥 기본값을 맨 첫번째 옵션으로 했습니다. 나중에 고쳐볼만한 요소인 것 같습니다
📄 Description of the PR
기능 설명 :
결재 기능 대부분 완성
Close # (이슈 번호)
🔧 What has been changed?
📸 Screenshots / GIFs (if applicable)
요청 종류 선택
결재 글 열람 모습
결재하면 사인패드 닫힘.
근데 새로고침하면 다시 열려서 수정해야함
✅ Checklist
[1. 외부 라이브러리 (react, axios 등) 2. 절대 경로 파일 (@components, @utils 등) 3. 스타일 파일 (.css, .scss 등)]