Skip to content
This repository has been archived by the owner on Aug 10, 2023. It is now read-only.

기능구현 - 유저정보CRUD(Delete) (#3) #8

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Conversation

dongwoochoi
Copy link

@dongwoochoi dongwoochoi commented May 22, 2023

개요

유저정보 CRUD기능 중 Delete 기능 구현

구현

  • Delete페이지에서는 User의 상태를 두가지로 구분
    -> 2중 인증이 안된 상태 / 2중 인증이 된 상태

2중인증이 안된 상태

  • Delete 버튼 클릭시 2중인증을 위한 팝업창 생성
  • 팝업창에서 유저의 Password를 한번더 입력함으로 써 2중인증 -> 로그인 endpoint에 요청
    -> id는 로그인할때 저장한 유저의 아이디, 패스워드는 재입력 받은 패스워드로 요청

2중인증이 된 상태

  • Delete 버튼 클릭시 회원 탈퇴

@dongwoochoi dongwoochoi added the enhancement 새 기능 또는 요청 label May 22, 2023
@dongwoochoi dongwoochoi changed the base branch from master to develop May 22, 2023 13:06
@98StarJune
Copy link
Member

개요

현재 본 브랜치의 이름은 delete 로 설정되어 있습니다. 현재의 브랜치 명은 정확하게 어떤 작업을 진행 중인지 알기 쉽지 않습니다.

제안사항

현재 멤버님께서 진행해주신 작업은 User 관련 Delete 작업이므로 feat/user_delete 와 같이 브랜치 명을 의미있게 만들 수 있습니다.

Copy link
Member

@98StarJune 98StarJune left a comment

Choose a reason for hiding this comment

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

코드 리뷰입니다

귀하의 노고에 진심으로 감사드립니다. 본 PR에 대한 리뷰를 다음과 같이 남겨드립니다.

제안 사항에 대한 수정

제안 사항으로 표시된 리뷰는 수정 제안을 드리는 것으로 타 파트원 및 다른 운영진의 의견을 첨언 받아 최종 결정하시기 바랍니다.

@@ -0,0 +1,3 @@
export interface d_response{
Copy link
Member

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로 데이터 교환 시에 사용되는 객체를 뜻합이다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

const fetchDelete = async () => {
try {
const response: AxiosResponse<d_response> = await axios.get('https://www.ideaconnect.online/auth');
console.log(response);
Copy link
Member

Choose a reason for hiding this comment

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

콘솔 출력

디버깅 용도로 사용된 Console 출력이 삭제되지 않았습니다. 사용자에게 노출 할 필요가 없는 내용의 경우 별도로 출력하지 않는 것이 일반적입니다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!


const fetchDelete = async () => {
try {
const response: AxiosResponse<d_response> = await axios.get('https://www.ideaconnect.online/auth');
Copy link
Member

Choose a reason for hiding this comment

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

URL의 재사용성 추천

현 상태와 같이 코드에 URL이 직접 작성된 상태로 URL의 변경이 발생할 경우 리팩토링을 진행해야 합니다. 따라서 URL 변경 가능성을 미리 대비하는 것이 좋습니다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

};

const fetchAuthentication = async () =>{
console.log(pw)
Copy link
Member

Choose a reason for hiding this comment

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

디버깅용 콘솔 출력

내용은 위와 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

'id' : '쿠키에 저장되어있던 id',
'pw' : pw
});
console.log(response);
Copy link
Member

Choose a reason for hiding this comment

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

디버깅용 콘솔 출력

내용은 위와 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

const fetchAuthentication = async () =>{
console.log(pw)
try{
const response: AxiosResponse<Response> = await axios.post('https://www.ideaconnect.online/login',{
Copy link
Member

Choose a reason for hiding this comment

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

URL의 재사용성 추천

내용은 위와 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

@98StarJune
Copy link
Member

merge 시 충돌이 예상됩니다

현재 수정된 package.json과 page.tsx는 해당 파트의 모든 멤버가 동시에 수정한 것으로 판단됩니다. 이 경우 merge 시에 충돌이 발생할 수 있습니다.

제안

이런것과 유사하게 모두에게 적용되어야하는 것이 있을 경우 별도의 이슈로 분리하고 develp에서 별도로 분기한 브랜치에서 작업하시는 것을 추천드립니다.

Copy link
Member

@98StarJune 98StarJune left a comment

Choose a reason for hiding this comment

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

(2차) 코드 리뷰입니다

귀하의 노고에 진심으로 감사드립니다. 본 PR에 대한 리뷰를 다음과 같이 남겨드립니다.

제안 사항에 대한 수정

제안 사항으로 표시된 리뷰는 수정 제안을 드리는 것으로 타 파트원 및 다른 운영진의 의견을 첨언 받아 최종 결정하시기 바랍니다.


const fetchDelete = async () => {
try {
const response: AxiosResponse<UserDeleteResponseDto> = await axios.get(`${AUTH_API_BASE_URL}/auth`);
Copy link
Member

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;

Copy link
Author

Choose a reason for hiding this comment

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

완료했습둥. Delete같은 경우 status 코드만 넘어오기 때문에 data를 위한 별도의 인터페이스는 따로 정의하지 않았습니다(기존에 정의했던 인터페이스를 삭제했습니다.)

@dongwoochoi dongwoochoi changed the title 기능구현 - 유저정보CRUD(Delete) 기능구현 - 유저정보CRUD(Delete) (#3) May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement 새 기능 또는 요청
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants