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

[feat]: Bookmark to survey #376

Merged
merged 13 commits into from
Feb 23, 2024
Merged

[feat]: Bookmark to survey #376

merged 13 commits into from
Feb 23, 2024

Conversation

devxb
Copy link
Member

@devxb devxb commented Feb 23, 2024

어떤 기능을 개발했나요?

survey에 북마크하는기능을 추가했습니다.

어떻게 해결했나요?

  • Target도메인에 job, image_url 필드 추가
  • Bookmark된 surveyId를 유지하는 추가 및 target 1 --> * bookmarked_survey 관계로 구성
  • 인수테스트 및 e2e 테스트 작성
  • BookmarkService 구현

이슈 넘버

참고자료

@devxb devxb changed the title feat: Bookmark to survey [feat]: Bookmark to survey Feb 23, 2024
* targetId에 해당하는 유저에게 survey를 북마크합니다.
* 이미 북마크되어있다면 북마크를 취소합니다.
*/
SurveyBookmarkDto flipBookmark(Long targetId, Long surveyId);
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
Member Author

Choose a reason for hiding this comment

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

음.. 그런가?
나눠서 만들어도 동시요청 들어오면 갱신손실은 똑같이 발생할 수 있는거아냐? (survey 모듈쪽이 클린아키텍처라서 읽고 업데이트 해야함)
결국 두 방식 모두 락 획득해서 작업해야하는데, 북마크는 갱신손실 나도 문제없을거라 생각했고
클라 입장에서 현재 북마크상태 상관없이 flip 호출만하면 되니까 이게 편할거라 생각해서 이렇게 구현했어
별루??

Copy link
Member

Choose a reason for hiding this comment

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

첫 따닥은 어차피 둘 다 막아야되고 나머지는 updatedAt 같은 거 받아서 낙관적락 같은 느낌이 멱등성 가져가도 될 거 같은디?

Copy link
Member Author

Choose a reason for hiding this comment

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

d8d662c

낙관적 락 추가함 이 로직이 동시성 문제나면 한쪽은 실패하는게 맞는거 같아서 낙관적락 예외 retry는 안했어

근데, 내가 잘 이해를 못하고있는거 같은데 이 로직에서 멱등성 가져가는게 API 사용성 가져가는거보다 장점인게 아직 와닿지 않는달까나
나는 여기서 멱등하지 않는게 사용성을 높여주는거 같아서 (누르면 반대상태로 전환됨) 흠흠...

Copy link
Member

@dojinyou dojinyou Feb 23, 2024

Choose a reason for hiding this comment

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

이런 기능들은 클라이언트쪽에서 디바운스를 걸어주는게 좋음. 북마크했다가 바로 취소하거나 할 수 있기 때문에. 그러면 최종적 상태에 대한 요청을 보내도록 유도할 수 있음.
근데 지금 위와 같은 형태는 빠르게 사용자가 요청 했다가 취소 하면 요청 2번을 처리해야 함.

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 이해했어 따닥 했을때 멱등성이 없으면, 클라이언트 요청을 맞춰주기위해서 요청처리를 2번해야하는구만
수정완
e08d6d8

나중에 취소 API도 하나 더 뚫겠음

@devxb devxb requested a review from dojinyou February 23, 2024 01:41
@devxb devxb requested a review from dojinyou February 23, 2024 03:17
Copy link

@devxb devxb merged commit 61d1dfa into main Feb 23, 2024
4 checks passed
@devxb devxb deleted the devxb/iss-#363 branch February 23, 2024 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] : 서베이 북마크 기능
2 participants