마이페이지, 메인페이지 CRUD 페이지 추가 #59

HyunJunSon · 2024-01-16T10:20:19Z

💡 Motivation and Context

여기에 왜 이 PR이 필요했는지, PR을 통해 무엇이 바뀌는지에 대해서 설명해 주세요

🔨 Modified

여기에 무엇이 크게 바뀌었는지 설명해 주세요

Mypage CRUD 페이지 추가
mainPage 페이지 추가
Entity 연관관계 추가
로그인 회원정보가 필요한 사항은 기본 스트링으로 처리해 두었습니다.

🌟 More

여기에 PR 이후 추가로 해야 할 일에 대해서 설명해 주세요
CI/CD 배포 깔끔하게 완료하겠습니다.

📋 커밋 전 체크리스트

[o] 추가/변경에 대한 단위 테스트를 완료하였습니다.
[o] 컨벤션에 맞게 작성하였습니다.

🤟🏻 PR로 완료된 이슈

closes #

Entity 연관관계 정보추가

cr-gpt · 2024-01-16T10:20:36Z

docker-compose.yml

@@ -36,7 +37,7 @@ services:
 # db-read:
 # image: younglong/pg-washpedia:latest
 # volumes:
-# - postgreEditDB:/var/lib/postgresql/data
+# - ./postgreEditDB:/var/lib/postgresql/data
 # environment:
 # POSTGRES_DB: read_db
 # POSTGRES_USER: ${{ secrets.PROJECT_USER }}


Based on the code patch you provided, here are some observations and suggestions for improvement:

In the batch service definition, you have commented out the image line and added a new line with ${IMAGE}. If you intend to use an environment variable as the image name, make sure that the ${IMAGE} variable is properly defined and set before using it in the docker-compose.yml file.

In the batch service definition, you changed the port mapping from "8082:8080" to "8082:8082". Make sure this change aligns with the configuration inside the Docker container. If the container is listening on port 8080, then the previous mapping was correct. If it's actually listening on port 8082, then this change is appropriate.

In the db-read service definition, you changed the volume mapping from postgreEditDB:/var/lib/postgresql/data to ./postgreEditDB:/var/lib/postgresql/data. Make sure that the postgreEditDB volume is properly defined and exists, or alternatively, replace it with a valid local directory path on your host machine.

Overall, without the full context of the Dockerfile, complete docker-compose.yml file, and any related scripts or dependencies, it's difficult to identify potential bugs or suggest further improvements. However, the points mentioned above should be considered and verified in your specific environment.

cr-gpt · 2024-01-16T10:20:52Z

module-api/build.gradle

