체크리스트 공지를 만들 수 있다

[minhyeok4dev]

318

각각의 커밋과 이슈를 매칭해서 작업했기 때문에 보시기엔 더 좋지 않을까 싶습니다 자체적으로 발견한 버그들까지 해결해서 올렸구요. 그 외 발견되는 이슈들은 이제 이곳에서 추가해 나가겠습니다.

[minhyeok4dev]

테스트할때 썼던 코드가 들어가있어서 삭제합니다

[minhyeok4dev]

Checklist의 assignee는 User 모델과 연결되어 더욱 동적인 역할을 할수있게 되었습니다. 또한 나중을 대비하여 여러명의 User를 assignee로 설정할 수 있도록 관계를 설정하였습니다. 3개의 모델이 들어간 Nested Form이 쫌 복잡해서 오래걸렸네요 휴

[shaynekang]

수고하셨습니다~~

1. 두 개 이상의 모델을 폼에서 사용하고 싶다면 공식적으로는 accepts_nested_attributes_for + fields_for 조합을 사용하지만, 이게 쓰다보면 엄청 구조가 복잡해지고 하다보면 안 되는게 많습니다. 이런 경우는 Form Object를 추천합니다. ㅎㅎ
2. views/admin/notices/_all_users_modal.html.erb는 views/admin/users/index.html.erb와 비슷한 점이 많은데, 두 개를 합칠 수 있지 않을까요? 사용자의 수가 많다보니 장기적으로는 리스트의 검색, 정렬, 필터링 등이 들어갈텐데, 사전에 합쳐주면 양 쪽에 모두 구현하는 불편함을 덜 것 같습니다. ㅎㅎ
3. views/admin/notices/_all_users_modal.html.erb 의 회원 리스트창은 padding을 조금 주면 디자인이 더 안정감 있을 것 같습니다. ㅎㅎ 그리고 Bootstrap Modal은 Header와 Footer를 붙여줘야 더 예쁜 것 같습니다.
4. views/admin/notices/_all_users_modal.html.erb에 User.all과 같이 모델 코드를 바로 부르는 부분이 있는데, 이렇게 뷰에서 모델을 바로 쓰는 코드가 생기면 코드 유지보수에 어려움이 많습니다.(그래서 MVC라는 개념이 존재하는거죠. ㅎㅎ) 가능하면 모델과 SQL은 컨트롤러에서 몰아서 처리해주세요. ㅎㅎ
5. 요건 아마 views/admin/users/index.html.erb에서 긁어온 것 같은데, views/admin/notices/_all_users_modal.html.erb에서 <% if user.activated? %>처럼 모델 값을 랜더링할때 조건문이 들어가는 코드는 데코레이터로 빼주면 뷰가 더 간결해집니다. ㅎㅎ
6. 자바스크립트의 네이밍 컨벤션은 camelCase입니다. user_modal보다는 userModal이 좋을 것 같습니다. ㅎㅎ

7. 자바스크립트는 워낙 난해한 언어라, 어떤 것이 정석적인 구현이라고는 얘기할 수 없습니다. 자바스크립트의 대가들마다 설계 방식이 천차만별이죠. ㅋ 이 부분은 일단 참고만 하고 패스해주셔도 됩니다. 프로토타입 기반 언어는 보통 init과 같은 함수로 객체를 생성하지 않습니다. 정석적인 방법은 다음과 같죠.

   javascript // 이렇게 익명 함수로 감싸면 전역변수로 선언되는 오류를 막을 수 있습니다. (function(){ UserModal = function(){ this.foo = "bar"; return this; };

   userModal = new UserModal(); console.log(userModal.foo); // > "bar" }());

   
   
   자바스크립트 서적을 찾아보면 이와 비슷한 구현 방식을 찾아볼 수 있을겁니다. 한 번 참고해보세요. ㅎㅎ
   그리고 어제 멘토링에서도 잠시 이야기했지만, 이와 비슷한 기능을 구현하는 현업에서의 가장 정석적인 방법은 jQuery의 [Plugin](http://learn.jquery.com/plugins/basic-plugin-creation/)을 활용하는 겁니다. 이걸 찾아보고 고치셔도 좋습니다. ㅎㅎ

8. controllers/assignee_comments_controller.rb 의 Notice.find(params[:notice_id]).checklists.find(params[:checklist_id]).assignee_comments.create(assignee_comments_params)같은 코드는 한 줄로 묶지 말고 여러개로 나눠주셔야 읽기 쉬울 것 같습니다. ㅋ 같은 파일의 다른 코드도 비슷한 부분이 있네요. 저는 보통 한 줄에 하나의 function call만 하는 걸 권장합니다. 예외적인 상황이 몇 가지 있긴 합니다만, 보통은 그렇게 짜는게 코드가 읽기 쉽더라구요. ㅎㅎ
9. 그리고 controllers/assignee_comments_controller.rb 의 #create는 예외 처리를 하지 않을거면 bang(!)으로 호출하는게 좋을 것 같습니다.
10. 전반적으로 뷰를 다채롭게(?) 짠 건 좋아 보이는데, 뷰 안에 로직 코드가(if-else 문이라던지) 많이 섞여있어서 코드가 복잡해지는 것 같습니다. Decorator나 Presenter를 활용해보세요. ㅎㅎ

[shaynekang]

1. Notice 모델의 must_has_checklists는 must_have_checklists여야 할 것 같은데... 또 티셔야 하나 만드는건가요. ㅋ 그리고 if self.checklists.empty? 조건문이 중복된 것 같네요. ㅎㅎ

   def must_has_checklists
    if self.checklists.empty?
      errors.add(:task, '하나 이상의 체크리스트가 있어야 합니다') if self.checklists.empty?
    end
   end

2. controllers/admin/notices_controller.rb 20.times { @notice.checklists.build.assign_histories.build } 를 보니 체크리스트는 20개 이상 생성을 못 하는 것 같네요. ㅋ 그리고 이렇게 되면 체크리스트가 아닌 공지를 생성/수정할 때도 체크리스트를 build해줘야 하는 것 같은데, 정석적으로 할려면 Ajax호출로 매 번 새로 생성해줘야 할 것 같습니다. ㅎㄷ

이상입니다. 수고하셨습니다~