search

DonutWorks / Ari

0 stars 0 forks source link

Feature/check payment upgrade #391

Closed minhyeok4dev closed 9 years ago

minhyeok4dev commented 9 years ago

383

회계 관련한 이슈 모음들을 처리했습니다 커밋명으로 어떤 기능들이었는지 바로 확인이 가능할것 같습니다

shaynekang commented 9 years ago

확인했습니다!

  1. {id: response.id, activity: notice.activity, notice: notice, response: response, user: response.user}같이 긴 해시는 개행을 해서 넣어주셔야 보기 편합니다. 그리고 이렇게 컨트롤러 코드가 길어지는 건 주기적으로 서비스 오브젝트로 빼 주세요. ㅎㅎ
  2. 자주 깅조하지만, 예외 처리를 별도로 하지 않았을 경우에는 뱅(!) 메소드를 사용해 주세요. models/expense_record.rb의 reset_response에선 self.save보단 self.save!가 더 좋습니다.

아직 MVC의 개념에 익숙하지 않은 것 같습니다. 자주 강조하지만, 가능한 모든 코드는 모델로, 그리고 가능하면 데코레이터/서비스 등을 활용해주세요. 컨트롤러에 SQL 쿼리를 날리는 부분 (where)이 많이 보이는데, 이런 것들은 전부 모델로 빼야 합니다.

여튼 수고하셨습니다~

yhoonkim commented 9 years ago
  1. 엑셀파일 양식 다운로드 하는 link_to 이름이 "명단예제"임. => "계좌내역 예제"로 변경하자!
  2. 계좌내역에서 수동으로 활동내 입금자를 찾을 수 있는 것 처럼, 활동>TO>참가자의 회비 내역을 클릭했을 모달상자가 떠서 계좌내역을 보고 연결시켜줄 수 있어야 하지 않을까? - 만약 회계내역에서 찾지 못하다면 "일단 확인"버튼으로 넘어갈 수 있고...(지금은 취소만 되는듯?)
minhyeok4dev commented 9 years ago

피드백 반영했습니다 @yhoonkim 우선 개발스펙까지는 회계에서 지정하는 부분까지였고, 그 부분은 새로운 고려사항인것같아!

yhoonkim commented 9 years ago

merging을 제가 맡아서 하고있습니다.