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