-
Notifications
You must be signed in to change notification settings - Fork 1
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
[POM-94] feat: 가게 전화번호 등록 기능 구현 #31
Changes from all commits
cf7caad
45892e2
f09cbab
385e175
3d7b3ba
fedbf50
02158f6
14a111b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package com.ray.pominowner.store.controller.dto; | ||
|
||
import jakarta.validation.constraints.NotBlank; | ||
|
||
public record PhoneNumberRequest(@NotBlank String phoneNumber) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package com.ray.pominowner.store.domain; | ||
|
||
import jakarta.persistence.Column; | ||
import jakarta.persistence.Embeddable; | ||
import lombok.AccessLevel; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.NoArgsConstructor; | ||
import org.springframework.util.Assert; | ||
|
||
import java.util.regex.Pattern; | ||
|
||
@Embeddable | ||
@EqualsAndHashCode | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
public class PhoneNumber { | ||
|
||
private static final String DEFAULT_VALUE = "NOT_SELECTED"; | ||
private static final String REGEX = "^01([0|1|6|7|8|9])-?([0-9]{3,4})-?([0-9]{4})$"; | ||
|
||
@Column(columnDefinition = "TEXT default 'NOT_SELECTED'") | ||
private String tel = DEFAULT_VALUE; | ||
|
||
public PhoneNumber(String phoneNumber) { | ||
validatePhoneNumber(phoneNumber); | ||
|
||
this.tel = phoneNumber; | ||
} | ||
|
||
private void validatePhoneNumber(String phoneNumber) { | ||
Assert.hasText(phoneNumber, "올바른 전화번호를 입력해 주세요."); | ||
Assert.isTrue(Pattern.matches(REGEX, phoneNumber), "올바른 전화번호를 입력해 주세요."); | ||
} | ||
|
||
public String getTel() { | ||
return tel; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
import jakarta.persistence.GeneratedValue; | ||
import jakarta.persistence.Id; | ||
import lombok.AccessLevel; | ||
import lombok.Builder; | ||
import lombok.NoArgsConstructor; | ||
import org.springframework.util.Assert; | ||
|
||
|
@@ -27,27 +28,51 @@ public class Store extends BaseTimeEntity { | |
@Column(columnDefinition = "TEXT default 'NOT_SELECTED'") | ||
private String info = DEFAULT_VALUE; // 선택 | ||
|
||
@Embedded | ||
@Column(columnDefinition = "TEXT default 'NOT_SELECTED'") | ||
private String tel = DEFAULT_VALUE; // 선택 | ||
private PhoneNumber tel = new PhoneNumber(); // 선택 | ||
|
||
private Long ownerId; // 추후 연관관계 설정 예정 | ||
|
||
public Store(final RequiredStoreInfo requiredStoreInfo) { | ||
@Builder | ||
private Store(Long id, RequiredStoreInfo requiredStoreInfo, String info, PhoneNumber tel, Long ownerId) { | ||
this.id = id; | ||
this.requiredStoreInfo = requiredStoreInfo; | ||
this.info = info; | ||
this.tel = tel; | ||
this.ownerId = ownerId; | ||
} | ||
|
||
public Store(RequiredStoreInfo requiredStoreInfo) { | ||
validateRequiredInfo(requiredStoreInfo); | ||
|
||
this.requiredStoreInfo = requiredStoreInfo; | ||
} | ||
|
||
private void validateRequiredInfo(final RequiredStoreInfo requiredStoreInfo) { | ||
private void validateRequiredInfo(RequiredStoreInfo requiredStoreInfo) { | ||
Assert.notNull(requiredStoreInfo, "가게 정보는 필수 입니다."); | ||
} | ||
|
||
public String getBusinessNumber() { | ||
return requiredStoreInfo.getBusinessNumber(); | ||
} | ||
|
||
public Store afterRegisterPhoneNumber(String phoneNumber) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메소드명만 보고는 폰 번호 등록 이후에 어떤 기능을 수행하는지 잘 모르겠어요 |
||
return Store.builder() | ||
.id(this.id) | ||
.requiredStoreInfo(this.requiredStoreInfo) | ||
.info(this.info) | ||
.tel(new PhoneNumber(phoneNumber)) | ||
.ownerId(this.ownerId) | ||
.build(); | ||
} | ||
|
||
public Long getId() { | ||
return id; | ||
} | ||
|
||
public String getBusinessNumber() { | ||
return requiredStoreInfo.getBusinessNumber(); | ||
public String getTel() { | ||
return tel.getTel(); | ||
Comment on lines
+74
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 필드 tel 의 타입과 해당 필드의 getter 타입이 달라요 |
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ public class StoreService { | |
@Transactional | ||
public Long registerStore(Store store) throws JsonProcessingException { | ||
storeServiceValidator.validateBusinessNumber(store.getBusinessNumber()); | ||
|
||
return storeRepository.save(store).getId(); | ||
} | ||
|
||
|
@@ -34,6 +35,12 @@ public void registerCategory(final List<String> categories, Long storeId) { | |
storeCategoryService.saveCategories(findStore(storeId), categories); | ||
} | ||
|
||
@Transactional | ||
public void registerPhoneNumber(String phoneNumber, Long storeId) { | ||
Store store = findStore(storeId).afterRegisterPhoneNumber(phoneNumber); | ||
storeRepository.save(store); | ||
} | ||
|
||
Comment on lines
+39
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. repostiroy.save() 메소드의 return 값을 뭔가 유용하게 이용할 수 있을 것 같아요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금 컨트롤러에 굳이 넘겨 줘야할 필요가 없어서 (자원이 생성된게 아니기 때문에) return 값을 설정하지 않았어요 |
||
private Store findStore(final Long storeId) { | ||
return storeRepository.findById(storeId) | ||
.orElseThrow(() -> new IllegalArgumentException("존재하지 않는 가게입니다.")); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package com.ray.pominowner.store.domain; | ||
|
||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.NullSource; | ||
import org.junit.jupiter.params.provider.ValueSource; | ||
|
||
import static org.assertj.core.api.Assertions.assertThatNoException; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
class PhoneNumberTest { | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"abcdef", "0212345678", "02", "", " "}) | ||
@NullSource | ||
@DisplayName("전화번호 형식이 올바르지 않으면 예외가 발생한다") | ||
void failWhenIncorrectPhoneNumberFormat(String phoneNumber) { | ||
// when, then | ||
assertThatThrownBy(() -> new PhoneNumber(phoneNumber)) | ||
.isInstanceOf(IllegalArgumentException.class) | ||
.hasMessage("올바른 전화번호를 입력해 주세요."); | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"010-1234-5678", "01012345678", "011-111-1111"}) | ||
@DisplayName("전화번호 형식이 올바르면 생성에 성공한다") | ||
void successWhenCorrectPhoneNumberFormat(String phoneNumber) { | ||
// when, then | ||
assertThatNoException().isThrownBy(() -> new PhoneNumber(phoneNumber)); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,7 +50,8 @@ void successRegisterStore() throws JsonProcessingException { | |
given(storeRepository.save(store)).willReturn(store); | ||
|
||
// when, then | ||
assertThatNoException().isThrownBy(() -> storeService.registerStore(store)); | ||
assertThatNoException() | ||
.isThrownBy(() -> storeService.registerStore(store)); | ||
} | ||
|
||
|
||
|
@@ -67,4 +68,16 @@ void successRegisterCategories() { | |
.isThrownBy(() -> storeService.registerCategory(categoryRequest, store.getId())); | ||
} | ||
|
||
@Test | ||
@DisplayName("전화번호가 정상적으로 등록된다") | ||
void successRegisterPhoneNumber() { | ||
// given | ||
String validPhoneNumber = "010-1234-5678"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. StoreTest 와 이 test 파일간의 유효한 폰 번호 문자열의 포맷이 달라요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 상관없다고 생각해요 어차피 유효한 번호인것만 테스트 하면 되니까요 |
||
given(storeRepository.findById(store.getId())).willReturn(Optional.of(store)); | ||
|
||
// when, then | ||
assertThatNoException() | ||
.isThrownBy(() -> storeService.registerPhoneNumber(validPhoneNumber, store.getId())); | ||
} | ||
|
||
} |
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.
registerPhoneNumber
앞에 after를 붙여준 이유가 있나요?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.
store.afterRegisterPhoneNumber()
가 자연스럽다고 생각했어요메서드 명 정할때 중요한 점이 사용하는 클라이언트 쪽에서 메서드 명을 정해야지 받는쪽 입장에서 메서드 명을 정하는 것은 객체지향적이지 못하다고 생각해요
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.
반환값이 store 엔티티이기 때문에 after 붙히는게 더 자연스럽다 생각합니다 :)