Refactor validateAuditPeriod middleware #35
Conversation
…ead of the parameters of URL
| const { year, half } = await findYearAndHalf(req); | ||
| const today = new Date(); | ||
| const year = today.getFullYear(); | ||
| const half = today.getMonth() < 8 ? 'spring' : 'fall'; |
There was a problem hiding this comment.
js에서 Date는 8월이 7에 대응되기 때문에 7로 수정하는것이 의도에 맞을 것 같아요.
|
|
||
| const auditPeriod = await AuditPeriod.findOne({ | ||
| where: { | ||
| year: year, | ||
| half: half, | ||
| }, | ||
| }); | ||
|
|
||
| if (!auditPeriod) { | ||
| logger.debug(`audit period does not exist`); | ||
| throw new errors.NotFoundError('감사기간이 존재하지 않습니다.'); |
There was a problem hiding this comment.
이 부분에 대해서 여러 고민을 해봤는데, Logic Error인 것 같아요.
예시로,
- 1분기 감사기간이 끝난 8월 이후 2분기 감사기간 중에 1분기 감사기간의 항목의 수정을 시도한다면 정상적으로 통과합니다.
- 다른 에러 케이스는 1분기 감사기간이 끝난 8월 이후 2분기 감사기간 전에 항목의 수정을 시도한다면 '감사기간이 존재하지 않습니다.' 에러가 발생할텐데요, 에러 메시지가 에러 상황을 잘 표현하지 못하는 것 같아요. 1. 정말 감사기간이 만들어지지 않은 경우와 2. 감사기간이 지나고 수정을 요청하는 경우를 구분할 수 있어야 할 것 같습니다.
There was a problem hiding this comment.
그럼 다음 로직을 따르겠습니다.
-
감사기간을 request parameter에 넣어준 경우: 해당 감사 기간이 존재한다면 그 감사기간에 대해서 request를 수행하고, 존재하지 않는다면 오류 메시지를 내보낸다.
-
감사 기간을 request parameter에 넣어 주지 않은 경우:
2월<= request가 들어온 month < 8월일 시 request가 들어온 연도의 봄학기 감사기간에 대하여 request 처리,8월<=request가 들어온 month<=12월의 경우 request가 들어온 연도의 가을학기, request가 들어온 month가 1월의 경우 request가 들어온 연도의 전년도 가을학기에 대하여 request를 처리한다.
Byunk
changed the title
@yym0329 PR 감사합니다! 그러나 아직 middleware가 cover하지 못한 케이스들이 존재하는 것 같습니다.
결국 유저의 요청이 처리하는 resource (e.g. budgets, incomes, expenses, etc)를 통해 (organization_id, year, half) 쌍을 찾아내고 이를 통해 validation하는 로직이 가장 안전할 것 같다는 생각이 드네요. 다양한 parameters 혹은 body 에 담긴 정보를 바탕으로 Key (organization_id, year, half)를 가져오는 모듈을 하나 생성하고, validateAuditPeriod 뿐만 아니라 다른 middleware에서도 이를 활용하는 쪽으로 refactoring을 하는 것이 어떨까요? Next step을 정리하면 다음과 같을 것 같아요.
|
|
Open #36 |
|
질문1: validateAuditPeriod의 year, half 종속성을 없앤다는 말은, year, half를 URL parameter에 넣어주지 않아도 validateAudioPeriod가 동작할 수 있도록 한다는 말인가요? validateAuditPeriod가 동작하는데 year, half 정보를 parameter 혹은 body의 field로 넣어주지 않아도 감사기간을 알아서 찾아야 한다는 뜻일까요? 만약 그렇다면 케이스를 네 가지로 나눌 수 있을 것 같아요.
질문 1-1: 근데 3번 케이스에서는 유저가 DB에서 item의 id를 parameter에 넣어서 호출한다고 했을 때, DB에서 id 활용하여 검색하려면 어떤 type 데이터인지 미리 알아야하지 않나요? 만약 그렇다면 이 경우 검색하는 로직을 어떻게 구성하면 좋을지 고민해보겠습니다 질문2: Key (organization_id, year, half) extraction module은 어느 위치에 작성하는 게 맞나요? 위 질문들이 해결되기 전까지는 우선 테스트 케이스를 추가하도록 하겠습니다 |
|
@yym0329 Case study는 좋은 것 같아요 👍 |
|
1번 질문에서 한 case study에 관련해서 추가 질문이 있습니다. auditPeriod를 참조해서 들고 있는 타입인 budget, card_record, card, account에 대해서만 validate Audit Period 하면 되나요? 아니면 cascade 식으로 위 타입 데이터를 참조하는 모든 데이터에 대해서 validateAuditPeriod 미들웨어가 작동하는 것이 필요한가요? |
|
@yym0329 후자인 것 같아요. audit period를 직접적으로 참조하진 않더라도 감사기간에 영향을 받을 수 있으니까요. |
|
일단 DB schema에 정의된 모든 데이터에 대한 요청에서 body와 parameter 상에 포함된 DB object id를 활용하여 auditPeriod를 추출할 수 있도록 findYearandHalf 모듈을 작성했습니다. 이 모듈을 활용하여 validateAuditPeriod 미들웨어를 작성했습니다. 그리고 findYearandHalf 모듈에 대하여, 모든 데이터 오브젝트에 대한 기본적인 테스트 케이스를 추가해뒀습니다 |
|
Refactoring까지 어느 정도 됐습니다. 테스트 케이스 중 year, half 파라미터가 어디에도 주어지지 않은 경우, today's date 기준으로 판단하는 것에 대한 것을 추가해야 합니다. 그 방법에 대해 질문이 있는데, new date() 이 js 내부 메소드를 스터빙하는 쪽으로 시도하면 되나요? |
|
@yym0329 잘 이해가 되지 않는데, 혹시 구체적으로 설명해주실래요? |
|
오늘 날짜 생성하는 부분을 어떻게 스터빙할까에 대한 질문이었습니다. 이 부분 테스트 작성하는 것 빼고는 Refactoring 일단 진행완료했습니다 |
…ead of the parameters of URL