-
Notifications
You must be signed in to change notification settings - Fork 0
조회수 기능 추가 #69
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
Conversation
- 중복 조회수와 중복 제외 조회수 도메인을 생성했습니다. 이때 중복 조회수는 후에 실제 서비스 분석에 사용될 수 있도록 유저 필드를 추가했습니다.
- 생각해보니, 중복 제외 조회수와 중복 조회수 table을 따로 만드는 것은 데이터 중복성이나 무결성 측면에서 좋지 않을 것 같습니다. 따라서 중복 제외 조회수의 경우는 mysql에서 가상테이블 뷰를 통해 구성할 예정입니다.
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.
고생 많으셨습니다!
요구사항이 복잡하지 않았던만큼 깔끔하게 잘 구현하신 것 같습니다!
프론트 분들께 잘 말씀드려서 반영 잘 되면 좋겠군요!
| @RequestBody ViewCreateRequest request | ||
| ) { | ||
| if (authContext.isAnonymous()) { | ||
| viewCommandService.create(request, null, deviceId); |
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.
개인적으로 인수로 null을 추가하는 것보다는 userId를 받지 않는 create()를 오버로딩하여 처리하면 더 깔끔하지 않을까 싶습니다!
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.
이렇게 하니 service 내 코드도 더 간단해지네요... 감사합니다!!
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.
오 좋네요!!
| import org.springframework.restdocs.payload.JsonFieldType; | ||
|
|
||
| public class ViewControllerTest extends RestDocsTest { | ||
| private static final String deviceIdHeader = "Device-Id"; |
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.
이건 별거 아닌데 'Header'라는 네이밍보단 'Key'가 'deviceIdValue'와 짝이 잘 맞지 않을까? 싶습니다. 혹은 둘 다 'deviceIdHeader(*)'로 통일해도 좋구요.
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.
오호 좋은 것 같습니다. deviceIdKey로 수정해두겠습니다!
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.
넵넵!
| } | ||
|
|
||
| @Test | ||
| void 유저id_없이도_조회수를_발행해야_한다() { |
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.
굿입니다 굿굿
- viewCommandService의 create 함수를 userId를 인자로 받는 버전과 받지 않은 버전 2가지로 나누어 작성하였습니다.
- deviceIdValue와 더 잘 매칭될 수 있도록 deviceIdHeader를 deviceIdKey 로 수정하였습니다.
- 오버로딩한 두 함수 모두를 테스트할 수 있도록 코드를 수정했습니다.
- 노트 단건 조회 api시 자동으로 해당 노트에 대한 조회수가 자동으로 생성되도록 작성하였습니다.
- NoteControllerTest에 조회수 추가 기능을 넣으면서 변경된 부분에 맞게 테스트를 수정했습니다. - RestDocsTest에 getDeviceIdHeader를 만들고 DeviceId 헤더가 필요한 api에서 해당 함수를 사용하도록 수정했습니다.
🍃 Pull Requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항
실제 서비스 분석에 사용될 수 있도록 조회수에 유저 필드를 추가했는데, 실제 서비스 분석에 조회수를 사용하게 될지는 의문이네요... 유저 필드를 빼는 것이 좋을까요?
서비스에서는 중복 포함 조회수와 중복 제외 조회수가 따로 필요하지만, 실제 중복 제외 조회수 테이블은 현재 view 테이블에서 select 연산을 통해 얻을 수 있는 정보들이기에 중복 제외 조회수 테이블을 따로 생성하지 않고 가상의 table인 뷰로 만들어 mysql에서 볼 수 있도록 할 예정입니다.