refactor(apply): redesign components of the my application page #761

woowahan-cron · 2024-07-02T05:29:44Z

Resolves #758

해결하려는 문제가 무엇인가요?

내 지원 현황 페이지의 컴포넌트 디자인을 정리하고 변경된 스타일을 적용한다.
- "과제 보기" 기능을 추가하기 전, 이와 관련된 미션 컴포넌트의 코드를 리팩토링하는 작업을 진행하였습니다.
- 컴포넌트의 책임을 명확히 하고, 코드의 재사용성과 유지보수성을 향상시키고자 했습니다.

어떻게 해결했나요?

MyMissionItem 컴포넌트 개선
- 사용하는 로직을 useMission이라는 훅으로 분리하여 컴포넌트의 복잡도를 낮추었습니다.
- 버튼 관련 비즈니스 로직을 상위 컴포넌트인 MyMissionItem으로 이동하고, 이를 훅으로 분리했습니다.
버튼 컴포넌트 리팩토링
- 제출(ApplyButton), 새로고침(RefreshButton), 예제 테스트 실행(JudgmentButton) 버튼의 비즈니스 로직을 분리했습니다.
- 별도의 컴포넌트 대신 @common/Button 컴포넌트를 사용하여 코드 중복을 제거했습니다.
새로운 훅 생성
- useMissionJudgment.ts: 예제 테스트 실행 요청 및 실행 가능 상태 판별 로직을 포함합니다.
- useRefresh.ts: 새로고침 요청 및 버튼 표시 상태 판별 로직을 포함합니다.
- useMission.ts: MyMissionItem의 로직을 커스텀 훅으로 분리하였습니다. 그리고 useMissionJudgment.ts 훅과 useRefresh.ts 훅을 의존하고 있습니다. (새로고침, 예제 테스트 요청 후 UI에 반영하는 작업)
프로젝트 구조 개선
- 컴포넌트 특정 훅은 해당 컴포넌트와 같은 디렉토리에 위치시켰습니다. 이는 의존성을 명확히 하고 관련 코드를 함께 관리하기 위함입니다.
부분적 UI 개선 작업
- 별도로 짚어 드렸어요!

어떤 부분에 집중하여 리뷰해야 할까요?

이번 PR에서는 커스텀 훅으로 분리하는 리팩터링 작업을 메인으로 진행했습니다.
이에 따라 어떻게 동작해야 하는지 명시하기 위해 테스트 코드를 커스텀 훅별로 작성했습니다!
테스트 코드를 통해 어떻게 동작하는지 파악하신다면 동작과 상태 파악이 수월하실 것 같습니다.

로직 통합 과정에서의 기능 변경 여부
- 특히 조건문 병합 시 의도하지 않은 동작 변경이 있는지 확인 부탁드립니다.
- isJudgmentAvailable, isRefreshAvailable 등의 조건식이 요구사항을 정확히 반영하는지 검토 부탁드립니다.
컴포넌트 구조 개선의 적절성
- 버튼 컴포넌트 통합 및 로직 분리가 적절한지 검토해 주세요.
- MyMissionItem에서 분리된 로직들의 위치가 적절한지 확인 부탁드립니다.
훅 분리 및 위치의 타당성
- useMission, useMissionJudgment, useRefresh 훅의 분리와 위치가 적절한지 검토해 주세요.
- 컴포넌트 특정 훅을 해당 컴포넌트 디렉토리에 두는 방식에 대한 의견을 주시면 감사하겠습니다.
타입 정의 및 사용의 정확성
- 특히 Mission, Judgment 관련 타입 사용이 올바른지 확인 부탁드립니다.

RCA 룰

r: 꼭 반영해 주세요. 적극적으로 고려해 주세요. (Request changes)
c: 웬만하면 반영해 주세요. (Comment)
a: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다. (Approve)

woowahan-cron · 2024-07-02T07:26:16Z

frontend/src/components/MyApplicationItem/MyApplicationButtons/RefreshButton.tsx

-  if (status !== JUDGMENT_STATUS.STARTED || missionItem.status !== MISSION_STATUS.SUBMITTING) {
-    return null;
-  }
-
-  if (missionId === undefined || recruitmentId === undefined) {
-    return null;
-  }
-
-  if (isJudgmentTimedOut(missionItem.judgment)) {
-    return null;
-  }


새로고침 버튼에서 수행하는 비즈니스 로직을 상위 컴포넌트에서 판별 후 렌더링 여부를 결정하도록 수정

woowahan-cron · 2024-07-02T07:31:49Z

frontend/src/components/MyApplicationItem/MyMissionItem/useRefresh.ts

+  const isRefreshAvailable =
+    isValidMissionId &&
+    missionItem.judgment?.status === JUDGMENT_STATUS.STARTED &&
+    missionItem.status === MISSION_STATUS.SUBMITTING &&
+    !isJudgmentTimedOut(missionItem.judgment);


이전에는 RefreshButton에서 버튼 자체를 렌더링할지 판별하는 코드가 있었습니다.

버튼 자체에서 비즈니스 로직을 판별하고 자신을 렌더링하는 것은 부적합하다는 판단이 들어 상위 컴포넌트인 MyMissionItem 컴포넌트에서 이를 렌더링할지 결정합니다.

MyMissionItem UI로직과 구분하기 위해 useRefresh 훅을 만들었습니다.

woowahan-cron · 2024-07-02T07:34:25Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMissionJudgement.ts

useJudgment.ts가 좀 더 적절할까요?

useMission 컴포넌트는 MyMissionItem에서 의존하는 커스텀 훅

커스텀 훅인 useMission에서 의존하는 커스텀 훅으로는 useMissionJudgement, useRefresh가 있습니다.

woowahan-cron · 2024-07-02T07:55:25Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMission.ts

+  const requestRefresh = async () => {
+    try {
+      const result = await fetchRefreshedResultData();
+      setMissionItem(result);
+    } catch (error) {
+      error instanceof Error && alert(error.message);
+    }
+  };
+
+  const requestMissionJudgment = async () => {
+    try {
+      const result = await fetchJudgmentMissionResult();
+
+      setMissionItem(result);
+    } catch (error) {
+      error instanceof Error && alert(error.message);
+    }
+  };


MyMissionItem 컴포넌트에서 새로고침 버튼이나 예제 테스트 실행 버튼을 눌러 UI와 상호작용 하는 코드는 useMission 커스텀 훅을 의존하도록 했는데 이 구성이 적절하다고 생각하시는지, 좀 더 바람직한 방향은 없을지 궁금합니다.

woowahan-cron · 2024-07-02T07:57:00Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMission.ts

기존 MyMissionItem 컴포넌트가 가진 로직을 커스텀 훅으로 분리하는 작업을 진행하였습니다.

woowahan-cron · 2024-07-03T00:33:25Z

frontend/src/components/MyApplicationItem/MissionDetail/MissionDetail.module.css