+ // test - fixture Monkey
+ testImplementation 'net.jqwik:jqwik:1.7.3'
+ testImplementation("com.navercorp.fixturemonkey:fixture-monkey-starter:1.0.0")
+ testImplementation("com.navercorp.fixturemonkey:fixture-monkey-jakarta-validation:1.0.0")
 }

 tasks.named('test') {


Overall, the code patch seems to include additions to the project dependencies. Here are some observations and suggestions:

Validataion: The line implementation 'org.springframework.boot:spring-boot-starter-validation:2.7.4' adds the Spring Boot Starter Validation library for validating input data. This seems fine if you need validation support in your application.

Test Dependencies: The lines starting with testImplementation add test dependencies. Including the Spring Boot Starter Test and Spring Security Test is common practice for testing Spring applications. Additionally, two new dependencies are added:

testImplementation 'net.jqwik:jqwik:1.7.3' adds the jqwik library for property-based testing.

testImplementation("com.navercorp.fixturemonkey:fixture-monkey-starter:1.0.0") and testImplementation("com.navercorp.fixturemonkey:fixture-monkey-jakarta-validation:1.0.0") add the FixtureMonkey libraries for generating test data and handling Jakarta Validation annotations.

Improvement Suggestions:

It's generally a good practice to specify the version of Spring Boot used throughout the project, including the main dependencies mentioned above (Spring Boot Starter Validation, Spring Boot Starter Test, Spring Security Test).

Make sure that the versions provided (2.7.4, 1.7.3, and 1.0.0) are the latest stable versions available at the time you're using them, as newer versions may provide bug fixes and improvements.

For a more thorough review, additional context about the project and its requirements would be helpful.

chan99k

이전 PR 에서 봤던 변경점도 같이 올라온 것 같은데, 이전 PR을 merge 하시고 합친 다음에 다시 올리시면 좋을 거 같습니다.

chan99k

이전 PR에 포함되었던 커밋도 현재 PR에 함께 포함되어 있어 변경점을 확인하기 어렵습니다.

이전에 올리셨던 PR을 머지시키고 난 뒤에 합쳐서 올려주시면 좋겠어요. 커밋 로그를 통해서 변경점들은 확인하였습니다.

chan99k · 2024-01-17T01:30:02Z

module-api/src/main/java/com/kernel360/main/controller/MainContoller.java

+@RestController
+public class MainContoller {
+ @GetMapping("")
+ ResponseEntity<String> mainPage(){
+ return ResponseEntity.status(HttpStatus.OK).body("main page입니다.");
+ }


커밋 새로해서 push 한 것에 반영되었습니다. pull 받는걸 깜빡하고 바로 올렸나 봅니다~ ㅠㅠ

chan99k · 2024-01-17T01:32:17Z

module-api/src/main/java/com/kernel360/mypage/controller/MyPageController.java

+ String bannerImageUrl = "http://localhost:8080/bannersample.png";
+ String suggestImageUrl = "http://localhost:8080/suggestsample.png";


chan99k · 2024-01-17T01:38:52Z

module-domain/src/main/java/com/kernel360/washinfo/entity/WashInfo.java

+ @OneToOne
 @JoinColumn(name = "member_no", nullable = false)
- private Member memberNo;
+ private Member member;


linglong67

flyway -> alter column 파일만 추가 반영해주세요~
(제가 #61 PR로 올렸습니다!)

linglong67 · 2024-01-17T02:45:30Z

module-api/src/main/java/com/kernel360/mypage/controller/MyPageController.java

Controller에서 ResponseEntity 사용 시, ApiResponse을 사용한 방식으로 처리하기로 얘기되었으니, 리팩토링 때는 그 방식으로 적용해주시면 좋을 것 같아요~! 예시 코드는 #56 PR 내용 참고해주시면 됩니다~!

다음번 커밋에 리팩토링 하겠습니다.

linglong67 · 2024-01-17T03:26:00Z

module-domain/src/main/java/com/kernel360/washinfo/entity/WashInfo.java

+ @OneToOne
 @JoinColumn(name = "member_no", nullable = false)


@ OneToOne → 즉시로딩(EAGER), nullable = false → 내부조인인데 member 정보를 가져오지 못하는 경우가 생길 수도 있을 것 같아요..!
이건 근데 제가 확인해보진 못한 부분이라 추후 테스트코드를 통해 확인이 필요해보입니다!

원투원은 기본 속성이 즉시로딩인건가요? join명세에서 nullable false일 경우 무조건 데이터를 가지고 있어야 한다로 추측이 되는데 발생하는 부작용이 어떤 것인지 자세한 설명을 듣고싶습니다.

https://stackoverflow.com/questions/26601032/default-fetch-type-for-one-to-one-many-to-one-and-one-to-many-in-hibernate

OneToOne 의 기본 Default값은 Eager로 확인됩니다.

확인해보니 지연 로딩을 방지하기위해 조회시 무조건 다 가져오는 구조이네요 프론트분하고 의논해서 판단했을 때 부가정보 등록페이지 접근시 맴버와 부가정보를 같이 가져 올 필요가 없다면 연관관계 맵핑을 수정해서 별도로 조회하게끔 해도 된다 보여집니다.

gunsight1

번거로우시겠지만 엔티티 관련이 반복적으로 코드가 수정되어서 이부분은 결정 후 픽스하고 맞췄으면 좋겠습니다.

gunsight1 · 2024-01-17T07:26:35Z

module-domain/src/main/java/com/kernel360/brand/entity/Brand.java

 import jakarta.persistence.*;
 import lombok.Getter;
 import lombok.Setter;

+import java.util.List;
+
 @Getter
 @Setter


entity에서 setter는 꼭 없애주세요

반영했습니다. Setter는 JPA buddy 작성과정에서 생긴것으로 보입니다. 각 도메인에서 필요없으시면 각자 삭제해주시면 될거 같습니다.

gunsight1 · 2024-01-17T07:33:01Z

module-domain/src/main/java/com/kernel360/auth/repository/AuthRepository.java

 import org.springframework.data.jpa.repository.JpaRepository;

-public interface AuthRepository extends JpaRepository <Auth, Id> {
+public interface AuthRepository extends JpaRepository <Auth, Long> {


JpaRepository의 <> 매개변수중 Id는 기본적으로 Long 타입인데, Long으로 바꾸신 이유가 무엇인지 궁금합니다.
참고로 리포지터리에 Id라고 명시하면 엔티티의 @id 애노테이션이 부여된 필드를 바라보게 되어있는걸로 알고있는데, 제가 잘못알고있으면 교정 부탁드립니다.

오 하나 배웠네요~ 몰랐습니다

gunsight1 · 2024-01-17T07:37:26Z

module-domain/src/main/java/com/kernel360/washinfo/entity/WashInfo.java

+ @OneToOne
 @JoinColumn(name = "member_no", nullable = false)


원투원은 기본 속성이 즉시로딩인건가요? join명세에서 nullable false일 경우 무조건 데이터를 가지고 있어야 한다로 추측이 되는데 발생하는 부작용이 어떤 것인지 자세한 설명을 듣고싶습니다.

cr-gpt · 2024-01-17T09:40:47Z

module-domain/src/main/java/com/kernel360/brand/entity/Brand.java

@@ -18,6 +19,9 @@ public class Brand extends BaseEntity {
 @Column(name = "brand_name", nullable = false, length = Integer.MAX_VALUE)
 private String brandName;

+ @OneToMany(fetch = FetchType.LAZY, mappedBy = "brand")
+ private List<Product> productList;
+
 @Column(name = "description", length = Integer.MAX_VALUE)
 private String description;



Overall, the code appears to be fine with a few minor suggestions for improvement:

In the import statements, make sure to organize them and remove any unused imports.

Consider using specific annotations from JPA (such as @onetomany) instead of relying on vendor-specific annotations (e.g., jakarta.persistence.OneToMany).

Add the @OrderBy annotation to specify the ordering of the products in the productList field if necessary.

Ensure that the BaseEntity class is implemented correctly and provides the required functionality for your application.

Consider adding appropriate validation annotations or constraints to ensure data integrity (e.g., validating the length of the brand name).

Here's an updated version of the code with the suggested improvements:

package com.kernel360.brand.entity; import com.kernel360.base.BaseEntity; import com.kernel360.product.entity.Product; import javax.persistence.*; import java.util.List; @Entity @Table(name = "brand") public class Brand extends BaseEntity { @Column(name = "brand_name", nullable = false) private String brandName; @OneToMany(fetch = FetchType.LAZY, mappedBy = "brand") private List<Product> productList; @Column(name = "description", length = Integer.MAX_VALUE) private String description; // Constructors, getters, and setters }

Remember to review the changes carefully and adapt them to your specific requirements.

@OnetoOne 기본은 Eager 로 되어 있습니다. null 관련 사항은 jpa프로그래밍 책 298쪽 내용 참고로 첨부하겠습니다. 저도 정확히 이해를 못했는데, 혹시 이해가 되시면 저도 설명 좀 부탁드리겠습니다.

#59 (comment) 이쪽에 제 생각을 적어두었습니다

gunsight1 · 2024-01-18T01:10:51Z

module-domain/src/main/java/com/kernel360/washinfo/entity/WashInfo.java

+ @OneToOne
 @JoinColumn(name = "member_no", nullable = false)


확인해보니 지연 로딩을 방지하기위해 조회시 무조건 다 가져오는 구조이네요 프론트분하고 의논해서 판단했을 때 부가정보 등록페이지 접근시 맴버와 부가정보를 같이 가져 올 필요가 없다면 연관관계 맵핑을 수정해서 별도로 조회하게끔 해도 된다 보여집니다.

…onflict solving

feat::myPage,main codeSet 추가,

7da424b

Entity 연관관계 정보추가

HyunJunSon self-assigned this Jan 16, 2024

HyunJunSon requested review from linglong67, chan99k and gunsight1 as code owners January 16, 2024 10:20

cr-gpt bot reviewed Jan 16, 2024

View reviewed changes

chan99k requested changes Jan 17, 2024

View reviewed changes

chan99k self-requested a review January 17, 2024 01:28

chan99k approved these changes Jan 17, 2024

View reviewed changes

chan99k self-requested a review January 17, 2024 01:29

chan99k requested changes Jan 17, 2024

View reviewed changes

linglong67 approved these changes Jan 17, 2024

View reviewed changes

Merge branch 'Kernel360:develop' into feature/mypage_CRUD

f0303d7

gunsight1 requested changes Jan 17, 2024

View reviewed changes

chan99k approved these changes Jan 17, 2024

View reviewed changes

feat::mypage_CRUD review 반영

e477458

cr-gpt bot reviewed Jan 17, 2024

View reviewed changes

gunsight1 approved these changes Jan 18, 2024

View reviewed changes

HyunJunSon merged commit 3d939b4 into Kernel360:develop Jan 18, 2024
1 check passed

chan99k added a commit to chan99k/F1-WashPedia-BE that referenced this pull request Jan 18, 2024

Merge pull request Kernel360#59 from HyunJunSon/feature/mypage_CRUD c…

09a8401

…onflict solving

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

마이페이지, 메인페이지 CRUD 페이지 추가 #59

마이페이지, 메인페이지 CRUD 페이지 추가 #59

HyunJunSon commented Jan 16, 2024

cr-gpt bot Jan 16, 2024

cr-gpt bot Jan 16, 2024

chan99k left a comment

chan99k left a comment

chan99k Jan 17, 2024

HyunJunSon Jan 18, 2024

chan99k Jan 17, 2024

chan99k Jan 17, 2024

linglong67 left a comment •

edited

Loading

linglong67 Jan 17, 2024

HyunJunSon Jan 17, 2024

linglong67 Jan 17, 2024

gunsight1 Jan 17, 2024

HyunJunSon Jan 17, 2024

gunsight1 Jan 18, 2024

gunsight1 left a comment

gunsight1 Jan 17, 2024

HyunJunSon Jan 18, 2024

gunsight1 Jan 17, 2024

HyunJunSon Jan 17, 2024

gunsight1 Jan 17, 2024

cr-gpt bot Jan 17, 2024

HyunJunSon Jan 18, 2024

gunsight1 Jan 18, 2024

gunsight1 Jan 18, 2024

		String bannerImageUrl = "http://localhost:8080/bannersample.png";
		String suggestImageUrl = "http://localhost:8080/suggestsample.png";

마이페이지, 메인페이지 CRUD 페이지 추가 #59

마이페이지, 메인페이지 CRUD 페이지 추가 #59

Conversation

HyunJunSon commented Jan 16, 2024

💡 Motivation and Context

🔨 Modified

🌟 More

📋 커밋 전 체크리스트

🤟🏻 PR로 완료된 이슈

cr-gpt bot Jan 16, 2024

Choose a reason for hiding this comment

cr-gpt bot Jan 16, 2024

Choose a reason for hiding this comment

chan99k left a comment

Choose a reason for hiding this comment

chan99k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linglong67 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gunsight1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr-gpt bot Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linglong67 left a comment •

edited

Loading