-
Notifications
You must be signed in to change notification settings - Fork 300
๐ 1๋จ๊ณ - ๋ ๊ฑฐ์ ์ฝ๋ ๋ฆฌํฉํฐ๋ง #790
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
Changes from all commits
c4960b6
a117751
3cdb2ed
9c9d031
625a895
188c9d7
8312ece
f1b3547
54e1f6d
c4db9bd
400b772
73effd2
797023c
660f24f
22a45a9
aa75be9
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,37 @@ | ||
| # ๐ 1๋จ๊ณ - ๋ ๊ฑฐ์ ์ฝ๋ ๋ฆฌํฉํฐ๋ง | ||
| *** | ||
|
|
||
| ## ์ฝ๋ ๋ฆฌ๋ทฐ | ||
| > PR ๋งํฌ : [#790](https://github.com/next-step/java-lms/pull/790) | ||
|
|
||
| ## ๋์ ํ์ต ๋ชฉํ | ||
|
|
||
| ### 1. TDD ์ฌ์ดํด์ ์์ํ๋ฉฐ ๊ธฐ๋ฅ์ ๊ตฌํํ๋ค. | ||
| - `์คํจ โ ์ฑ๊ณต โ ๋ฆฌํฉํฐ๋ง` ๊ณผ์ ์ผ๋ก ์์ ํ๋ค. | ||
| - ์ต๊ด์ฒ๋ผ ์ด ๊ณผ์ ์ ์ง๋์น๋ฉด, ๋ค์ ๋์์ ์ฌ์ดํด์ ๋ฐ๋ณตํ๋ค. | ||
| - TDD ์ฌ์ดํด ์ต๊ด์ ๋ง๋ค์. | ||
|
|
||
| ### 2. ๋๋ฉ์ธ์ ํนํ๋ ์ด๋ฆ์ ์ง๋๋ค. | ||
| - AI์๊ฒ ์ถ์ฒ ๋ฐ๋ ์ต๊ด์ ๋ง๋ค์. | ||
|
|
||
| ### 3. ๋ฐ๋ณต๋ ํผ๋๋ฐฑ์ ํผํ์. | ||
| - ๊ฐ์ ํผ๋๋ฐฑ์ ๋ฐ๋ณตํ์ง ์๋๋ก, ์ฒดํฌ๋ฆฌ์คํธ๋ฅผ ๋ง๋ค์ด ์ ๊ฒํ์. | ||
|
|
||
| ## ์ง๋ฌธ ์ญ์ ํ๊ธฐ ์๊ตฌ์ฌํญ | ||
|
|
||
| - [x] ์ง๋ฌธ ๋ฐ์ดํฐ๋ฅผ ์์ ํ ์ญ์ ํ๋ ๊ฒ์ด ์๋๋ผ ๋ฐ์ดํฐ์ ์ํ๋ฅผ ์ญ์ ์ํ(deleted - boolean type)๋ก ๋ณ๊ฒฝํ๋ค. | ||
| - [x] ๋ก๊ทธ์ธ ์ฌ์ฉ์์ ์ง๋ฌธํ ์ฌ๋์ด ๊ฐ์ ๊ฒฝ์ฐ ์ญ์ ๊ฐ๋ฅํ๋ค. | ||
| - [x] ๋ต๋ณ์ด ์๋ ๊ฒฝ์ฐ ์ญ์ ๊ฐ ๊ฐ๋ฅํ๋ค. | ||
| - [x] ์ง๋ฌธ์์ ๋ต๋ณ ๊ธ์ ๋ชจ๋ ๋ต๋ณ์๊ฐ ๊ฐ์ ๊ฒฝ์ฐ ์ญ์ ๊ฐ ๊ฐ๋ฅํ๋ค. | ||
| - [x] ์ง๋ฌธ์ ์ญ์ ํ ๋ ๋ต๋ณ ๋ํ ์ญ์ ํด์ผ ํ๋ฉฐ, ๋ต๋ณ์ ์ญ์ ๋ํ ์ญ์ ์ํ(deleted)๋ฅผ ๋ณ๊ฒฝํ๋ค. | ||
| - [x] ์ง๋ฌธ์์ ๋ต๋ณ์๊ฐ ๋ค๋ฅธ ๊ฒฝ์ฐ ๋ต๋ณ์ ์ญ์ ํ ์ ์๋ค. | ||
| - [x] ์ง๋ฌธ๊ณผ ๋ต๋ณ ์ญ์ ์ด๋ ฅ์ ๋ํ ์ ๋ณด๋ฅผ DeleteHistory๋ฅผ ํ์ฉํด ๋จ๊ธด๋ค. | ||
|
|
||
| ## ๋ฆฌํฉํฐ๋ง ์๊ตฌ์ฌํญ | ||
|
|
||
| - [x] nextstep.qna.service.QnaService์ deleteQuestion()๋ ์์ ์ง๋ฌธ ์ญ์ ๊ธฐ๋ฅ์ ๊ตฌํํ ์ฝ๋์ด๋ค. | ||
| ์ด ๋ฉ์๋๋ ๋จ์ ํ ์คํธํ๊ธฐ ์ด๋ ค์ด ์ฝ๋์ ๋จ์ ํ ์คํธ ๊ฐ๋ฅํ ์ฝ๋๊ฐ ์์ฌ ์๋ค. | ||
| - [x] QnaService์ deleteQuestion() ๋ฉ์๋์ ๋จ์ ํ ์คํธ ๊ฐ๋ฅํ ์ฝ๋(ํต์ฌ ๋น์ง๋์ค ๋ก์ง)๋ฅผ ๋๋ฉ์ธ ๋ชจ๋ธ ๊ฐ์ฒด์ ๊ตฌํํ๋ค. | ||
| - [x] QnaService์ ๋น์ง๋์ค ๋ก์ง์ ๋๋ฉ์ธ ๋ชจ๋ธ๋ก ์ด๋ํ๋ ๋ฆฌํฉํฐ๋ง์ ์งํํ ๋ TDD๋ก ๊ตฌํํ๋ค. | ||
| - [x] QnaService์ deleteQuestion() ๋ฉ์๋์ ๋ํ ๋จ์ ํ ์คํธ๋ src/test/java ํด๋ nextstep.qna.service.QnaServiceTest์ด๋ค. | ||
| ๋๋ฉ์ธ ๋ชจ๋ธ๋ก ๋ก์ง์ ์ด๋ํ ํ์๋ QnaServiceTest์ ๋ชจ๋ ํ ์คํธ๋ ํต๊ณผํด์ผ ํ๋ค. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # โ๏ธ PR ์ ํ์ธํด์ผ๋ ๋ชฉ๋ก | ||
|
Contributor
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. ๐ |
||
| *** | ||
|
|
||
| - ๊ฐ์ฒด์งํฅ ์ํ์ฒด์กฐ ์์น์ ์ค์ํ์๋๊ฐ ? | ||
| - TDD ์ฌ์ดํด๋ก ๊ตฌํํ์๋๊ฐ ? | ||
| - TDD ์ฌ์ดํด ๋จ์๋ก ์ปค๋ฐํ์๋๊ฐ ? | ||
| - ๊ฐ์ฒด์๊ฒ ์ํ๋ฅผ ๋ ธ์ถ์ํค์ง ์๊ณ ์ฑ ์์ ๋งก๊ฒผ๋๊ฐ ? | ||
| - ํ ์ฝ์ผ๋ ๊ฐ์ฒด์ ์ฑ ์๊ณผ ๊ฒฐ๊ณผ๋ง ๊ฒ์ฆํ๋๊ฐ ? | ||
| - ๋ถ๋ณ์ฑ์ ์ต๋ํ ์ ์งํ๋๊ฐ ? | ||
| - ๋๋ฉ์ธ ์ฉ์ด๋ก ๋ค์ด๋ฐ ํ๋๊ฐ ? | ||
| - ํ๋ ฅ ๊ตฌ์กฐ๊ฐ ์์ฐ์ค๋ฝ๊ฒ ์ฝํ๋๊ฐ ? | ||
| - ๊ฐ์ ํผ๋๋ฐฑ์ ๋ฐ๋ณตํ์ง๋ ์์๋๊ฐ ? | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,40 +1,37 @@ | ||||||||||
| package nextstep.qna.domain; | ||||||||||
|
|
||||||||||
| import nextstep.qna.CannotDeleteException; | ||||||||||
| import nextstep.qna.NotFoundException; | ||||||||||
| import nextstep.qna.UnAuthorizedException; | ||||||||||
| import nextstep.users.domain.NsUser; | ||||||||||
|
|
||||||||||
| import java.time.LocalDateTime; | ||||||||||
|
|
||||||||||
| public class Answer { | ||||||||||
| public class Answer extends SoftDeletableModel { | ||||||||||
| private Long id; | ||||||||||
|
|
||||||||||
| private NsUser writer; | ||||||||||
|
|
||||||||||
| private Question question; | ||||||||||
|
|
||||||||||
| private String contents; | ||||||||||
|
|
||||||||||
| private boolean deleted = false; | ||||||||||
|
|
||||||||||
| private LocalDateTime createdDate = LocalDateTime.now(); | ||||||||||
|
|
||||||||||
| private LocalDateTime updatedDate; | ||||||||||
| private QuestionBody contents; | ||||||||||
|
|
||||||||||
| public Answer() { | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Answer(NsUser writer, Question question, String contents) { | ||||||||||
| this(null, writer, question, contents); | ||||||||||
| this(null, writer, question, new QuestionBody(contents)); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Answer(Long id, NsUser writer, Question question, String contents) { | ||||||||||
| this(id, writer, question, new QuestionBody(contents)); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Answer(Long id, NsUser writer, Question question, QuestionBody contents) { | ||||||||||
| this.id = id; | ||||||||||
| if(writer == null) { | ||||||||||
| if (writer == null) { | ||||||||||
| throw new UnAuthorizedException(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if(question == null) { | ||||||||||
| if (question == null) { | ||||||||||
| throw new NotFoundException(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
@@ -47,13 +44,12 @@ public Long getId() { | |||||||||
| return id; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Answer setDeleted(boolean deleted) { | ||||||||||
| this.deleted = deleted; | ||||||||||
| return this; | ||||||||||
| private void updateDeleted() { | ||||||||||
|
Contributor
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. ๊ตณ์ด ์ด ๋ฉ์๋ ํ์์์ง ์์๊น? |
||||||||||
| deleted(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public boolean isDeleted() { | ||||||||||
| return deleted; | ||||||||||
| return getDeleted(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public boolean isOwner(NsUser writer) { | ||||||||||
|
|
@@ -64,14 +60,18 @@ public NsUser getWriter() { | |||||||||
| return writer; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public String getContents() { | ||||||||||
| return contents; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void toQuestion(Question question) { | ||||||||||
| this.question = question; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException { | ||||||||||
|
Contributor
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.
Suggested change
์ญ์ ๊ฐ๋ฅ ์ฌ๋ถ๋ฅผ ํ์ธํ๋ ๊ฒ์ด ํต์ฌ์ด ์๋๋ผ ์ญ์ ๊ฐ ํต์ฌ์ด์ง ์์๊น? |
||||||||||
| if (!this.isOwner(loginUser)) { | ||||||||||
| throw new CannotDeleteException("๋ค๋ฅธ ์ฌ๋์ด ์ด ๋ต๋ณ์ด ์์ด ์ญ์ ํ ์ ์์ต๋๋ค."); | ||||||||||
| } | ||||||||||
| updateDeleted(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
Contributor
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. ํ์ฌ๋ valid์ updateDeleted๊ฐ ๋ถ๋ฆฌ๋์ด ๊ตฌํ๋์ด ์์.
Suggested change
Contributor
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. ์ด ํผ๋๋ฐฑ ๋ฐ์ํ๋์ง ํ์ธ ํ์ |
||||||||||
| @Override | ||||||||||
| public String toString() { | ||||||||||
| return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]"; | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||
| package nextstep.qna.domain; | ||||||
|
|
||||||
| import nextstep.qna.CannotDeleteException; | ||||||
| import nextstep.users.domain.NsUser; | ||||||
|
|
||||||
| import java.time.LocalDateTime; | ||||||
| import java.util.ArrayList; | ||||||
| import java.util.List; | ||||||
|
|
||||||
| public class Answers { | ||||||
|
Contributor
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. ์ผ๊ธ ์ฝ๋ ์ ์ ์ฉ ๐ |
||||||
|
|
||||||
| private List<Answer> answers = new ArrayList<>(); | ||||||
|
|
||||||
| public void add(Answer answer) { | ||||||
| this.answers.add(answer); | ||||||
| } | ||||||
|
|
||||||
| public List<Answer> getAnswers() { | ||||||
| return answers; | ||||||
| } | ||||||
|
|
||||||
| public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException { | ||||||
|
Contributor
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.
Suggested change
์ญ์ ๊ฐ๋ฅ ์ฌ๋ถ๋ฅผ ํ์ธํ๋ ๊ฒ์ด ํต์ฌ์ด ์๋๋ผ ์ญ์ ๊ฐ ํต์ฌ์ด์ง ์์๊น? |
||||||
| for (Answer answer : this.answers) { | ||||||
| answer.validateAnswerOwner(loginUser); | ||||||
|
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public List<DeleteHistory> addDeleteAnswerHistory() { | ||||||
| List<DeleteHistory> deleteHistories = new ArrayList<>(); | ||||||
| for (Answer answer : this.answers) { | ||||||
| deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now())); | ||||||
| } | ||||||
| return deleteHistories; | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,22 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| import nextstep.qna.CannotDeleteException; | ||
| import nextstep.users.domain.NsUser; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Question { | ||
| public class Question extends SoftDeletableModel { | ||
| private Long id; | ||
|
|
||
| private String title; | ||
| private QuestionBody title; | ||
|
|
||
| private String contents; | ||
| private QuestionBody contents; | ||
|
|
||
| private NsUser writer; | ||
|
|
||
| private List<Answer> answers = new ArrayList<>(); | ||
|
|
||
| private boolean deleted = false; | ||
|
|
||
| private LocalDateTime createdDate = LocalDateTime.now(); | ||
|
|
||
| private LocalDateTime updatedDate; | ||
| private Answers answers = new Answers(); | ||
|
|
||
| public Question() { | ||
| } | ||
|
|
@@ -31,6 +26,10 @@ public Question(NsUser writer, String title, String contents) { | |
| } | ||
|
|
||
| public Question(Long id, NsUser writer, String title, String contents) { | ||
| this(id, writer, new QuestionBody(title), new QuestionBody(contents)); | ||
| } | ||
|
|
||
| public Question(Long id, NsUser writer, QuestionBody title, QuestionBody contents) { | ||
| this.id = id; | ||
| this.writer = writer; | ||
| this.title = title; | ||
|
|
@@ -41,52 +40,56 @@ public Long getId() { | |
| return id; | ||
| } | ||
|
|
||
| public String getTitle() { | ||
| return title; | ||
| public NsUser getWriter() { | ||
| return writer; | ||
| } | ||
|
|
||
| public Question setTitle(String title) { | ||
| this.title = title; | ||
| return this; | ||
| public void addAnswer(Answer answer) { | ||
| answer.toQuestion(this); | ||
| this.answers.add(answer); | ||
| } | ||
|
|
||
| public String getContents() { | ||
| return contents; | ||
| public boolean isOwner(NsUser loginUser) { | ||
| return writer.equals(loginUser); | ||
| } | ||
|
|
||
| public Question setContents(String contents) { | ||
| this.contents = contents; | ||
| return this; | ||
| public boolean isDeleted() { | ||
| return getDeleted(); | ||
| } | ||
|
|
||
| public NsUser getWriter() { | ||
| return writer; | ||
| private void validateOwner(NsUser loginUser) throws CannotDeleteException { | ||
| if (!this.isOwner(loginUser)) { | ||
| throw new CannotDeleteException("์ง๋ฌธ์ ์ญ์ ํ ๊ถํ์ด ์์ต๋๋ค."); | ||
| } | ||
| updateDeleted(); | ||
| } | ||
|
|
||
| public void addAnswer(Answer answer) { | ||
| answer.toQuestion(this); | ||
| answers.add(answer); | ||
| public void delete(NsUser loginUser) throws CannotDeleteException { | ||
| validateOwner(loginUser); | ||
| answers.validateAnswerOwner(loginUser); | ||
| } | ||
|
|
||
| public boolean isOwner(NsUser loginUser) { | ||
| return writer.equals(loginUser); | ||
| } | ||
| public List<DeleteHistory> toDeleteHistories(){ | ||
| List<DeleteHistory> deleteHistories = new ArrayList<>(); | ||
| deleteHistories.add(new DeleteHistory(ContentType.QUESTION, this.id, this.writer, LocalDateTime.now())); | ||
|
|
||
| public Question setDeleted(boolean deleted) { | ||
| this.deleted = deleted; | ||
| return this; | ||
| List<DeleteHistory> answerDeleteHistories = addDeleteAnswerHistory(); | ||
| deleteHistories.addAll(answerDeleteHistories); | ||
|
|
||
| return deleteHistories; | ||
| } | ||
|
|
||
|
Contributor
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. ํ์ฌ๋ delete()์์ ์ํ ๋ณ๊ฒฝ๊ณผ List ์์ฑ ํ ๋ฐํํ๋ ํ์ฌ ๊ตฌํ๊ณผ delete()์ toDeleteHistories()์ ๊ฐ์ด ์ํ ๋ณ๊ฒฝ๊ณผ List ๋ฐํํ๋ ๋ฉ์๋ ๋ ๊ฐ๋ก ๋ถ๋ฆฌํ๋ ๊ฒ ์ค ์ด๋ ์ ๊ทผ ๋ฐฉ์์ด ์ข์๊น? |
||
| public boolean isDeleted() { | ||
| return deleted; | ||
| private List<DeleteHistory> addDeleteAnswerHistory() { | ||
| return this.answers.addDeleteAnswerHistory(); | ||
| } | ||
|
|
||
| public List<Answer> getAnswers() { | ||
| return answers; | ||
| private void updateDeleted() { | ||
| deleted(); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]"; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||||
| package nextstep.qna.domain; | ||||||||||
|
|
||||||||||
| public class QuestionBody { | ||||||||||
|
|
||||||||||
| private String value; | ||||||||||
|
Contributor
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.
Suggested change
์ด ๋ ๊ฐ์ ๊ฐ์ ๊ฐ์ง๋ ๊ฐ์ฒด๋ฅผ ์๋ฏธํ์ |
||||||||||
|
|
||||||||||
| public QuestionBody(String value) { | ||||||||||
| this.value = value; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| public abstract class SoftDeletableModel { | ||
|
Contributor
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. ๐ |
||
|
|
||
| private LocalDateTime createdDate = LocalDateTime.now(); | ||
|
|
||
| private LocalDateTime updatedDate; | ||
|
|
||
| private boolean deleted = false; | ||
|
|
||
| protected void deleted(){ | ||
| this.deleted = true; | ||
| } | ||
|
|
||
| protected boolean getDeleted() { | ||
| return deleted; | ||
| } | ||
| } | ||
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.
๐