+@media screen and (max-width: 800px) {
+  .title-container {
+    flex-direction: column;
+    gap: 0.875rem;
+  }
+}


나중에 과제보기 버튼이 추가되면 우측에 버튼이 배치되어 있는 경우 가독성이 떨어지는 것을 대비하여
폭이 좁아질 경우 (800px) 기준으로 flex-direction을 row -> column로 변경하여 대응했습니다.
이것이 적절한지도 검토 부탁드려요!

(실제로 지금 과제보기 버튼이 추가되어 있는 상태는 아니고 크롬 개발자 도구에서 버튼을 복사하여 배치한 후 캡처한 화면입니다)

800px 이상의 화면

800px 미만의 화면

좋습니당 :)
바꿔주신 걸 보니 버튼이 아래로 내려오면 '테스트 코드 실행이 끝나기까지 3~5분이 걸릴 수 있습니다'는 왼쪽 정렬로 해두어도 좋을 것 같네용ㅋㅋ

wmakerjun

👏🏻 지원플랫폼 개선을 위해 개발하시느라 너무너무 고생 많으셨습니다!
제가 지원 플랫폼 코드를 별로 보지 못해서, 3가지 측면에 우선 집중해서 코드리뷰 남겨봤습니다.

테스트 코드의 구조화 및 리팩터링
코드 가독성 및 기능 분리
몇 가지 궁금한 점들

그래서 대부분 코멘트의 레벨을 c 혹은 a로 남겼어요. 보시고 필요하다고 생각하시는 부분만 고려해주시면 될 것 같습니다 😄

너무 고생하셨습니다 👏🏻

wmakerjun · 2024-07-03T23:16:48Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      const { result } = renderHook(() =>
+        useMissionJudgment({
+          missionItem: unsubmittedMissionItem,
+          recruitmentId: mockRecruitmentId,
+        })
+      );


c: 현재 각 테스트마다 renderHook을 반복 호출하고 있는것 같은데요. 혹시 이를 헬퍼 함수처럼 추출하여 재사용하는건 어떨까요?

function renderUseMissionJudgment(missionItem = mockMissionItem, recruitmentId = mockRecruitmentId) { return renderHook(() => useMissionJudgment({ missionItem, recruitmentId })); } // 예시 it('판정 가능한 경우', () => { const { result } = renderUseMissionJudgment(); expect(result.current.isJudgmentAvailable).toBe(true); });

wmakerjun · 2024-07-04T22:56:55Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+jest.mock("../../../../hooks/useTokenContext");
+jest.mock("../../../../utils/validation/judgmentTime");
+
+describe("useMissionJudgment 훅 테스트", () => {


c: 테스트 케이스들을 잘 작성해주셨네요! 👍
각 기능에 대해 세세한 단위 테스트까지 커버하고 있어 좋은것 같아요!
그래서 이 기능들이 더 잘 보이고, 나중에 관련 테스트를 추가하기 쉽게 테스트 구조를 기능 의미 단위로 구조화하면 가독성을 높일 수 있을 것 같은데 어떠신가요~?

describe('useMissionJudgment', () => { describe('isJudgmentAvailable', () => { describe('미션이 제출되지 않은 경우', () => { it('제출 기간 전일 때 false를 반환해야 한다', () => { /* ... */ }); it('제출 기간 중일 때 false를 반환해야 한다', () => { /* ... */ }); }); describe('미션이 제출된 경우', () => { it('예제 테스트 채점이 시작되지 않았을 때 true를 반환해야 한다', () => { /* ... */ }); it('예제 테스트 채점이 시작되었을 때 true를 반환해야 한다', () => { /* ... */ }); }); describe('미션 상태에 따른 판정 가능 여부', () => { it('ENDED 상태일 때 false를 반환해야 한다', () => { /* ... */ }); it('UNSUBMITTABLE 상태일 때 false를 반환해야 한다', () => { /* ... */ }); }); }); describe('fetchJudgmentMissionResult', () => { it('판정 결과를 성공적으로 가져와야 한다', async () => { /* ... */ }); it('판정 결과 가져오기 실패 시 에러를 던져야 한다', async () => { /* ... */ }); }); });

오! 이렇게 한 단계 depth를 더 나눌 생각을 못했었네요! 나누고 나니 테스트 가독성이 더욱 좋아진 것 같아요!
depth를 나눠서 가독성을 개선하되 도메인 내 용어의 사용은 좀 더 분명하게 드러낼게요!
ex) 미션을 제출 -> 미션 과제 제출물을 제출

wmakerjun · 2024-07-04T22:58:43Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+  const mockMissionItem: Mission = {
+    id: 1,
+    title: "테스트 미션",
+    description: "테스트 설명",
+    submittable: true,
+    submitted: true,
+    startDateTime: "2023-01-01T00:00:00",
+    endDateTime: "2023-12-31T23:59:59",
+    status: MISSION_STATUS.SUBMITTING,
+    runnable: true,
+    judgment: {
+      pullRequestUrl: "https://github.com/test/pr",
+      commitHash: "abcdef1234567890",
+      status: JUDGMENT_STATUS.STARTED,
+      passCount: 0,
+      totalCount: 10,
+      message: "",
+      startedDateTime: "2023-06-01T12:00:00",
+      commitUrl: "https://github.com/test/commit",
+    },
+  };


a. 객체 구조가 꽤 복잡해져서, 테스트 코드에서 미션 객체를 직접 수정할 때 실수할 확률도 있을것 같아요! 혹시 이걸 팩토리 함수로 만들어서 사용해보면 어떨까요~?

Suggested change

const mockMissionItem: Mission = {

id: 1,

title: "테스트 미션",

description: "테스트 설명",

submittable: true,

submitted: true,

startDateTime: "2023-01-01T00:00:00",

endDateTime: "2023-12-31T23:59:59",

status: MISSION_STATUS.SUBMITTING,

runnable: true,

judgment: {

pullRequestUrl: "https://github.com/test/pr",

commitHash: "abcdef1234567890",

status: JUDGMENT_STATUS.STARTED,

passCount: 0,

totalCount: 10,

message: "",

startedDateTime: "2023-06-01T12:00:00",

commitUrl: "https://github.com/test/commit",

},

};

function createMockMission(overrides = {}) {

return {

...mockMissionItem,

...overrides,

};

}

// 사용 예:

it('제출되지 않은 미션은 판정 불가능해야 한다', () => {

const unsubmittedMission = createMockMission({ submitted: false });

// ... 테스트 로직

});

제안 감사합니다! 일괄 반영하겠습니다.

wmakerjun · 2024-07-04T23:03:00Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      await act(async () => {
+        const judgmentResult = await result.current.fetchJudgmentMissionResult();
+        expect(judgmentResult).toEqual({ ...mockMissionItem, judgment: mockResponse.data });
+        expect(judgmentResult.judgment?.status).toBe(JUDGMENT_STATUS.SUCCEEDED);
+        expect(judgmentResult.judgment?.passCount).toBe(8);
+        expect(judgmentResult.judgment?.totalCount).toBe(10);
+      });


a: expect.objectContaining(object)을 이용하면 좀 더 간결하게 테스트할 수 있을 것 같아요~!

// 예시

Suggested change

await act(async () => {

const judgmentResult = await result.current.fetchJudgmentMissionResult();

expect(judgmentResult).toEqual({ ...mockMissionItem, judgment: mockResponse.data });

expect(judgmentResult.judgment?.status).toBe(JUDGMENT_STATUS.SUCCEEDED);

expect(judgmentResult.judgment?.passCount).toBe(8);

expect(judgmentResult.judgment?.totalCount).toBe(10);

});

await act(async () => {

const judgmentResult = await result.current.fetchJudgmentMissionResult();

expect(judgmentResult).toEqual(expect.objectContaining({

judgment: expect.objectContaining({

status: JUDGMENT_STATUS.SUCCEEDED,

passCount: 8,

totalCount: 10,

}),

}));

});

wmakerjun · 2024-07-04T23:13:51Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useMission.test.ts

+    it("missionItem, 버튼 라벨, 포맷된 날짜, 그리고 가용성 플래그를 올바르게 설정해야 한다", () => {
+      const { result } = renderHook(() =>
+        useMission({ mission: mockMission, recruitmentId: mockRecruitmentId })
+      );
+
+      expect(result.current.getter.missionItem).toEqual(mockMission);
+      expect(result.current.getter.applyButtonLabel).toBe(BUTTON_LABEL.SUBMIT);
+      expect(result.current.getter.formattedStartDateTime).toBe("2023-01-01 00:00");
+      expect(result.current.getter.formattedEndDateTime).toBe("2023-12-31 23:59");
+      expect(result.current.getter.isJudgmentAvailable).toBe(true);
+      expect(result.current.getter.isRefreshAvailable).toBe(true);
+    });


a: 테스트 명이 테스트의 세부 내용을 잘 설명하고 있는 것 같아요! 그런데 혹시 좀 더 간결하게 표현해보는 건 어떨까요? 그리고 objectContaining을 이용해서 테스트 검증을 하면 더 간결해질 것 같아요~!

Suggested change

it("missionItem, 버튼 라벨, 포맷된 날짜, 그리고 가용성 플래그를 올바르게 설정해야 한다", () => {

const { result } = renderHook(() =>

useMission({ mission: mockMission, recruitmentId: mockRecruitmentId })

);

expect(result.current.getter.missionItem).toEqual(mockMission);

expect(result.current.getter.applyButtonLabel).toBe(BUTTON_LABEL.SUBMIT);

expect(result.current.getter.formattedStartDateTime).toBe("2023-01-01 00:00");

expect(result.current.getter.formattedEndDateTime).toBe("2023-12-31 23:59");

expect(result.current.getter.isJudgmentAvailable).toBe(true);

expect(result.current.getter.isRefreshAvailable).toBe(true);

});

it("올바른 초기 상태를 반환해야 한다", () => {

const { result } = renderHook(() =>

useMission({ mission: mockMission, recruitmentId: mockRecruitmentId })

);

expect(result.current.getter).toEqual(

expect.objectContaining({

missionItem: expect.objectContaining(mockMission),

applyButtonLabel: BUTTON_LABEL.SUBMIT,

formattedStartDateTime: "2023-01-01 00:00",

formattedEndDateTime: "2023-12-31 23:59",

isJudgmentAvailable: true,

isRefreshAvailable: true,

})

);

});

wmakerjun · 2024-07-04T23:35:07Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMission.ts

+  const [missionItem, setMissionItem] = useState<Mission>({ ...mission });
+  const { isRefreshAvailable, fetchRefreshedResultData } = useRefresh({
+    missionItem,
+    recruitmentId: Number(recruitmentId),


a: 제가 정확한 로직을 몰라서 남기고 있는걸수도 있는데요. recruitmentId를 문자열로 쓰는 경우가 있고 숫자로 쓰는 경우가 있는것 같아요. 이게 헷갈리는 포인트가 될 수 있을것 같은데, 이를 개선할 수 있는 방법이 있을까요? 🤔

맞습니다. recruitmentId가 프로젝트 내에서 어떤 경우에는 숫자로 쓰이고 있고 어떤 경우에는 문자열로 쓰이고 있습니다. 이 부분을 정리하는 것은 다른 이슈에서 진행해봐야 할 것 같아요. 왜냐하면, 저도 아직 정확한 로직을 다 파악한 것은 아니기 때문에 일관성을 맞추는 작업을 지금 진행하기에는 두렵기 때문인데요😅. 다른 이슈를 해결하면서 이 부분도 꼭 고민을 해야 할 부분이기에, 말씀해 주신 부분 고민해서 개선해보겠습니다~!

wmakerjun · 2024-07-04T23:36:43Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useMission.test.ts

+  useNavigate: jest.fn(),
+}));
+
+describe("useMission", () => {


c: useMission이라는 타이틀과 실제 테스트를 보면 중요한 로직을 담당하고 있는것처럼 느껴져요. 혹시 이 훅에 대한 간단한 통합 테스트가 한 개 추가되면 어떨까요~?

컴포넌트가 잘 동작하는지 통합 테스트도 작성해보겠습니다~

wmakerjun · 2024-07-04T23:40:25Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMissionJudgment.ts

+  const isJudgmentAvailable =
+    submitted &&
+    (judgment?.status !== JUDGMENT_STATUS.STARTED || isJudgmentTimedOut(judgment)) &&
+    !([MISSION_STATUS.ENDED, MISSION_STATUS.UNSUBMITTABLE] as MissionStatus[]).includes(status);


c: isJudgmentAvailable의 조건문이 중요한 로직 중에 하나로 보이는데 다소 복잡해 보이는 것 같아요~! 이를 더 명확하게 분리하면 가독성도 올라가고 테스트하기도 더 좋지 않을까 싶어요!

Suggested change

const isJudgmentAvailable =

submitted &&

(judgment?.status !== JUDGMENT_STATUS.STARTED || isJudgmentTimedOut(judgment)) &&

!([MISSION_STATUS.ENDED, MISSION_STATUS.UNSUBMITTABLE] as MissionStatus[]).includes(status);

const isSubmitted = submitted;

const isJudgmentNotStartedOrTimedOut =

judgment?.status !== JUDGMENT_STATUS.STARTED || isJudgmentTimedOut(judgment);

const isMissionActive =

![MISSION_STATUS.ENDED, MISSION_STATUS.UNSUBMITTABLE].includes(status);

const isJudgmentAvailable =

isSubmitted && isJudgmentNotStartedOrTimedOut && isMissionActive;

놓치고 있던 부분이었네요! 반영해두겠습니다💪

wmakerjun · 2024-07-05T00:45:30Z

frontend/src/components/MyApplicationItem/MyMissionItem/useRefresh.ts

+  const isRefreshAvailable =
+    isValidMissionId &&
+    missionItem.judgment?.status === JUDGMENT_STATUS.STARTED &&
+    missionItem.status === MISSION_STATUS.SUBMITTING &&
+    !isJudgmentTimedOut(missionItem.judgment);


c: isRefreshAvailable의 조건문이 다소 복잡한 것 같아요. 이를 조금 분리해서 작성해보면 어떨까요?

Suggested change

const isRefreshAvailable =

isValidMissionId &&

missionItem.judgment?.status === JUDGMENT_STATUS.STARTED &&

missionItem.status === MISSION_STATUS.SUBMITTING &&

!isJudgmentTimedOut(missionItem.judgment);

const isValidMissionId = missionItem.id !== undefined && recruitmentId !== undefined;

const isJudgmentStarted = missionItem.judgment?.status === JUDGMENT_STATUS.STARTED;

const isMissionSubmitting = missionItem.status === MISSION_STATUS.SUBMITTING;

const isJudgmentNotTimedOut = !isJudgmentTimedOut(missionItem.judgment);

const isRefreshAvailable =

isValidMissionId &&

isJudgmentStarted &&

isMissionSubmitting &&

isJudgmentNotTimedOut;

이 부분도 개선하면 훨씬 읽을 때 가독성이 개선될 것 같아요🔥 ㅎㅎ

wmakerjun · 2024-07-05T00:47:14Z

frontend/src/components/MyApplicationItem/MyMissionItem/useRefresh.ts

+import useTokenContext from "../../../hooks/useTokenContext";
+
+type Props = {
+  recruitmentId?: Recruitment["id"];


a: recruitmentId를 옵셔널로 받아야 하는 이유가 있을까요~? 🤔
현재 훅에서는 필수로 사용하는 것처럼 보여서요~!

제 생각에는 이 부분도 recruitmentId가 숫자로 쓰이는 경우와 문자열로 쓰이는 경우에 관해 피드백을 주신 코멘트와 함께 개선해 보면 좋겠다고 생각했습니다!

아직 저도 지원플랫폼 내부 구조를 다 경험한 것이 아니라서 코드를 한 번에 일관성을 맞추는 작업이 아직 테스트 코드가 많이 없기 때문에 조금 두렵게 느껴지더라구요🥲. 그래서 일단은 MyMissionItem 컴포넌트의 로직을 분리하는 데에만 집중을 했습니다😓. 제안 주신 부분도 다음 이슈에서 같이 개선해볼게요!

woowahan-cron

@wmakerjun

준이 피드백 주신 부분은 코드에 반영을 완료했습니다.
전반적으로 테스트 코드를 좀 더 깔끔하게 정리할 수 있는 노하우를 많이 말씀해 주신 것 같아요!
마치 복잡했던 바탕화면 아이콘을 깔끔하고 체계적으로 정리하는 느낌을 받았습니다 😁

before	after

이제는 우테코의 새로운 기수들과 함께할 수 있는 창구로 활용된 지도 역사가 깊어가고 있어서 가독성을 개선한 코드 작업과 유지/보수의 중요성이 나날이 높아지는 것 같습니다. 이번 리뷰를 통해 개선하는 작업으로 먼 훗날 작업하시는 누군가에게 유용한 코드가 되었으면 하는 바람으로 아래와 같이 개선을 진행해 보았습니다🙂

반영한 내용

테스트 코드에서 유틸 헬퍼 함수를 만들어 테스트 코드 정리
훅 조건부 리팩터링
- useMission
- useRefresh
테스트 코드 describe 범주화 및 세분화 작업

expect.objectContaining 단언문을 통한 검증으로 리팩터링

expect(result.current.getter).toEqual(
   expect.objectContaining({
     missionItem: expect.objectContaining(mockMission),
     applyButtonLabel: BUTTON_LABEL.SUBMIT,
     formattedStartDateTime: "2023-01-01 00:00",
     formattedEndDateTime: "2023-12-31 23:59",
     isJudgmentAvailable: true,
     isRefreshAvailable: true,
   })
 );
});

통합 테스트 코드 작성

다음 이슈에서 추후 다룰 것

아직 이 프로덕트의 전반적인 부분을 저도 다 알고 있지 않기 때문에 어떠한 맥락에서 requirementId 타입이 숫자/문자열로 혼용하여 쓰이고 있는지, 그리고 그에 따른 일관성을 맞추는 작업을 하면서 발생할 수 있는 부작용은 무엇인지까지 가팅 고려하여 추후 다음 이슈에서 개선해 보겠습니다.

woowapark

크론~ 리팩터링 작업 고생 많으셨습니다 🙂
r, c, a 코멘트들이 섞여있어서 Request Changes 로 드려요.
확인해보시구 궁금하신 부분이나 제 표현이 애매한 부분 등등은 편하게 알려주셔요~!

woowapark · 2024-07-09T05:16:40Z

frontend/src/components/MyApplicationItem/RecruitmentDetail/RecruitmentDetail.tsx

@@ -9,7 +9,7 @@ type RecruitmentDetailProps = {

 const RecruitmentDetail = ({ children, startDate, endDate }: RecruitmentDetailProps) => {
  return (
-    <>
+    <div>


a: React Fragment 대신 <div>를 사용하도록 변경해주신 이유가 있을까용? :) 개인적으로는 별도의 스타일을 주거나, 시맨틱하게 써야 하는 특정 태그를 쓰는 게 아니라면 오히려 따로 DOM에 노드가 추가되지 않는 <>가 간편해서 선호하는 편이라 여쭤봅니당

div가 시멘틱하진 않아서 저도 어떻게 사용해야 할지 고민되더라구요!
RecruitmentDetail 상위 컴포넌트인 MyMissionItem 컴포넌트의 title-container 스타일에서 flex-direction 영향을 받고 있어서 DOM 노드를 불가피하게 추가하게 되었습니닷!

이전까지는 아래의 버튼 배열을 display: relative의 속성에 별도의 하위 div가 있었고 이를 absolute로 우측 상단에 배치했었는데 이를 변경하는 과정에서 발생했습니다!

AS-IS

TO-BE

woowapark · 2024-07-09T05:22:33Z

frontend/src/components/@common/Button/Button.tsx

@@ -23,6 +25,7 @@ const Button = ({
      className={classNames(className, styles[variant], styles.button, {
        [styles.cancel]: cancel,
      })}
+      disabled={disabled}


a: React.ButtonHTMLAttributes<HTMLButtonElement>가 이미 disabled를 포함하고 있는 것으로 알고 있는데요! 혹시 더 명시적으로 표현해주기 위한 수정 사항일까요? 🙂

네넵! 버튼을 커스텀해서 사용하고 있어서 명시적으로 표현하기 위해서였는데 ButtonProps 선언부에서는 굳이 disabled 속성을 표기해 줄 필요가 없겠군요! 그 부분만 적용해서 반영해보겠습니다.

woowapark · 2024-07-09T05:31:23Z

frontend/src/components/MyApplicationItem/MissionDetail/MissionDetail.tsx

-      <JudgmentResultText judgment={judgment} />
+      <div className={styles["detail-status-container"]}>
+        <JudgmentResultText judgment={judgment} />
+        <div>{children}</div>


r: <MissionDetail />은 현재 <MyMissionItem />에서만 한 번 사용되는 것으로 보이는데요! 혹시 children으로 받는 부분이 이후에 커스텀을 대비한 요소일까요~? 지금 시점에서는 오히려 children으로 유연하게 열어두기보단<MyMissionItem /> 내부에 추가하는 편이 <MissionDetail />을 쓰는 쪽에서도 신경 쓸 게 덜하지 않나 하는 생각이 들어 여쭤봅니당 :)

앗 그러네요! 훅으로 분리했기 때문에 오히려 지금 children으로 넘겨 둔 Element는 MissionDetail 컴포넌트 이하로 옮겨두는 것이 더 좋을 것 같아요!

그리고 말씀하신 것처럼 지금은 MyMissionItem에서 한 번만 사용되고 있기 때문에 MyApplicationItem 이하 같은 레벨의 컴포넌트로 구성되어 있는데 이것을 MyMissionItem 컴포넌트 하위로 옮겨두는 것이 더 좋을 것 같다는 생각이 듭니다!

MyApplicationItem 컴포넌트

MissionDetail (현재 위치)

MyMissionItem

MissionDetail(수정 후 위치)

이전에 훅은 hooks/에 모여 있으니 특정 컴포넌트에서만 사용되는 훅은 다음번 PR에서 옮기는 것을 제안해 주셨는데 컴포넌트는 관계가 훅보다는 복잡한 것 같고 나중에 한꺼번에 작업하기 부담이 될 것 같다는 생각이 들었어요!

의존하고 있는 스타일이 있습니다. (ApplicationButtonStyles) -> 분리해도 되지만, 분리할 경우 동일한 속성 코드 선언의 반복으로 차후 수정 작업 시 일관성을 지키기 어렵겠다는 느낌이 드네요!

woowapark · 2024-07-09T05:33:44Z

frontend/src/components/MyApplicationItem/MyApplicationFormItem/MyApplicationFormItem.tsx

+                {applyButtonLabel}
+              </Button>
+            </li>
+          </ul>


c: 이후에 추가될 것을 대비한 ul일까용? 🙂

넵 맞습니다! 바로 다음 이슈에서 과제 보기 버튼을 추가할 예정이에요 ㅎㅎ

아하 해당 UI가 추가될 때 같이 추가해주면 더 좋을 것 같습니다 :) 이 PR에서 다루는 이슈에서는 필요하지 않을 요소인 것 같아서요~! 🙂

woowapark · 2024-07-09T05:41:12Z

frontend/src/components/MyApplicationItem/MyApplicationItem.module.css

+    gap: 0.875rem;
+  }
+
+  .title-button-list > li {


a: 혹시나 마크업을 수정해서 .title-button-list 바로 하위에 li 태그가 있지 않게 되면 스타일이 적용되지 않을 수도 있을 것 같아요. 지금 문제될 부분은 아니라고 생각하지만 위의 .title-button-list li:not(:last-child) 에서는 child combinator 대신 descendant selector를 써주셨길래 일관성 측면에서 맞춰도 좋을 것 같아 의견으로만 남겨둡니당 :) 선택자 우선순위 등의 문제로 넣으신거라면 그것도 좋고요!

오! 좋습니다 ㅎㅎ
헷갈릴 수도 있으니 일관성 맞추고 어떤 클래스에서 쓰이는 것인지도 분명하게 표시해 두면 좋겠다고 생각해서 반영해두겠습니다.

woowapark · 2024-07-09T08:48:25Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      });
+    });
+
+    it("판정 결과 가져오기 실패 시 에러를 던져야 한다", async () => {


r: 도메인 용어로 'Judgment'는 '채점'이었던 것 같아서 명세에서도 일관된 용어를 쓰도록 맞춰주면 좋을 것 같습니당 🙂

판정 -> 채점으로 용어 통일해두겠습니다

woowapark · 2024-07-09T08:50:48Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      it("미션 상태가 제출 기간 사이에 제출이 불가능한 경우(UNSUBMITTABLE)일 때 false를 반환해야 한다", () => {
+        expectJudgmentAvailability({ status: MISSION_STATUS.UNSUBMITTABLE }, false);
+      });
+    });


c: '테스트에선 DRY보다 DAMP가 더 좋을 수 있다'는 주장에 따라 지나치게 가독성을 방해하거나 유지보수에 문제가 있는 게 아니라면 단언문 등은 좀 더 명시적으로 개별 테스트 케이스에서 써줘도 좋을 것 같은데 어떠신가요?

아래의 다른 파일 코멘트에서도 비슷하게 피드백을 주셨던 것으로 기억해요! 이 부분도 같이 수정해두겠습니다 🙂
너무 엔지니어링 관점에서만 코드 작업을 하다 보니 테스트 명세도 구체적인 구현의 내용을 위주로 생각했던 것 같아요 🥲

woowapark · 2024-07-09T08:52:50Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      });
+    });
+
+    describe("미션 상태에 따른 판정 가능 여부", () => {


r: 요기도 도메인 용어 상 '판정'보다는 '채점 가능 여부' 혹은 '채점 요청 가능 여부' 등으로 표현을 맞춰주면 좋을 것 같아요~!

판정 -> 채점으로 용어 통일해두겠습니다! 👍

woowapark · 2024-07-09T08:53:56Z

frontend/types/domains/recruitments.ts

 };

+export type Judgment = {


a: 기존 코드 그대로 분리만 한 것으로 보여 이견 없습니다 :) 이후에 테스트 코드에서도 쓰이는 것 등을 고려하면 class 로 만들어서 활용해도 좋을 것 같더라고요.

맞습니다! 테스트 코드에서 Judgment 타입을 따로 분리하여 작성한 샘플 데이터 코드가 있는데 더욱 명확하게 보여주기 위해 분리해두었습니다! 추후 개선이 필요하면 class로 선언해 보는 것도 고려해 보겠습니다!

woowapark · 2024-07-09T08:58:02Z

frontend/src/components/MyApplicationItem/MyMissionItem/useRefresh.ts

+        token,
+      });
+
+      return { ...missionItem, judgment: response.data };


