Skip to content

Conversation

@nayonsoso
Copy link
Collaborator

@nayonsoso nayonsoso commented Feb 11, 2025

관련 이슈

작업 내용

회의에 나온 내용들을 반영했습니다😊

  • 내 정보 관련 uri를 /my-page/** 에서 /my/** 로 변경
  • 대학교 관련 uri를 /university/** 에서 /universities/** 로 변경
  • 위시 대학 추가&삭제 공용 api 를 추가 api 와 삭제 api 두개로 분리
  • 사용하지 않는 api들 삭제 (e.g. 프로필만 바꾸기, 이미지만 바꾸기)
  • 내 정보 수정 api 에서 프로필 사진을 url 이 아니라 파일 형태로 한번에 전송하도록 변경

특이 사항

다음 PR에서 지원 정보, 커뮤니티, 성적 등록 api 변경하겠습니다!

리뷰 요구사항 (선택)

❗️uri 변경 검토❗️
'내 위시 대학인지' 조회하는 api 가 {{URL}}/universities/{{university-id}}/like 인데요,
이걸 {{URL}}/universities/{{university-id}}/like/check 로 바꾸는건 어떤가요?
이게 더 의미 전달이 잘 되는 것 같아서요!

❗️uri 변경 사항❗️
https://github.com/solid-connection/api-docs/pull/4/files

Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 슬랙에서 e2e테스트에 대해 왜 그렇게 말씀하셨는지 바로 이해가 되네요 🥲 이건 논의를 제대로 해봐야겠네요!

@Gyuhyeok99
Copy link
Contributor

'내 위시 대학인지' 조회하는 api 가 {{URL}}/universities/{{university-id}}/like 인데요,
이걸 {{URL}}/universities/{{university-id}}/like/check 로 바꾸는건 어떤가요?
이게 더 의미 전달이 잘 되는 것 같아서요!

말씀하신 것으로 변경하는 게 더 의미가 더 명확해진다는 장점이 있는 거 같습니다!
그런데 GET 요청 자체가 조회(check)의 의미를 내포한다고 생각해서 불필요한 URI 패턴이 추가되는 건 아닌가 고민해봤습니다.

제 생각에 like와 관련해서 좋아요 상태 외에 다른 정보를 조회하는 API가 생긴다면(좋아요 수 등), check를 붙이는 것도 괜찮을 것 같습니다. 하지만, 그게 아니라면 GET 요청 자체가 "좋아요 상태"를 조회하는 의미를 충분히 내포하고 있으니, check를 붙이지 않아도 헷갈릴 여지는 없다고 생각합니다.

또한, check를 붙이게 되면 향후 북마크 같은 다른 기능에서도 API 설계의 일관성을 고려했을 때 불필요하게 URI가 길어질 가능성도 있을 것 같아요.

그래서 만약 check를 붙이지 않는다면, 주석으로 "이 API는 좋아요 상태를 확인하는 용도"라는 걸 명확하게 설명해주는 방법도 좋을 것 같습니다! 😀

@nayonsoso
Copy link
Collaborator Author

또한, check를 붙이게 되면 향후 북마크 같은 다른 기능에서도 API 설계의 일관성을 고려했을 때 불필요하게 URI가 길어질 가능성도 있을 것 같아요.

여기에 설득되었네요 😄
그럼 uri 는 그대로 두는걸로 하겠습니다~

@nayonsoso nayonsoso merged commit 89c2bce into main Feb 14, 2025
@nayonsoso nayonsoso deleted the refactor/181-university-and-my-api branch February 15, 2025 19:16
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.

3 participants