Conversation
개요현재 본 브랜치의 이름은 제안사항현재 멤버님께서 진행해주신 작업은 User 관련 Delete 작업이므로 |
98StarJune
left a comment
There was a problem hiding this comment.
코드 리뷰입니다
귀하의 노고에 진심으로 감사드립니다. 본 PR에 대한 리뷰를 다음과 같이 남겨드립니다.
제안 사항에 대한 수정
제안 사항으로 표시된 리뷰는 수정 제안을 드리는 것으로 타 파트원 및 다른 운영진의 의견을 첨언 받아 최종 결정하시기 바랍니다.
src/types/delete.ts
Outdated
| @@ -0,0 +1,3 @@ | |||
| export interface d_response{ | |||
There was a problem hiding this comment.
명확성 부족
변수명, 함수명, 클래스 명과 같이 어떤 이름을 정의할 때는 의미를 담고 있어야 하며, 이를 한 눈에 알아보기 쉽게 하는 것이 도움됩니다.
제안
User와 관련된 작업이며 Delete의 Response에 대한 내용이므로 UserDeleteResponseDto로 제안드립니다. 다만, 명칭은 각 언어별로 권장하는 방식이 존재합니다. 더 좋은 방식의 이름을 정할 수 있디면 해당 방법을 채택하는 것을 추천드립니다.
참고 : DTO란?
DTO는 Data Transfer Object로 데이터 교환 시에 사용되는 객체를 뜻합이다.
src/utils/popup.tsx
Outdated
| const fetchDelete = async () => { | ||
| try { | ||
| const response: AxiosResponse<d_response> = await axios.get('https://www.ideaconnect.online/auth'); | ||
| console.log(response); |
There was a problem hiding this comment.
콘솔 출력
디버깅 용도로 사용된 Console 출력이 삭제되지 않았습니다. 사용자에게 노출 할 필요가 없는 내용의 경우 별도로 출력하지 않는 것이 일반적입니다.
src/utils/popup.tsx
Outdated
|
|
||
| const fetchDelete = async () => { | ||
| try { | ||
| const response: AxiosResponse<d_response> = await axios.get('https://www.ideaconnect.online/auth'); |
There was a problem hiding this comment.
URL의 재사용성 추천
현 상태와 같이 코드에 URL이 직접 작성된 상태로 URL의 변경이 발생할 경우 리팩토링을 진행해야 합니다. 따라서 URL 변경 가능성을 미리 대비하는 것이 좋습니다.
src/utils/popup.tsx
Outdated
| }; | ||
|
|
||
| const fetchAuthentication = async () =>{ | ||
| console.log(pw) |
src/utils/popup.tsx
Outdated
| 'id' : '쿠키에 저장되어있던 id', | ||
| 'pw' : pw | ||
| }); | ||
| console.log(response); |
src/utils/popup.tsx
Outdated
| const fetchAuthentication = async () =>{ | ||
| console.log(pw) | ||
| try{ | ||
| const response: AxiosResponse<Response> = await axios.post('https://www.ideaconnect.online/login',{ |
merge 시 충돌이 예상됩니다현재 수정된 package.json과 page.tsx는 해당 파트의 모든 멤버가 동시에 수정한 것으로 판단됩니다. 이 경우 merge 시에 충돌이 발생할 수 있습니다. 제안이런것과 유사하게 모두에게 적용되어야하는 것이 있을 경우 별도의 이슈로 분리하고 develp에서 별도로 분기한 브랜치에서 작업하시는 것을 추천드립니다. |
src/utils/popup.tsx
Outdated
|
|
||
| const fetchDelete = async () => { | ||
| try { | ||
| const response: AxiosResponse<UserDeleteResponseDto> = await axios.get(`${AUTH_API_BASE_URL}/auth`); |
There was a problem hiding this comment.
타입이 올바르지 않습니다
axios의 통신 결과가 반환되며 UserDeleteResponseDto는 response.data에 있을 것으로 추측됩니다.
제안 코드
const response : AxiosResponse<Response> = await axios.get(...);
const { status: number, data: UserDeleteResponseDto} = response;There was a problem hiding this comment.
완료했습둥. Delete같은 경우 status 코드만 넘어오기 때문에 data를 위한 별도의 인터페이스는 따로 정의하지 않았습니다(기존에 정의했던 인터페이스를 삭제했습니다.)
개요
유저정보 CRUD기능 중 Delete 기능 구현
구현
-> 2중 인증이 안된 상태 / 2중 인증이 된 상태
-> id는 로그인할때 저장한 유저의 아이디, 패스워드는 재입력 받은 패스워드로 요청