c: 응답에서 judgment의 타입은 제대로 추론되지 않을 것 같아요. 응답값 및 리턴값도 함께 타이핑해주면 어떨까요?

src/api/recruitments.ts의 fetchMyMissionJudgment에서 추론할 수 있도록 제네릭으로 명시해두었습니다! (일관성)

woowapark · 2024-07-09T09:04:03Z

혹시 prettier 이슈로 인해 스토리북 실행했을 때 오류가 나고 있는데 요건 제 로컬에서만 그럴까요?-?
(devleop 브랜치에서도 동일하네요)

woowahan-cron

공원🌳 적지 않은 내용인데도 정성껏 코드 리뷰를 해주셔서 감사합니다!

대개 받은 리뷰 중에서 가장 시간을 많이 할애했던 부분은 훅과 컴포넌트의 역할을 분명히 하는 것이었던 것 같습니다. 컴포넌트보다는 훅에 역할을 몰빵했었고 훅에서 훅을 불러오면 나중에 하나의 훅을 불러와서 쓰는 것이 재사용성 측면에서 유리하지 않을까 했었어요. 그런데 제안해 주신 것처럼 훅에서 훅을 불러 혼합하여 사용하는 것보다는 차라리 컴포넌트 자체에서 명확하게 분리하는 것이 테스트 코드가 깔끔하게 나오더라구요! 또한, 컴포넌트에서 라우팅 로직을 들고 있어야 테스트도 명확하게 분리되는 것을 깨닫는 시간이었던 것 같습니다 👍

