gdsc-kaist / board-of-audit-and-inspection-system

KAIST board of audit and inspection system backend
https://dev-bai.gdsckaist.com/api-docs
0 stars 2 forks source link

Refactor validateAuditPeriod middleware #35

Open yym0329 opened 8 months ago

yym0329 commented 8 months ago

…ead of the parameters of URL

Byunk commented 7 months ago
  1. 1분기 감사기간이 끝난 8월 이후 2분기 감사기간 중에 1분기 감사기간의 항목의 수정을 시도한다면 정상적으로 통과합니다.
  2. 다른 에러 케이스는 1분기 감사기간이 끝난 8월 이후 2분기 감사기간 전에 항목의 수정을 시도한다면 '감사기간이 존재하지 않습니다.' 에러가 발생할텐데요, 에러 메시지가 에러 상황을 잘 표현하지 못하는 것 같아요. 1. 정말 감사기간이 만들어지지 않은 경우와 2. 감사기간이 지나고 수정을 요청하는 경우를 구분할 수 있어야 할 것 같습니다.

@yym0329 PR 감사합니다! 그러나 아직 middleware가 cover하지 못한 케이스들이 존재하는 것 같습니다.

  1. validateAuditPeriod에서 year와 half에 대한 종속성을 없애는 것이 목표였는데 이 경우 문제가 해결되지 않음
  2. year와 half가 parameter에 담기지 않았을 경우 위에서 제시한 두가지 로직 에러가 해결되지 않음.

결국 유저의 요청이 처리하는 resource (e.g. budgets, incomes, expenses, etc)를 통해 (organization_id, year, half) 쌍을 찾아내고 이를 통해 validation하는 로직이 가장 안전할 것 같다는 생각이 드네요.

다양한 parameters 혹은 body 에 담긴 정보를 바탕으로 Key (organization_id, year, half)를 가져오는 모듈을 하나 생성하고, validateAuditPeriod 뿐만 아니라 다른 middleware에서도 이를 활용하는 쪽으로 refactoring을 하는 것이 어떨까요?

Next step을 정리하면 다음과 같을 것 같아요.

  1. Key (organization_id, year, half) extraction module
  2. 감사기간에 대한 edge test cases 추가
  3. refactor middlewares
Byunk commented 7 months ago

Open #36

yym0329 commented 7 months ago

질문1: validateAuditPeriod의 year, half 종속성을 없앤다는 말은, year, half를 URL parameter에 넣어주지 않아도 validateAudioPeriod가 동작할 수 있도록 한다는 말인가요? validateAuditPeriod가 동작하는데 year, half 정보를 parameter 혹은 body의 field로 넣어주지 않아도 감사기간을 알아서 찾아야 한다는 뜻일까요?

만약 그렇다면 케이스를 네 가지로 나눌 수 있을 것 같아요.

  1. URL parameter에 year, half를 넣은 경우 : parameter의 year, half 사용
  2. request body의 year, half 필드에 감사 기간을 넣은 경우: body의 year, half 사용
  3. 유저가 접근하고자 하는 DB 상의 자원에서 year, half를 얻을 수 있는 경우: DB 자원에서 year, half 추론
  4. 그 외: request time을 활용해서 감사 기간 결정 / Error message 출력

질문 1-1: 근데 3번 케이스에서는 유저가 DB에서 item의 id를 parameter에 넣어서 호출한다고 했을 때, DB에서 id 활용하여 검색하려면 어떤 type 데이터인지 미리 알아야하지 않나요? 만약 그렇다면 이 경우 검색하는 로직을 어떻게 구성하면 좋을지 고민해보겠습니다

질문2: Key (organization_id, year, half) extraction module은 어느 위치에 작성하는 게 맞나요?

위 질문들이 해결되기 전까지는 우선 테스트 케이스를 추가하도록 하겠습니다

Byunk commented 7 months ago

@yym0329 Case study는 좋은 것 같아요 👍 1번 질문에 대한 답변은 현재 master branch에서 활용하는 방법을 (request의 params or body에 담겨있는 파라미터 활용) 발전 시켜도 좋을 것 같아요. 2번 질문은 어려운데요 우선 middleware 쪽에 파일을 하나 만들어서 작업하시고 같이 structure를 고민해보시죠.

yym0329 commented 7 months ago

1번 질문에서 한 case study에 관련해서 추가 질문이 있습니다. auditPeriod를 참조해서 들고 있는 타입인 budget, card_record, card, account에 대해서만 validate Audit Period 하면 되나요? 아니면 cascade 식으로 위 타입 데이터를 참조하는 모든 데이터에 대해서 validateAuditPeriod 미들웨어가 작동하는 것이 필요한가요?

Byunk commented 7 months ago

@yym0329 후자인 것 같아요. audit period를 직접적으로 참조하진 않더라도 감사기간에 영향을 받을 수 있으니까요.

yym0329 commented 7 months ago

일단 DB schema에 정의된 모든 데이터에 대한 요청에서 body와 parameter 상에 포함된 DB object id를 활용하여 auditPeriod를 추출할 수 있도록 findYearandHalf 모듈을 작성했습니다. 이 모듈을 활용하여 validateAuditPeriod 미들웨어를 작성했습니다.

그리고 findYearandHalf 모듈에 대하여, 모든 데이터 오브젝트에 대한 기본적인 테스트 케이스를 추가해뒀습니다

yym0329 commented 7 months ago

Refactoring까지 어느 정도 됐습니다. 테스트 케이스 중 year, half 파라미터가 어디에도 주어지지 않은 경우, today's date 기준으로 판단하는 것에 대한 것을 추가해야 합니다. 그 방법에 대해 질문이 있는데, new date() 이 js 내부 메소드를 스터빙하는 쪽으로 시도하면 되나요?

Byunk commented 7 months ago

@yym0329 잘 이해가 되지 않는데, 혹시 구체적으로 설명해주실래요?

yym0329 commented 7 months ago

오늘 날짜 생성하는 부분을 어떻게 스터빙할까에 대한 질문이었습니다. 이 부분 테스트 작성하는 것 빼고는 Refactoring 일단 진행완료했습니다