Feature/user page #76

hywook4 · 2021-08-18T11:38:10Z

resolves #75

zych1751

물론 코드를 읽어보면 알겠지만, 이슈를 해결한 것이면 간단하게라도 뭐가 문제였는지 + 어떻게 해결했는지를 알려주시면 좋을것 같아요 :)
api 수정이 생기면 우선은 swagger처럼 따로 관리하고 있는게 없으니 doc에 정리해주세요.

zych1751 · 2021-08-18T13:02:41Z

src/api/v1/follow.ts

@@ -139,10 +139,10 @@ router.get('/', async (request: express.Request, response: express.Response) =>
  }

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


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

zych1751 · 2021-08-18T13:06:23Z

src/api/v1/follow.ts

  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)


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이 나올 수 도 있을것 같아서요.

zych1751 · 2021-08-18T13:06:47Z

src/api/v1/logoffedPostList.ts

    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)


위와 동일

zych1751 · 2021-08-18T13:08:44Z

src/api/v1/logoffedPostList.ts

+
    response.json(videoListResponse);
+


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

zych1751 · 2021-08-18T13:09:00Z

src/api/v1/logoffedPostList.ts

@@ -40,11 +40,14 @@ router.get('/recent-videos', async (request: express.Request, response: express.
        return;
    }

+


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

zych1751 · 2021-08-18T13:14:18Z

src/api/v1/user.ts

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


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

zych1751 · 2021-08-18T13:20:04Z

src/api/v1/user.ts

@@ -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) => {


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

zych1751 · 2021-08-18T13:23:11Z

src/api/v1/user.ts

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


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으로 전체적으로 바꿔도 좋을듯 하네요.

zych1751 · 2021-08-18T13:24:54Z

src/api/v1/user.ts

+    if(userId == null){
+        userId = request.body.userInfo ? request.body.userInfo.userId : null;
+    }


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

zych1751 · 2021-08-18T13:26:59Z

src/api/v1/user.ts

+});
+
+
+router.get('/user-info/:userId', async (request: express.Request, response: express.Response) => {


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

hywook4 added 3 commits August 16, 2021 23:52

Add api for user and change path for logged in user info

c3621da

FIX #75 to return right pageTokwn

76bb189

Resolve conflicts

f31a996

zych1751 requested changes Aug 18, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/user page #76

Feature/user page #76

hywook4 commented Aug 18, 2021

zych1751 left a comment

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

zych1751 Aug 18, 2021

		@@ -139,10 +139,10 @@ router.get('/', async (request: express.Request, response: express.Response) =>
		}

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

	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

		@@ -40,11 +40,14 @@ router.get('/recent-videos', async (request: express.Request, response: express.
		return;
		}

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

		});


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

Feature/user page #76

Are you sure you want to change the base?

Feature/user page #76

Conversation

hywook4 commented Aug 18, 2021

zych1751 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment