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.

Sign up for GitHub

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

Jump to bottom

[Refactor] 코드 리뷰하면서 확인했던 코드 리팩토링 #294

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[Refactor] 코드 리뷰하면서 확인했던 코드 리팩토링 #294

wants to merge 1 commit into from

Conversation

limbaba1120
Copy link
Collaborator

  • UserService: login을 하면 null을 반환하는 코드 예외로 처리
  • UserViewController 에 있던 Admin 코드를 Admin Controller로 분리하고 로직 구현을 Service에서 함
  • P6SpySqlFormatter: formatter 를 사용하고 싶으면 application.yml 에 format_sql을 false 로 변경해야함.

@limbaba1120
refactor: 코드 리뷰하면서 확인했던 코드 리팩토링
acfcbf2 
- UserService: login을 하면 null을 반환하는 코드 예외로 처리
- UserViewController 에 있던 Admin 코드를 Admin Controller로 분리하고 로직 구현을 Service에서 함
- P6SpySqlFormatter: formatter 를 사용하고 싶으면 application.yml 에 format_sql을 false 로 변경해야함.
tigris24
tigris24 approved these changes Sep 6, 2024
View reviewed changes
Copy link
Collaborator

@tigris24 tigris24 left a comment

Choose a reason for hiding this comment

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

로그인과 어드민 관련 리팩토링 확인했습니다

backend/src/main/java/org/example/config/WebOAuthSecurityConfig.java
@@ -116,21 +116,28 @@ public OAuth2AuthorizationRequestBasedOnCookieRepository oAuth2AuthorizationRequ
@Bean
public CorsConfigurationSource corsConfigurationSource() {
CorsConfiguration corsConfiguration = new CorsConfiguration();
corsConfiguration.addAllowedOriginPattern("*");

corsConfiguration.addAllowedOriginPattern("https://www.lookus.shop");
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 되면 env에 있는 WEBSITE_DOMAIN이랑 API_DOMAIN을 사용하지 않으시는 것 같은데 comment에 추가해주셨으면 좋겠습니다

@limbaba1120 limbaba1120 self-assigned this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tigris24 tigris24 tigris24 approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@limbaba1120 limbaba1120

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@limbaba1120 @tigris24