-
Notifications
You must be signed in to change notification settings - Fork 45
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
실습 - 페이지 설정 #94
base: movingone
Are you sure you want to change the base?
실습 - 페이지 설정 #94
Conversation
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.
안녕하세요 동원님,
PR 메시지에 남겨주신 부분 + 추가 리뷰 몇 개 남겨봤어요.
계속해서 미션 진행해주시면서 리뷰도 함께 반영해보면 좋을 것 같아요!
이 외에도 궁금한 점이 있다면 언제든지 댓글 또는 DM 남겨주시면 됩니다.
한 번 파이팅해서 요구사항 구현해보도록 해요 🔥 🔥 🔥 🔥
|
||
import java.util.List; | ||
|
||
@org.springframework.stereotype.Controller |
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.
@org.springframework.stereotype.Controller | |
@Controller |
깔끔하게 어노테이션 이름만 보이게 해보는건 어떨까요?
import java.util.List; | ||
|
||
@org.springframework.stereotype.Controller | ||
public class Controller { |
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.
해당 컨트롤러가 어떤 도메인 담당하는지, 또는 어떤 api 를 담고 있는지 나타내는 이름을 지어보면 좋을 것 같아요.
@Autowired | ||
private JdbcTemplate jdbcTemplate; |
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.
빈을 주입해주는 방식은 여러 방법이 있어요.
그 중 생성자를 활용해서 넣어주는 방식도 있는데요,
각 방식의 장단점을 알아보고 가장 선호하는 방식으로 변경해보면 어떨까요?
@@ -0,0 +1,3 @@ | |||
spring.h2.console.enabled=true | |||
spring.h2.console.path=/h2-console | |||
spring.datasource.url=jdbc:h2:mem:database |
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.
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.
Git 에서는 마지막줄이 변경되지 않으면 내용이 변경된 것으로 인식할 수도 있어서 빈줄을 추가하는게 맞을까요?
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.
이 글을 한 번 참고해보시죠! 영어긴한데... 번역을 사용해봐도 좋을 것 같습니다 :)
src/main/java/domain/Reserve.java
Outdated
@@ -0,0 +1,35 @@ | |||
package domain; | |||
|
|||
public class Reserve { |
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.
도메인 이름은 명사로 지어보는게 어떨까요?
'예약' 은 대중적으로 Reservation 이라는 이름으로 많이 사용하는 것 같아요.
} | ||
@PostMapping("/reservations") |
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.
메서드 간 공백은 잊지 말고 넣어주세요!
import java.util.List; | ||
|
||
@org.springframework.stereotype.Controller | ||
public class Controller { |
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.
막히는 부분이 생겨서 리뷰요청 보내주신 점 👍
다음부터는 막히는 부분이 있어도 컴파일이 되는 상태로 만들어서 PR 을 남겨보죠!
src/main/java/domain/Reserve.java
Outdated
public String getTime() { | ||
return time; | ||
} |
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.
public String getTime() { | |
return time; | |
} | |
public Time getTime() { | |
return time; | |
} |
@@ -8,5 +8,4 @@ public class RoomescapeApplication { | |||
public static void main(String[] args) { |
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.
controller 와 domain 패키지를 roomescape 패키지 안으로 옮겨보는건 어떨까요?
이 구조라면 @SpringBootApplication
어노테이션이 제 기능을 발휘하지 못할 수도 있을 것 같아요!
PR 메시지에 남겨주신 문제도 이 문제이지 않을까 싶네요 😄
src/main/resources/schema.sql
Outdated
create table reservation | ||
( | ||
id BIGINT NOT NULL AUTO_INCREMENT, | ||
name VARCHAR(255) NOT NULL, | ||
date VARCHAR(255) NOT NULL, | ||
time_id BIGINT, | ||
PRIMARY KEY (id), | ||
FOREIGN KEY (time_id) REFERENCES reservation_time (id) | ||
); | ||
|
||
create table reservation_time | ||
( | ||
id BIGINT NOT NULL AUTO_INCREMENT, | ||
start_at VARCHAR(255) NOT NULL, | ||
PRIMARY KEY (id) | ||
); |
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.
애플리케이션을 실행할 때 오류가 발생하진 않으셨나요?
reservation
테이블이 reservation_time
테이블을 참조하고 있는데 reservation_time
테이블을 더 늦게 만들어주고 있어요.
이 순서를 변경해야할 것 같아요!
return jdbcTemplate.query(sql, | ||
(rs, rowNum) -> { | ||
Reserve reserve = new Reserve( | ||
rs.getLong("id"), | ||
rs.getString("name"), | ||
rs.getString("date"), | ||
rs.getString("time") | ||
); | ||
return reserve; | ||
}); |
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.
예약 조회 쿼리 수정
조회 시 ReservationTime 정보도 함께 조회하기 위해 아래와 같이 쿼리를 수정해주세요.
SELECT
r.id as reservation_id,
r.name as reservation_name,
r.date as reservation_date,
t.id as time_id,
t.start_at as time_start_at
FROM reservation as r
inner join reservation_time as t
on r.time_id = t.id
기존에 Reserve 클래스는 Long, String, String, String 으로 이루어져 있었지만 time 부분을 String 에서 -> Time 클래스로 바꾸면서 내부의 rs.getString("time") 값 처리와 selet 결과를 어떤식으로 해야 할 지 감이 잡히지 않습니다
reserve라는 객체안에 하나씩 값을 매핑한다 생각했었는데 inner join을 하면서 어떤식으로 값을 추가하면 될지 모르겠습니다
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.
오 이 리뷰는 다른 분이 어딘가에서 남겼던 리뷰일까요?
inner join 을 한다고 하더라도, 결국 가져오는 정보는 SELECT 와 FROM 사이에 있는 것들입니다.
실제로 저 쿼리를 한 번 DB 에 실행해서 어떤 결과가 나오는지 확인해봐도 좋을 것 같네요.
저렇게 해서 가져오는 다른 값들을 resultSet.getString("name")
처럼 가져오듯,
시간 역시 말해주신대로 String 으로 가져와서 Time 클래스로 바꿔봐도 전혀 이상할게 없습니다.
@PostMapping("/reservations") | ||
public void addReserve(@RequestBody Reserve reserve) { | ||
String sql = "insert into reservation(name, date, time) values (?, ?, ?)"; | ||
jdbcTemplate.update(sql, reserve.getName(), reserve.getDate(), reserve.getTime()); | ||
} |
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.
예약 추가 쿼리 수정
예약 추가 시, 시간을 문자열(ex. "10:00") 형태로 입력하던 부분을 ReservationTime의 식별자(ex. 1)로 수정해주세요.
말 자체가 잘 이해가 되어지지 않습니다 "ReservationTime의 식별자(ex. 1)로 수정해주세요."
String 타입으로 받던것이 수정된다는 말일까요?
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.
@@ -0,0 +1,3 @@ | |||
spring.h2.console.enabled=true | |||
spring.h2.console.path=/h2-console | |||
spring.datasource.url=jdbc:h2:mem:database |
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.
Git 에서는 마지막줄이 변경되지 않으면 내용이 변경된 것으로 인식할 수도 있어서 빈줄을 추가하는게 맞을까요?
@PostMapping("/times") | ||
public ResponseEntity<String> startTime(@RequestBody Time time) { | ||
public ResponseEntity<String> addTime(@RequestBody Time time) { | ||
String sql = "insert into reservation_time(start_at) values ?"; | ||
jdbcTemplate.update(sql, time.getStartAt()); | ||
return ResponseEntity.ok().body("success"); | ||
} |
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.
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.
확인버튼이 눌리긴하지만,
서버에 오류가 있어서 응답이 제대로 오지 않고 있습니다.
현재 작성하신 코드를 보면,
time_id
가 들어가야할 자리에 reservation
의 id
를 넣어주고 있습니다.
이를 time 관련 id 로 먼저 수정해야할 것 같아요.
그리고 time_id 를 클라이언트가 서버쪽에 전달하기 위해서는 클라이언트가
현재 예약 가능한 시간이 뭐가 있는지를 알아야할 것 같아요.
만들어주신 각 api 가 잘 작동하는지 확인해보고 의도대로 페이지가 보이게 수정해보는건 어떨까요?
그러면 예약이 정상적으로 등록될 것 같습니다!!!
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.
추가로 남겨주신 질문에 대한 답변 :
url 에 대한 이야기는 나중에 정말 다양한 주제로 나눠볼 수 있는데,
그 중 /time
이 좋은지, 아니면 /times
가 좋은지도 포함되는 것 같아요 😂
일단 지금은 프론트엔드 관련 url 은 단수형, 그리고 백엔드 관련 url 은 복수형으로 두면 어떨까 싶습니다.
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.
안녕하세요!!!
오랜만이네요 :)
우선 아직 작동되지 않는 기능들이 있어서 머지하지는 않고 다시 request change 드립니다.
만약 PR 에 적힌 내용들이 너무 반영하기 어렵다 싶으면
저는 reservation time 의 id 를 받는 부분은 일단 제외해두고 만들어봐도 괜찮을 것 같아요.
그리고 기능이 정상 작동하는 것에 먼저 중점을 두시면 좋을 것 같은데,
하다가 막히는게 있다면 언제든지 DM 주세요.
같이 문제를 해결해보면 좋을 것 같아요.
파이팅입니다 🔥 🔥 🔥
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.
아직까지 실행결과 볼 수 없었음
@PostMapping("/times") | ||
public ResponseEntity<String> addTime(@RequestBody Time time) { | ||
String sql = "insert into reservation_time(start_at) values ?"; | ||
public ResponseEntity<Time> addTime(@RequestBody Time time) { | ||
String sql = "insert into reservation_time (start_at) values ?"; | ||
KeyHolder keyHolder = new GeneratedKeyHolder(); | ||
jdbcTemplate.update(connection -> { | ||
PreparedStatement ps = connection.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS); | ||
ps.setString(1, time.getStartAt()); | ||
return ps; | ||
}, keyHolder); | ||
|
||
return ResponseEntity.ok().body("success"); | ||
Long id = keyHolder.getKey().longValue(); | ||
time.setId(id); | ||
|
||
return ResponseEntity.ok().body(time); | ||
} |
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.
시간을 추가함에 따라서 화면에서도 추가된 시간이 보여야 한다 생각했는데 차이가 없습니다
시간을 추가한 순간 PostMapping
과 GetMapping
이 동시에 일어나는 개념일까요?
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.
@PostMapping("/reservation") | ||
public void addReserve(@RequestBody Reservation reserve, Time time) { | ||
|
||
String sql = "insert into reservation(name, date, time_id) values (?, ?, ?)"; | ||
jdbcTemplate.update(sql, reserve.getName(), reserve.getDate(), reserve.getId()); | ||
jdbcTemplate.update(sql, reserve.getName(), reserve.getDate(), time.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.
reserve.getId()
를 @RequestBody Reservation reserve, Time time
Time을 추가로 받으면서 -> time.getId() 로 바꾸면 되는게 맞을까요? 확인하려 했지만 그전에 Time값 저장에서 문제가 생겨서 확인할 수 없었습니다
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.
지금은 아직 1단계니까, 1단계 요구사항을 먼저 만족하는 것에 집중해볼까요?
현재 여러 오류 및 미구현 기능이 겹쳐있어서 개선하는데 더 큰 어려움을 느끼실 것 같아요.
우선 다음과 같은 흐름으로 프로젝트를 수정해보면 좋을 것 같네요:
- templates/admin/reservation-legacy.html 가 잘 보이도록 수정
- 해당 PR merge (또는 이어서 다음 단계 수행)
- (다음 단계) 예약 추가, 예약 목록 보기, 예약 삭제 api 구현
- (다음 단계) 시간 추가, 시간 목록 보기, 시간 삭제 api 구현
import java.util.List; | ||
|
||
@Controller | ||
public class ReserveController { |
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.
Reservation
이라는 이름으로 도메인을 바꿔주셨으니,
관련된 곳들도 이름을 바꿔볼까요?
(해당 class 이름 말고도 다른 reserve
들을 확인해주세요!)
}); | ||
} | ||
|
||
@GetMapping("/reservation/{id}") |
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.
여기는 복수형으로 두시면 좋을 것 같아요!
해당 리뷰에서 말한 프론트엔드 / 백엔드는 각각
프론트엔드 : 웹 페이지 (파일)
백엔드 : api (Spring Boot 를 활용하여 만드는 것)
이었습니다 :)
@PostMapping("/times") | ||
public ResponseEntity<String> addTime(@RequestBody Time time) { | ||
String sql = "insert into reservation_time(start_at) values ?"; | ||
public ResponseEntity<Time> addTime(@RequestBody Time time) { | ||
String sql = "insert into reservation_time (start_at) values ?"; | ||
KeyHolder keyHolder = new GeneratedKeyHolder(); | ||
jdbcTemplate.update(connection -> { | ||
PreparedStatement ps = connection.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS); | ||
ps.setString(1, time.getStartAt()); | ||
return ps; | ||
}, keyHolder); | ||
|
||
return ResponseEntity.ok().body("success"); | ||
Long id = keyHolder.getKey().longValue(); | ||
time.setId(id); | ||
|
||
return ResponseEntity.ok().body(time); | ||
} |
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.
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.
안녕하세요 동건님!
코멘트로도 남긴 내용인데,
지금 한 번에 여러 단계를 함께 작업하시고 있고,
다양한 오류 & 미구현 기능이 겹쳐서
개선 작업을 하시는데 어려움을 겪고 계신 것 같아요.
우선 '페이지 설정' 관련 요구사항을 먼저 처리하고
그 다음 예약 기능, 시간 기능을 하나씩 처리해나가면 좋을 것 같습니다.
우선 페이지부터 잘 나오게 해보죠!!!!
이렇게하면 http://localhost:8080//admin/reservation 에서 방탈출 예약 페이지로 화면이 넘어갈 줄 알았는데 안넘어갔습니다. 아무리 생각해도 이유를 모르겠어서 리뷰 요청합니다