제안해 주신 부분이 많다 보니 아래와 같이 주요 내용들에 대한 피드백을 정리해 봤습니다! 혹시라도 문의사항이나 궁금하신 점은 다시 남겨주시면 말씀드리도록 할게요!

React Fragment 대신 <div> 사용

상위 컴포넌트의 스타일 영향으로 인해 불가피하게 DOM 노드를 추가해야 했던 부분을 설명드렸습니다.

Button 컴포넌트의 disabled 속성

React.ButtonHTMLAttributes에서 이미 제공하는 disabled 속성을 명시적으로 추가한 부분을 제거하는 작업을 진행하였습니다.

MissionDetail 컴포넌트의 children 사용

현재 MyMissionItem 컴포넌트에서만 사용되고 있는 MissionDetail 컴포넌트의 children을 받는 부분을 제거하고, 해당 엘리먼트를 컴포넌트 내부로 옮겼습니다.

스타일 일관성

제안해주신 대로 스타일 선택자에서 child combinator 대신 descendant selector를 사용하여 일관성을 맞추는 작업을 진행하였습니다.

🔥 훅의 getter/setter 구분 작업

함수와 값을 구분하기 위해 getter로 명시한 부분을 유지하되, 일관성 있는 형식을 유지하도록 다른 훅들의 리턴값도 동일하게 수정하였습니다. 이 부분도 코멘트로 달아뒀는데 의견을 좀 더 여쭙고 싶습니다 🙂

테스트 명세 개선

테스트 코드의 명세에서는 프로덕션 코드에서 사용되는 변수나 함수명을 지칭하는 대신 스펙이 제대로 동작하는지 확인할 수 있도록 명세를 개선했습니다.
- 훅 테스트 명세 개선
- 컴포넌트 테스트 명세 개선 外

도메인 용어 일관성

'판정'이라는 용어를 '채점'으로 통일하는 작업을 진행했습니다.

라우팅 및 이벤트 핸들링 로직

훅에서 라우팅 로직을 제거하고, 컴포넌트에서 해당 로직을 처리하도록 수정하였습니다.

🔥 hooks 분리 작업

다른 이슈에서 진행하면 좋겠다고 제안해 주셨는데요🙂. 일단 이 부분은 지금 경로를 옮기게 되면 확인하시기 번거롭겠단 생각이 들어서 코드 리뷰가 끝나고 PR이 머지되기 전에 hooks/ 디렉터리로 옮겨두겠습니다. (테스트 코드 개선을 좀 많이 하게 됐는데 그 사이에 공원의 피드백 코멘트를 잃어버린 거 같아요 왜 안 보일까요😓)

스토리북 실행이 되지 않는 문제

develop 브랜치에서도 실행되지 않는 문제가 발생하셨다고 하셨는데요! 이 부분은 만나서 같이 해결해 보시죠! 저는 해당 브랜치에서도 이상없이 잘 동작하고 있는데 까닭이 무엇인지는 만나서 같이 살펴봐야 감을 잡을 수 있을 것 같습니다!

반영하고 다듬는 데 시간이 오래 걸렸는데 ✨좋은 제안 해 주셔서 감사합니다 👍

woowahan-cron · 2024-07-11T01:12:49Z

frontend/src/components/MyApplicationItem/RecruitmentDetail/RecruitmentDetail.tsx

