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

Feature/user page #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/api/v1/follow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ router.get('/', async (request: express.Request, response: express.Response) =>
}

const follows = result.map(data => data.dataValues[target]);

Copy link
Collaborator

Choose a reason for hiding this comment

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

필요없는 공백이 들어갔네요.

response.json({
follows: follows,
pageToken: follows.length > 0 ? follows[follows.length - 1].id : null
pageToken: follows.length > 0 ? result[result.length-1].dataValues.id : parseInt(request.query.pageToken as string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pageToken: follows.length > 0 ? result[result.length-1].dataValues.id : parseInt(request.query.pageToken as string)
pageToken: follows.length > 0 ? result[result.length-1].dataValues.id : lastFollowId

위 코드가 깔끔할 것 같긴한데, null에서 바꾼 이유가 있을까요? parseInt의 결과가 NaN이 나올 수 도 있을것 같아서요.

});

return;
Expand Down
5 changes: 4 additions & 1 deletion src/api/v1/logoffedPostList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ router.get('/recent-videos', async (request: express.Request, response: express.
return;
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

특별한 이유가 있는게 아니면 newline은 1개가 좋을 것 같아요.

const videoListResponse: VideoListResponse = {
videoList: videos.map((video) => video.toVideoResponse()),
pageToken: videos.length > 0 ? videos[videos.length - 1].id : null
pageToken: videos.length > 0 ? videos[videos.length - 1].id : parseInt(request.query.pageToken as string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

위와 동일

};

response.json(videoListResponse);

Comment on lines +48 to +50
Copy link
Collaborator

Choose a reason for hiding this comment

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

다 띄워 적는것 보다는 관련되어있는 부분은 같이쓰면 좋을 것 같아요.

return;
});

Expand Down
32 changes: 31 additions & 1 deletion src/api/v1/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,39 @@ router.put('/profile-image', async (request: express.Request, response: express.
});


router.get('/user-info', async (request: express.Request, response: express.Response) => {
router.get('/login-user-info', async (request: express.Request, response: express.Response) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 수정 안해도 되는데, my-user-info가 직관적일것 같긴해요.

const userId = request.body.userInfo ? request.body.userInfo.userId : null;

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서도 마찬가지로 관련있는 단위끼리는 newline없이 묶어주세요.

if(!userId){
errorSend(response, 'require_body_parameter_userId', null);
return;
}

const userInfo = await User.findByPk(userId);

if(!userInfo){
errorSend(response, 'no_user_found', 'No corresponding user for given token');
return;
}

response.json({
id: userInfo.id,
email: userInfo.email,
nickname: userInfo.nickname,
imagePath: userInfo.imagePath,
aboutMe: userInfo.aboutMe
});
Comment on lines +108 to +114
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
response.json({
id: userInfo.id,
email: userInfo.email,
nickname: userInfo.nickname,
imagePath: userInfo.imagePath,
aboutMe: userInfo.aboutMe
});
response.json(userInfo.toUserInfoResponse());

이렇게 진행하되, userInfo를 user으로 전체적으로 바꿔도 좋을듯 하네요.

return;
});


router.get('/user-info/:userId', async (request: express.Request, response: express.Response) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 API는 로그인을 하지 않은 상태에서도 볼 수 있어야 하지 않을까요? 의견 부탁드릴게요.
만약 로그인을 안한 상태에서도 볼 수 있게 하려면, /v1/user에서 middleware를 통해 로그인 검사를 하고 있는 구조를 좀 바꿔줘야 할 것 같아요.

let userId = request.params.userId;

if(userId == null){
userId = request.body.userInfo ? request.body.userInfo.userId : null;
}
Comment on lines +122 to +124
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 요청에서 userId가 없다고 본인의 정보를 내려주는 건 이상한 것 같아요. 차라리 400 에러를 주고, 프론트에서 처리를 하는게 맞을 것 같아요.


if(!userId){
errorSend(response, 'require_body_parameter_userId', null);
return;
Expand Down
5 changes: 4 additions & 1 deletion src/api/v1/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ router.get('/user-videos/:userId', async (request: express.Request, response: ex
return;
}


const videoListResponse: VideoListResponse = {
videoList: videos.map((video) => video.toVideoResponse()),
pageToken: videos.length > 0 ? videos[videos.length - 1].id : null
pageToken: videos.length > 0 ? videos[videos.length - 1].id : parseInt(request.query.pageToken as string)
};

response.json(videoListResponse);

Comment on lines +83 to +90
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 파일도 위와 동일.

return;
});

Expand Down