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.

Sign up for GitHub

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

[BE/#603] 채팅방 나갔다가 들어오면 이전 채팅 안보이게 구현 #604

Merged
merged 8 commits into from
Jan 14, 2024
Merged

[BE/#603] 채팅방 나갔다가 들어오면 이전 채팅 안보이게 구현 #604

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca8b194
Feat : 채팅방 entity 수정
namewhat99 Jan 11, 2024
ddd94c0
Feat : 채팅방 나가면 나간 시간 기록
namewhat99 Jan 11, 2024
474e43f
Feat : 채팅방 나간 후 돌아오면 이전 채팅 안 뜨게 설정
namewhat99 Jan 11, 2024
9e3755a
Fix : 주석 제거
namewhat99 Jan 11, 2024
a57e2b6
Fix : PR 수정 사항 반영
namewhat99 Jan 13, 2024
fafefd4
Fix : 오류 코드 수정
namewhat99 Jan 13, 2024
de92ad7
Feat : chat 가져올때 pagination 코드 미리 작성
namewhat99 Jan 14, 2024
c10de5a
Feat : 채팅 페이지네이션 분리 및 주석처리
namewhat99 Jan 14, 2024
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
40 changes: 31 additions & 9 deletions BE/src/chat/chat.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ export class ChatService {
'chat_info.sender as sender',
])
.where('chat_room.writer = :userId', { userId: userId })
.andWhere('chat_room.writer_left IS false')
.andWhere('chat_room.writer_hide IS false')
.orWhere('chat_room.user = :userId', { userId: userId })
.andWhere('chat_room.user_left IS false')
.andWhere('chat_room.user_hide IS false')
.orderBy('chat_info.create_date', 'DESC')
.getRawMany();

Expand Down Expand Up @@ -223,7 +223,27 @@ export class ChatService {
relations: ['chats', 'userUser', 'writerUser'],
});
Copy link
Member

Choose a reason for hiding this comment

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

일단 room이 찾아 지지 않을 때의 예외처리가 없어서 삭제되거나 존재하지 않는 roomId로 접근하면 에러가 발생 할 것 같네요..

Copy link
Member

Choose a reason for hiding this comment

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

마찬가지로 위에 update 문도 없는 방을 업데이트 하려면 예외처리가 필요할 것 같고 유저의 권한을 확인 하는 로직이 밑에 있어서 본인의 방이 아닌 경우에도 채팅 읽음 처리를 하는 문제가 생길 수도 있을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그러면 checkAuth 를 맨 처음으로 옮겨서 처음에 해당 방이 존재하는지와 해당 방에 존재하는 사람이 맞는지 확인 후에 아래 로직들을 실행하는 방식으로 바꾸겠습니당~


let chats = room.chats;
this.checkAuth(room, userId);

if (
room.writer === userId &&
room.writer_hide === false &&
room.writer_left_time !== null
) {
Comment on lines +251 to +255
Copy link
Member

Choose a reason for hiding this comment

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

room.writer_hide === false 이 부분이 true 인 경우의 처리가 없는 것 같습니다. 사용자에게 안보이도록 x_hide 가 true 라면 예외처리가 필요하지 않을까 하는 생각이 드는데 어떻게 생각하시는지요...?
404나 요런 처리가 필요하지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

url 로 접근하는 부분때문인가요? 자신이 숨긴 채팅방을 url 을 통해 400 혹은 409 로 던지면 될 것 같은데 어케 생각하시나요?

chats = chats.filter((chat) => {
return chat.create_date > room.writer_left_time;
});
} else if (
room.user === userId &&
room.user_hide === false &&
room.user_left_time !== null
) {
chats = chats.filter((chat) => {
return chat.create_date > room.user_left_time;
});
}

Copy link
Member

Choose a reason for hiding this comment

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

좀 사소하긴 하지만 findRoomById 메소드 안에서 너무 많은 작업을 하고 있어 가독성등이 좀 떨어지지 않나 싶습니다. 이 부분은 getChats() 같은 메소드를 따로 만들어서 분리하면 좀 더 깔끔하지 않을 까 싶습니다

Copy link
Member

Choose a reason for hiding this comment

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

그리고 만약 분리한다면 chat.create_date > room.user_left_time 이 조건을 만족하는 데이터만 뽑아와서 db와 통신 비용도 낮아지고 찾는데 걸리는 시간이 좀 줄어 들지 않을까 싶습니다. 현재는 모든 채팅을 다 조회하고 모든 채팅을 다 확인해서 오버헤드가 있지 않을 까 하는 생각이 듭니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

실험을 해 본 결과인데 현재 응답시간 자체가 평균적으로 30ms 로 좋은 편이기도 하고 해당 사람의 나간 시간을 얻기위해 DB 에 접근해서 시간을 얻고 다시 한번 DB 에 접근하는것 보다는 서버에서 가져온 데이터를 처리해서 주는게 현재로써는 조금 더 좋을거라고 생각이 드네요. 나중에 혹시 이에 관해서 문제가 생기면 말씀해주신 방법으로 바꿔보는것도 좋을 것 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

메서드 분리는 진행하겠읍니다~

return {
writer: room.writer,
writer_profile_img:
Expand All @@ -236,7 +256,7 @@ export class ChatService {
? this.configService.get('DEFAULT_PROFILE_IMAGE')
: room.userUser.profile_img,
post_id: room.post_id,
chat_log: room.chats,
chat_log: chats,
};
}

Expand Down Expand Up @@ -275,10 +295,10 @@ export class ChatService {
where: { id: roomId },
});

if (room.writer === userId && room.user_left !== false) {
room.user_left = false;
} else if (room.user === userId && room.writer_left !== false) {
room.writer_left = false;
if (room.writer === userId && room.user_hide !== false) {
room.user_hide = false;
} else if (room.user === userId && room.writer_hide !== false) {
room.writer_hide = false;
}
await this.chatRoomRepository.save(room);
}
Expand All @@ -302,9 +322,11 @@ export class ChatService {
});

if (room.writer === userId) {
room.writer_left = true;
room.writer_hide = true;
room.writer_left_time = new Date();
} else if (room.user === userId) {
room.user_left = true;
room.user_hide = true;
room.user_left_time = new Date();
}

await this.chatRoomRepository.save(room);
Expand Down
10 changes: 8 additions & 2 deletions BE/src/entities/chatRoom.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,16 @@ export class ChatRoomEntity {
last_chat_id: number;

@Column({ default: false })
writer_left: boolean;
writer_hide: boolean;

@Column({ default: false })
user_left: boolean;
user_hide: boolean;

@Column({ default: null, type: 'timestamp' })
writer_left_time: Date;

@Column({ default: null, type: 'timestamp' })
user_left_time: Date;

@OneToMany(() => ChatEntity, (chat) => chat.chatRoom)
chats: ChatEntity[];
Expand Down
Loading