@@ -9,7 +9,7 @@ type RecruitmentDetailProps = {

 const RecruitmentDetail = ({ children, startDate, endDate }: RecruitmentDetailProps) => {
  return (
-    <>
+    <div>


div가 시멘틱하진 않아서 저도 어떻게 사용해야 할지 고민되더라구요!
RecruitmentDetail 상위 컴포넌트인 MyMissionItem 컴포넌트의 title-container 스타일에서 flex-direction 영향을 받고 있어서 DOM 노드를 불가피하게 추가하게 되었습니닷!

이전까지는 아래의 버튼 배열을 display: relative의 속성에 별도의 하위 div가 있었고 이를 absolute로 우측 상단에 배치했었는데 이를 변경하는 과정에서 발생했습니다!

AS-IS

TO-BE

woowahan-cron · 2024-07-11T01:14:23Z

frontend/src/components/MyApplicationItem/MyMissionItem/useMissionJudgment.ts

+
+  return {
+    isJudgmentAvailable,
+


상태와 함수를 분리해 둔 의도였는데 굳이 2개라면 붙이는 편이 좀 더 낫겠군요 ㅎㅎ 반영해두겠습니다

woowahan-cron · 2024-07-11T01:16:43Z

frontend/src/components/@common/Button/Button.tsx

@@ -23,6 +25,7 @@ const Button = ({
      className={classNames(className, styles[variant], styles.button, {
        [styles.cancel]: cancel,
      })}
+      disabled={disabled}


네넵! 버튼을 커스텀해서 사용하고 있어서 명시적으로 표현하기 위해서였는데 ButtonProps 선언부에서는 굳이 disabled 속성을 표기해 줄 필요가 없겠군요! 그 부분만 적용해서 반영해보겠습니다.

woowahan-cron · 2024-07-11T01:27:39Z

frontend/src/components/MyApplicationItem/MissionDetail/MissionDetail.tsx

-      <JudgmentResultText judgment={judgment} />
+      <div className={styles["detail-status-container"]}>
+        <JudgmentResultText judgment={judgment} />
+        <div>{children}</div>


앗 그러네요! 훅으로 분리했기 때문에 오히려 지금 children으로 넘겨 둔 Element는 MissionDetail 컴포넌트 이하로 옮겨두는 것이 더 좋을 것 같아요!

woowahan-cron · 2024-07-11T01:35:13Z

frontend/src/components/MyApplicationItem/MissionDetail/MissionDetail.tsx

-      <JudgmentResultText judgment={judgment} />
+      <div className={styles["detail-status-container"]}>
+        <JudgmentResultText judgment={judgment} />
+        <div>{children}</div>


그리고 말씀하신 것처럼 지금은 MyMissionItem에서 한 번만 사용되고 있기 때문에 MyApplicationItem 이하 같은 레벨의 컴포넌트로 구성되어 있는데 이것을 MyMissionItem 컴포넌트 하위로 옮겨두는 것이 더 좋을 것 같다는 생각이 듭니다!

MyApplicationItem 컴포넌트

MissionDetail (현재 위치)

MyMissionItem

MissionDetail(수정 후 위치)

이전에 훅은 hooks/에 모여 있으니 특정 컴포넌트에서만 사용되는 훅은 다음번 PR에서 옮기는 것을 제안해 주셨는데 컴포넌트는 관계가 훅보다는 복잡한 것 같고 나중에 한꺼번에 작업하기 부담이 될 것 같다는 생각이 들었어요!

의존하고 있는 스타일이 있습니다. (ApplicationButtonStyles) -> 분리해도 되지만, 분리할 경우 동일한 속성 코드 선언의 반복으로 차후 수정 작업 시 일관성을 지키기 어렵겠다는 느낌이 드네요!

woowahan-cron · 2024-07-15T04:32:33Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      });
+    });
+
+    describe("미션 상태에 따른 판정 가능 여부", () => {


판정 -> 채점으로 용어 통일해두겠습니다! 👍

woowahan-cron · 2024-07-15T04:36:33Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useMission.test.ts

+        expect(result.current.getter.missionItem.title).toBe("Judged Mission");
+      });
+
+      it("fetchJudgmentMissionResult 실패 시 에러 메시지를 보여줘야 한다.", async () => {


너무 엔지니어 관점에서만 프로젝트를 바라보고 테스트 코드 작업을 했던 것 같아요 😓
어떤 역할을 하는 함수인지 쉽게 확인할 수 있으려면 구체적인 구현 내용보다는 컴포넌트의 스펙 문서 같은 문구로 써 두는 편이 로직을 이해하기에도 수월한 것 같아요! 비슷한 코멘트를 위에서 주셨는데 이 부분도 같이 수정해두겠습니다 👍💪

woowahan-cron · 2024-07-15T04:37:23Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useJudgment.test.ts

+      it("미션 상태가 제출 기간 사이에 제출이 불가능한 경우(UNSUBMITTABLE)일 때 false를 반환해야 한다", () => {
+        expectJudgmentAvailability({ status: MISSION_STATUS.UNSUBMITTABLE }, false);
+      });
+    });


아래의 다른 파일 코멘트에서도 비슷하게 피드백을 주셨던 것으로 기억해요! 이 부분도 같이 수정해두겠습니다 🙂
너무 엔지니어링 관점에서만 코드 작업을 하다 보니 테스트 명세도 구체적인 구현의 내용을 위주로 생각했던 것 같아요 🥲

woowahan-cron · 2024-07-15T06:15:32Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/useMission.test.ts

+        .mockResolvedValue({ ...createMissionItem(), title: "Judged Mission" }),
+    });
+    (useNavigate as jest.Mock).mockReturnValue(jest.fn());
+    (generatePath as jest.Mock).mockImplementation((path) => path);


처음에는 MyMissionItem 컴포넌트에서 로직이 너무 많고 복잡해서 로직만 따로 몰아둔 useMission 훅을 만들고 이를 컴포넌트에서 불러와서 사용하려고 했어요.

그런데 말씀하신 대로 useMission이 useMissionJudgment와 useRefresh 훅을 각각 의존하고 있어서 테스트 코드가 너무 지저분해진 것 같아요. 그래서 훅에서는 제대로 된 값과 상태를 도출하는지를 테스트하고 컴포넌트에서는 제대로 라우팅하고 있는지와 렌더링이 제대로 되고 있는지를 테스트하고 있지 못했네요😓 이번에도 변경사항이 좀 클 거 같습니다 ㅠㅠ

comment 수준으로 남겨주셨지만, 여기서 개선하지 않고 가면 나중에 의존성이 꼬인 훅을 풀기 더욱 어려워질 것 같아서 조언해 주신 대로 작업을 진행해볼게요!

useMission 훅에서 useMissionJudgment, useRefresh 훅의 의존성을 제거

useMissionJudgment와 useRefresh는 각각 사용하는 컴포넌트에서 import하도록 수정

useMission 훅에서는 의도한 값이 나오는지만 테스트 (라우팅 테스트 제거)

useMission 훅에서 라우팅 로직 제거

MyMissionItem 컴포넌트에서 라우팅 역할과 책임 부여

MyMissionItem.test.tsx 컴포넌트 테스트에서 라우팅 테스트

woowahan-cron · 2024-07-16T01:52:50Z

frontend/src/components/MyApplicationItem/MyMissionItem/__tests__/MyMissionItem.test.tsx

+      expect(screen.getByText(LABELS.APPLY)).toBeDisabled();
+    });
+
+    it("judgment가 없을 때 새로고침 버튼이 보이지 않아야 한다", () => {


훅이 훅을 부르고 컴포넌트의 역할까지 불러와서 사용하는 바람에 테스트 코드가 난잡해졌던 것 같아요.
그리고 MyMissionItem 컴포넌트를 테스트할 때도 테스트의 역할과 경우가 겹쳐 코드가 범람했던 것 같은데요 🥲
리뷰를 받지 않았다면 저도 미처 챙기지 못했을 부분이었던 것 같습니다!

훅의 역할을 정리하고 컴포넌트에서 분명하게 처리해야 할 역할을 다시 분리하다 보니까 테스트 코드가 명쾌해지기는 했는데..ㅋㅋ 다시 봐주셔야 할 거 같습니다🫠.

비즈니스 로직 그리고 값에 관련된 테스트는 훅에서 수행

렌더링과 라우팅에 관련된 테스트는 컴포넌트 테스트에서 수행
명확하게 분리 하여 다시 코드를 작성했어요! 이전보다는 확실히 테스트 케이스가 분명해져서 확인하시기는 수월하시리라 믿어봅니다 🙏

woowapark

크론~ 수정해주신 사항들 확인하였습니다! 👍
수정 사항이 적지 않으셨을텐데 고생하셨어요~! 🤼 🤼 🤼
훅 리턴에서의 getter 포맷에 대한 부분만 추가 의견 남겨두었는데요. 혹시 요 부분만 마저 확인해주실 수 있을까요? 이후 작업 일정이 있으실 것 같아서 우선 approve로 해둡니당!

wmakerjun · 2024-07-19T07:36:58Z

크론~! 제가 남긴 피드백도 수정하시느라 너무너무 고생하셨습니다!
저는 r로 남겨둔 코멘트가 따로 없으니, 공원 피드백 확인 하신 후 머지해주시면 될 것 같습니다!
고생하셨습니다 😆

woowahan-cron · 2024-07-24T00:39:56Z

공원, 준 리뷰해주셔서 감사합니다!
말씀해 주신 부분도 코멘트 남겨 드렸어요!

woowahan-pjs

고생하셨습니다. 👍

woowahan-cron added 9 commits July 2, 2024 14:25

refactor: 과제보기 컴포넌트 스타일 코드 개선 작업

bc579f7

refactor: 과제보기 컴포넌트 새로고침 버튼 개선 작업

86676d0

refactor: 새로고침 버튼 역할 분리

4c7e910

refactor: JudgmentButton 훅 분리 작업

5480ba6

refactor: 예제 테스트 실행 역할 분리

2cb8366

refactor: MyMissionItem 훅 분리

6f417f9

refactor: Apply, Refresh, Judgment 버튼 삭제 및 버튼(Button) 컴포넌트 사용

d218f86

refactor: 조건문 수정

604377f

refactor: 새로고침 가능여부 조건

b1ed5fb

woowahan-cron requested a review from woowapark July 2, 2024 05:29

woowahan-cron self-assigned this Jul 2, 2024

woowahan-cron requested a review from wmakerjun July 2, 2024 05:46

woowahan-cron commented Jul 2, 2024

View reviewed changes

woowahan-cron requested review from woowahan-pjs, woowahan-neo and woowabrie July 3, 2024 00:18

woowahan-cron added 4 commits July 3, 2024 14:31

chore: 명칭 통일을 위해 파일 이름 수정(useMissionJudgement -> useMissionJudgment)

989ea2f

test: useRefresh 훅 테스트

a8041dd

test: useJudgment 훅 테스트

15eb864

test: useMission 훅 테스트

a2e5ab5

woowahan-cron force-pushed the refactor/trimming-mission-item branch from fe57c89 to a2e5ab5 Compare July 3, 2024 06:07

woowahan-cron marked this pull request as ready for review July 3, 2024 06:36

woowahan-cron commented Jul 3, 2024

View reviewed changes

wmakerjun reviewed Jul 5, 2024

View reviewed changes

woowahan-cron added 6 commits July 8, 2024 14:02

refactor: useJudgment 코드 리뷰 반영하여 리팩터링

0c1a2ad

refactor: useMission.test 코드 리뷰 반영하여 리팩터링

c53dd81

refactor: useRefresh.test 코드 리팩터링

4eeaa4b

refactor: 테스트 코드 리팩터링

6cafd49

refactor: useMission 훅 조건부 리팩터링

85349b1

refactor: useRefresh 조건부 리팩터링

680bd89

woowahan-cron added 2 commits July 9, 2024 11:03

refactor: useRefresh 테스트 공통 함수 호출

f01dfd7

test: MyMissionItem 컴포넌트 테스트

a9fea3b

woowahan-cron force-pushed the refactor/trimming-mission-item branch from 45a16d3 to a9fea3b Compare July 9, 2024 03:51

woowahan-cron commented Jul 9, 2024

View reviewed changes

woowahan-cron requested a review from wmakerjun July 9, 2024 04:16

woowapark requested changes Jul 9, 2024

View reviewed changes

woowahan-cron added 12 commits July 11, 2024 10:14

refactor: useMissionJudgment 개행 제거

f38c0da

refactor: Button 컴포넌트 선언부 수정

e8914a5

refactor: MissionDetail 컴포넌트 속성 수정 (관심사 분리 작업)

28a44c0

refactor: 하위 컴포넌트 계층 이동 (종속 관계)

24437bc

refactor: MyApplicationItem module 스타일 일관성 맞추기 작업

0a01049

refactor: 응답 값 Judgment 추론

f1517c3

refactor: 버튼 이름 상수화 작업

bc9d6d7

refactor: 판정 -> 채점 용어 수정 작업

20924fd

refactor: 테스트 명세 문구 수정

04bcc21

refactor: useMission 의존성 분리 작업

b4277b4

test: useMission 훅 의존성 분리 작업 후 테스트 개선

f84fe3a

refactor: MyMissionItem 컴포넌트 테스트

5e46853

woowahan-cron commented Jul 16, 2024

View reviewed changes

woowahan-cron requested a review from woowapark July 16, 2024 02:25

woowapark approved these changes Jul 19, 2024

View reviewed changes

refactor: useMission 훅 리팩터링

c7213d2

rename: hook 파일 이동

acd9682

woowahan-pjs approved these changes Jul 24, 2024

View reviewed changes

woowahan-pjs changed the title ~~refactor(mission): refine the code of components associated with the MyMissionItem component~~ refactor(apply): redesign components of the my application page Jul 24, 2024

woowahan-pjs merged commit c591ee2 into develop Jul 24, 2024

woowahan-pjs deleted the refactor/trimming-mission-item branch July 24, 2024 06:55

-  const isJudgmentAvailable =
-    submitted &&
-    (judgment?.status !== JUDGMENT_STATUS.STARTED || isJudgmentTimedOut(judgment)) &&
-    !([MISSION_STATUS.ENDED, MISSION_STATUS.UNSUBMITTABLE] as MissionStatus[]).includes(status);
+   const isSubmitted = submitted;
+   const isJudgmentNotStartedOrTimedOut =
+     judgment?.status !== JUDGMENT_STATUS.STARTED || isJudgmentTimedOut(judgment);
+   const isMissionActive =
+     ![MISSION_STATUS.ENDED, MISSION_STATUS.UNSUBMITTABLE].includes(status);
+   const isJudgmentAvailable =
+     isSubmitted && isJudgmentNotStartedOrTimedOut && isMissionActive;

refactor(apply): redesign components of the my application page #761

refactor(apply): redesign components of the my application page #761

Conversation

woowahan-cron commented Jul 2, 2024 • edited Loading

해결하려는 문제가 무엇인가요?

어떻게 해결했나요?

어떤 부분에 집중하여 리뷰해야 할까요?

RCA 룰

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

800px 이상의 화면

800px 미만의 화면

Choose a reason for hiding this comment

wmakerjun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wmakerjun Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowahan-cron Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wmakerjun Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowahan-cron left a comment

Choose a reason for hiding this comment

반영한 내용

다음 이슈에서 추후 다룰 것

woowapark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AS-IS

TO-BE

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowapark commented Jul 9, 2024

woowahan-cron left a comment

Choose a reason for hiding this comment

스타일 일관성

🔥 훅의 getter/setter 구분 작업

테스트 명세 개선

도메인 용어 일관성

🔥 hooks 분리 작업

스토리북 실행이 되지 않는 문제

Choose a reason for hiding this comment

AS-IS

woowahan-cron commented Jul 2, 2024 •

edited

Loading

wmakerjun Jul 4, 2024 •

edited

Loading

woowahan-cron Jul 8, 2024 •

edited

Loading

wmakerjun Jul 4, 2024 •

edited

Loading