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

Feature/2 : 상품 리스트 조회 api 추가 #4

Merged
merged 3 commits into from
Sep 28, 2024
Merged

Feature/2 : 상품 리스트 조회 api 추가 #4

merged 3 commits into from
Sep 28, 2024

Conversation

soleu
Copy link
Collaborator

@soleu soleu commented Sep 28, 2024

#️⃣연관된 이슈

#2

📝작업 내용

  1. 메인 화면 : 상품 리스트 조회 api 추가
  1. IN_PROGRESS, WAIT 상태의 모든 상품 조회
  2. 자신의 상품인지 확인할 수 있는 flag 값 추가
    로그인 관련은 후에 api 개발 후, 추가 개발 예정
  1. 에러 핸들러 추가

스크린샷 (선택)

image

@soleu soleu added the feature label Sep 28, 2024
@soleu soleu requested a review from mason136 September 28, 2024 08:31
@soleu soleu self-assigned this Sep 28, 2024
Copy link

sonarcloud bot commented Sep 28, 2024

@@ -0,0 +1,3 @@
package com.get_offer.common.exception

class ProductNotFoundException(message: String) : RuntimeException(message)
Copy link
Collaborator

Choose a reason for hiding this comment

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

not found 에러일 때마다 에러 클래스를 만드실 건가요?
응답 코드, 에러 코드, 에러 메세지를 하나의 예외 클래스로 전달할 수 있지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

에러 메세지는 각 �값마다 분리하고 싶어서 message로 보내고, 그 외에 것들은 통일하겠습니다!

package com.get_offer.common.exception

class ErrorMessageModel(
var status: Int? = null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

응답 바디에 Http 상태 코드를 넣을 필요가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

지금 생각해보니 굳이 필요없을것같아 Api Reponse와 통합하겠습니다

private val productService: ProductService
) {
@GetMapping("/activeList")
fun getActiveProductList(@RequestParam userId: String): List<ActiveProductListDto> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

나중에는 인증 도입해서 인증한 사용자의 유저 id를 넘기실 예정일까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

API 응답으로 List를 내보내기보단 객체로 감싸서 내보내는게 클라 개발자분들이 이용하기에는 더 편합니다.
API 응답 클래스를 하나 만드시죠

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 인증 도입해서 리졸버로 처리할 예정입니다
api 응답 클래스는 추가해놓겠습니당

'2024-01-01 00:00:00', '2024-01-01 00:00:00');
INSERT INTO USERS (ID, NICKNAME, USER_IMAGE, CREATED_AT, UPDATED_AT)
VALUES (2, 'test', 'https://drive.google.com/file/d/1R9EIOoEWWgPUhY6e-t4VFuqMgknl7rm8/view?usp=sharing',
'2024-01-01 00:00:00', '2024-01-01 00:00:00');
Copy link
Collaborator

Choose a reason for hiding this comment

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

sql 파일은 테스트에서만 쓰는데 테스트 폴더 resource로 옮기시죠

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sql 파일은 로컬 개발 환경에서는 테스트 없이도 필요할 것 같아, spring boot 뜰때마다 스크립트 실행하도록 다음 pr에서 수정하였습니다 :)

productRepository.findAllByStatusInOrderByEndDateDesc(listOf(ProductStatus.IN_PROGRESS, ProductStatus.WAIT))

return productList.map { x ->
val imageList = x.images.split(";")
Copy link
Collaborator

Choose a reason for hiding this comment

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

구분자로 쓰는 방식보다는 json 컬럼이나 별도의 테이블 쓰는 방식을 추천드립니다.
사용자가 직접 썸네일 지정하는 기능도 제공할 수 있고 직관적입니다.
이미지 데이터에 ;가 있으면 어떻게 하죠?,, 구분자는 비추천드려요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네네! 좋은 방식인것같아요!
바꾸는게 조금 걸릴 것 같아 다음 pr때 변경해놓겠습니다 :)

@soleu soleu merged commit 50e1774 into main Sep 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants