경북대 BE_김영인 6주차 과제(1단계)#143
Open
Youngini wants to merge 28 commits intokakao-tech-campus-2nd-step2:younginifrom
Open
경북대 BE_김영인 6주차 과제(1단계)#143Youngini wants to merge 28 commits intokakao-tech-campus-2nd-step2:younginifrom
Youngini wants to merge 28 commits intokakao-tech-campus-2nd-step2:younginifrom
Conversation
gongdongho12
requested changes
Aug 3, 2024
gongdongho12
left a comment
gongdongho12
left a comment
There was a problem hiding this comment.
1단계 작업 고생하셨습니다
그리구 비지니스 로직 중에 중요한 부분인 KakaoLogin에 대한 테스트가 없는데요
중요한 요소들은 테스트 객체로 추가되면 좋을 것 같습니다!
minor) 사용 안하는 API는 추후 git에서 가져오면 되는데 그냥 삭제하는 편이 깔끔하지 않을까요?
Comment on lines
+8
to
+46
| ('Coffee Maker', 49, 'http://example.com/coffeemaker.jpg', 3) [42122-224] | ||
| at org.h2.message.DbException.getJdbcSQLException(DbException.java:514) | ||
| at org.h2.message.DbException.getJdbcSQLException(DbException.java:489) | ||
| at org.h2.message.DbException.get(DbException.java:223) | ||
| at org.h2.message.DbException.get(DbException.java:199) | ||
| at org.h2.table.Table.getColumn(Table.java:759) | ||
| at org.h2.command.Parser.parseColumn(Parser.java:1190) | ||
| at org.h2.command.Parser.parseColumnList(Parser.java:1175) | ||
| at org.h2.command.Parser.parseInsert(Parser.java:1549) | ||
| at org.h2.command.Parser.parsePrepared(Parser.java:719) | ||
| at org.h2.command.Parser.parse(Parser.java:592) | ||
| at org.h2.command.Parser.parse(Parser.java:564) | ||
| at org.h2.command.Parser.prepareCommand(Parser.java:483) | ||
| at org.h2.engine.SessionLocal.prepareLocal(SessionLocal.java:639) | ||
| at org.h2.engine.SessionLocal.prepareCommand(SessionLocal.java:559) | ||
| at org.h2.jdbc.JdbcConnection.prepareCommand(JdbcConnection.java:1166) | ||
| at org.h2.jdbc.JdbcStatement.executeInternal(JdbcStatement.java:245) | ||
| at org.h2.jdbc.JdbcStatement.execute(JdbcStatement.java:231) | ||
| at org.h2.server.web.WebApp.getResult(WebApp.java:1345) | ||
| at org.h2.server.web.WebApp.query(WebApp.java:1143) | ||
| at org.h2.server.web.WebApp.query(WebApp.java:1119) | ||
| at org.h2.server.web.WebApp.process(WebApp.java:245) | ||
| at org.h2.server.web.WebApp.processRequest(WebApp.java:177) | ||
| at org.h2.server.web.JakartaWebServlet.doGet(JakartaWebServlet.java:129) | ||
| at org.h2.server.web.JakartaWebServlet.doPost(JakartaWebServlet.java:166) | ||
| at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:590) | ||
| at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:658) | ||
| at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:195) | ||
| at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:140) | ||
| at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:51) | ||
| at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:164) | ||
| at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:140) | ||
| at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) | ||
| at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) | ||
| at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:164) | ||
| at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:140) | ||
| at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) | ||
| at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) | ||
| at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:164) |
There was a problem hiding this comment.
이런 trace는 디버깅 하실떄 사용하는 값이라 PR에 첨부하지 않도록 ignore 처리하는게 어떨까요?
Comment on lines
+28
to
+29
| public ResponseEntity<List<CategoryListDto>> getAllCategories(@RequestParam(defaultValue = "0") int page, | ||
| @RequestParam(defaultValue = "10") int size) { |
There was a problem hiding this comment.
Comment on lines
+35
to
+57
| /* @GetMapping("/{id}") | ||
| public ResponseEntity<Category> getCategoryById(@PathVariable Long id) { | ||
| Category category = categoryService.findById(id); | ||
| return ResponseEntity.ok(category); | ||
| } | ||
|
|
||
| @PostMapping | ||
| public ResponseEntity<Category> createCategory(@RequestParam String name) { | ||
| Category category = categoryService.addCategory(name); | ||
| return ResponseEntity.ok(category); | ||
| } | ||
|
|
||
| @PutMapping("/{id}") | ||
| public ResponseEntity<Category> updateCategory(@PathVariable Long id, @RequestBody UpdateCategoryDto updateCategoryDto) { | ||
| Category updatedCategory = categoryService.updateCategory(id, updateCategoryDto); | ||
| return ResponseEntity.ok(updatedCategory); | ||
| } | ||
|
|
||
| @DeleteMapping("/{id}") | ||
| public ResponseEntity<Void> deleteCategory(@PathVariable Long id) { | ||
| categoryService.deleteCategory(id); | ||
| return ResponseEntity.noContent().build(); | ||
| }*/ |
There was a problem hiding this comment.
저는 개인적으로 미사용 API는 다시 살리는게 어려운 건 아니구 git에 전부 기록되어 있어서 삭제하는 방향을 선호하는데요
클린 코드 관점에서 지워보는게 어떠실까요?
Comment on lines
+36
to
+44
| public void subtract(Integer amount) { | ||
| if (amount <= 0) { | ||
| throw new IllegalArgumentException("감소할 수량은 0보다 커야 합니다."); | ||
| } | ||
| if (this.quantity < amount) { | ||
| throw new IllegalArgumentException("재고 수량이 부족합니다."); | ||
| } | ||
| this.quantity -= amount; | ||
| } |
There was a problem hiding this comment.
이미 quantity validation이 걸려있는데 sutract 메서드에서 처리하지 않더라도 처리할 수 있지 않을까요?
Comment on lines
+3
to
+11
| public class ErrorMessage { | ||
| public static final String PRODUCT_MISSING_FIELDS = "상품의 이름, 가격, 설명을 모두 입력해야합니다."; | ||
| public static final String PRODUCT_NOT_FOUND = "일치하는 상품이 없습니다."; | ||
| public static final String INTERNAL_SERVER_ERROR = "서버에 문제가 발생했습니다."; | ||
| public static final String MEMBER_EMAIL_ALREADY_EXISTS = "이메일이 이미 존재합니다."; | ||
| public static final String MEMBER_NOT_FOUND = "일치하는 회원이 없습니다."; | ||
| public static final String INVALID_LOGIN_CREDENTIALS = "일치하는 이메일이 없거나 비밀번호가 틀렸습니다."; | ||
| public static final String EMAIL_NOT_FOUND = "일치하는 이메일이 없습니다."; | ||
| } |
Comment on lines
+22
to
+23
| @Service | ||
| public class KakaoTokenService { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
팀원들과 작성한 명세서를 바탕으로 API를 수정했습니다.
프론트와 연결을 위해 사용하지 않는 api들은 주석 처리를 할 예정입니다.