taken lectures api/#35 #36

gahyuun · 2024-02-24T07:23:55Z

📌 작업 내용

구현 내용 및 작업 했던 내역

msw와 fetch를 사용하여 기이수 과목 리스트 api 연결

🤔 고민 했던 부분

원래 기이수 과목을 completed subjects라는 네이밍으로 가져가려 했습니다 하지만 백엔드에서 넘겨주는 값의 네이밍이 taken lectures였고 백엔드 네이밍과 다르면 좀 헷갈릴 것 같아서 taken lectures라고 했습니다
현재 ui 폴더에 taken lectures 폴더는 기이수 과목에 대한 도메인 컴포넌트가 존재할 예정입니다. 마이페이지 기이수 과목 리스트를 taken lecture list라고 결정했기에 결과 페이지에 있는 기이수 과목 리스트는 taken category lecture list 정도로 생각하고 있습니다
fetch에서 error 처리는 아직 진행하지 않았습니다
taken lecture title 컴포넌트를 생성했습니다. 이 컴포넌트를 도메인 컴포넌트로 분리할까, view 컴포넌트로 분리할까 고민을 했었는데 아래 이유로 도메인 컴포넌트로 분리했습니다

재사용이 되긴 해도 taken lecture 도메인을 가진 컴포넌트에서만 재사용이 된다
역할을 생각했을 때 도메인에 종속된다 ( 네이밍도 taken lecture 단어를 포함 시키지 않고 짓기 애매하다)

테이블 컴포넌트에 대한 리팩토링

table component에서 전달 받는 data의 타입을 (string||number)[][] => {id:number;[key:string]:string|number}[] 타입으로 변경했습니다

배열에서 객체로 바꾼 이유 => id 필드가 필요했고 id 필드만 따로 구분하기 위해서 객체로 변경했습니다
id 필드가 필요한 이유 => action button 클릭 시 해당 과목의 아이디가 필요합니다!

action button을 JSX Element가 아닌 JSX Element 를 return하는 함수로 변경했습니다

id 필드가 필요한 이유랑 같은 맥락인데 action button 클릭 시 해당 과목의 id (row에 존재하는 과목 정보의 아이디) 가 필요하고 이를 가져오기 위해 함수로 변경했습니다

의도한 건 아니지만 data의 타입을 객체로 변경한 후 parseTakenLecture 라는 함수는 불필요해져서 제거했습니다!

github-actions · 2024-02-24T07:25:31Z

🚀storybook: https://65ccb85d5afe55a024495bc0-cotzkskzqa.chromatic.com/

seonghunYang

수고하셨습니다. 작업하신 내용을 꼼꼼히 검토하였습니다.
몇 가지 의견과 제안을 남겼습니다. 확인하시고 의견을 부탁드립니다.

seonghunYang · 2024-02-24T16:47:50Z

app/business/taken-lectures/taken-lecture-list.query.ts

+  const response = await fetch(API_PATH.takenLectures);
+  const data = await response.json();
+  return data;


[comment]

현재 요구사항에는 에러 발생 시의 처리 방안이 포함되어 있지 않습니다. 이 부분에 대한 정의가 필요하므로, 회고 시간에 논의해봅시다!

seonghunYang · 2024-02-24T16:53:07Z

app/ui/view/molecule/list/list-root.tsx

+type ListRootProps = {
+  data: (string | number)[][];
+  render: (item: (string | number)[], index: number) => ReactNode;


[comment]

제네릭에서 string이랑 number로만 타입을 제한한 이유가 있을까요?

table column에는 string number 외 데이터 타입이 들어올 상황이 없을 것 같아서 제한을 시켰습니다!

seonghunYang · 2024-02-24T16:54:15Z

app/ui/taken-lectures/taken-lecture-title.tsx

[request change]

제 생각에는 현재 'taken-lecture-title' 컴포넌트의 분리 수준을 고려했을 때, 'taken-lecture-title' 컴포넌트가 view 컴포넌트로 더 적절해 보입니다. 이유는 다음과 같습니다:

'taken-lecture-title' 컴포넌트 내부에는 비즈니스 도메인 로직이 없습니다.

따라서 'taken-lecture-title' 컴포넌트는 현재 view 관심사만을 수행하고 있습니다.

view 관심사는 props를 받아 화면에 그려주는 presentational component를 뜻합니다.

'현재 분리 수준'이라고 말한 것은 '커스텀하기'와 '업데이트' 버튼에 도메인 로직이 추가될 수 있기 때문입니다. 만약 그렇게 된다면 'taken-lecture-title' 컴포넌트는 props로 버튼을 받는 것이 아니라, 내부에서 자체적으로 '커스텀하기'와 '업데이트' 버튼을 렌더링하게 됩니다. 해당 비즈니스 도메인 로직이 'taken-lecture-title' 컴포넌트에 위치한다면, 도메인 컴포넌트가 더 적절하다고 볼 수 있습니다.

결국 도메인 컴포넌트로 분리하기 위해서는 해당 버튼을 props로 받는 것이 아니라, 내부에서 렌더링하고 거기에 비즈니스 도메인 로직을 작성할 필요가 있어보이네요!

앗 도메인 로직이 포함되어있지 않고 렌더링만 한다는 관점에서는 view component가 적절할 것 같네요!
저는 도메인 컴포넌트의 정의를 도메인에 종속된다고 생각해서 그렇게 분리를 했습니다!
taken lecture title은 "기이수 과목에 대한 정보(제목)을 렌더링한다" 라는 역할로 정의할 수 있고 역할이 도메인에 종속 된다고 생각해서 도메인 컴포넌트로 분리했습니다
혹시 이에 대해서는 어떻게 생각하시나요??

좋은 의견이네요. "기이수 과목에 대한 정보(제목)을 렌더링한다"라는 역할로 정의하면 도메인 컴포넌트가 맞습니다. 하지만 제 의견으론 현재 taken lecture title 컴포넌트가 해당 역할을 충실히 수행하고 있다고 보이지는 않습니다. 이 부분에 대해 회고 시간에 더 이야기해 보면 좋을 것 같네요!

seonghunYang · 2024-02-24T17:00:17Z

app/ui/taken-lectures/taken-lecture-list.tsx

[comment]

도메인 컴포넌트가 적절하게 작성된 것 같습니다.

또한, 저는 도메인 컴포넌트에 대한 테스트가 반드시 필요하다고 생각합니다. 그래서 도메인 컴포넌트에 대한 테스트 작성을 제안하고자 하는데, 다른 분들의 의견은 어떠신가요?

도메인 컴포넌트에 대한 테스트가 반드시 필요하다고 생각하시는 이유를 여쭤볼 수 있을까요?

음.. 제가 전에 많이 고민했던 주제라 고민의 흐름을 남겨보겠습니다. 도움이 되었으면 좋겠네요 :)

첫 번째로 고려해야 할 중요한 질문은 "어떤 테스트 코드를 작성하면 가장 효과적일까?"입니다. '효과적'이란 가장 적절한 코드의 양으로 "애플리케이션이 의도대로 동작함을 검증"하는 테스트의 목적을 달성하는 것을 의미합니다. 테스트 코드를 작성하는 과정에서 모든 것에 대한 테스트 코드를 작성하는 것은 지루하고, 상당히 소모적이며 때론 불가능하다는 것을 알게 됩니다. 따라서, 어떻게 가장 적절한 테스트 코드를 작성할 것인지에 대한 고민이 필요합니다. 이 질문에 대한 답을 찾기 위해 여러 참고자료를 검토했습니다.

가장 먼저 살펴본 것은 마틴 파울러의 "테스트 피라미드"입니다. 간단히 말하면, 유닛 테스트 > 통합 테스트 > E2E 테스트의 순서로 테스트 코드를 작성하는 것입니다. FE 관점에서는 로직을 가진 훅과 컴포넌트가 유닛 테스트 단위라고 생각할 수 있습니다. 이에 대한 자세한 내용은 아래에 첨부된 글을 참고하시기 바랍니다.
원문: https://martinfowler.com/bliki/TestPyramid.html
번역: https://blog.voidmainvoid.net/451
(원문을 읽는 것을 추천드립니다. 참고로 마틴 파울러는 제가 가장 좋아하는 프로그래머 중 한명입니다)

하지만 "테스트 피라미드"에 대해 의문이 생겼습니다. 그 의문은 "FE에서도 이 '테스트 피라미드'가 유효한가?"입니다. 제 경험상 BE 관점에서는 테스트 피라미드가 유효합니다. 이는 도메인 > 애플리케이션 > 어댑터의 의존성 흐름이 유닛 테스트 > 통합 테스트 > E2E 테스트의 흐름과 일치하기 때문입니다. 그러나 FE에서는 그렇지 않습니다. 비즈니스 로직과 컴포넌트는 각기 다른 계층을 가지며, 둘은 반드시 결합해서 작동합니다. 즉, 로직과 컴포넌트가 결합되어 제대로 동작하는 것을 확인해야 "애플리케이션이 의도대로 작동하는 것을 검증"하는 목표를 달성할 수 있습니다. 하지만 엄밀히 말하면, 비즈니스 로직과 컴포넌트가 결합된 테스트(도메인 컴포넌트라고 할 수 있습니다)는 통합 테스트에 가깝습니다.

이 의문을 해결하기 위해 다양한 사람들의 의견을 수집했고, 이 문제가 나만의 문제가 아니라는 사실을 깨달았습니다.

요약하면, 마틴 파울러의 "테스트 피라미드"는 구식이며, 통합 테스트를 유닛 테스트보다 많이 작성하는 것이 더 현명하다는 주장입니다. 이 테스트 모델을 FE 분야에서는 "트로피 테스트"라고 부릅니다.

그 다음으로 트로피 테스트의 개념을 제안한 사람인 Kent C.Dodds의 글을 찾아 읽었습니다. 그의 글을 요약하면, 통합 테스트는 유닛 테스트보다 더 적은 코드로 애플리케이션의 의도대로 작동함을 더 잘 보장할 수 있기 때문에 통합 테스트를 작성해야 한다고 합니다. 해당 글을 아래에 첨부합니다.
Write tests. Not too many. Mostly integration: https://kentcdodds.com/blog/write-tests
("Not too many. Mostly integration"이라는 제목이 인상적입니다. 참고로, Kent C.Dodds는 유명 FE 개발자이며 RTL를 개발했습니다.)

정리하면 우리는 “애플리케이션이 의도대로 동작함”을 보장하기 위해서 테스트 코드를 작성해야합니다. 그러나 우리의 시간은 제한되어 있기 때문에 어떤 테스트에 시간을 투자할지 결정해야 합니다. 전통적인 "테스트 피라미드" 모형이 있지만, 이는 현대 애플리케이션과 프론트엔드에는 적합하지 않습니다. 이는 유닛테스트가 더 많은 테스트 코드를 작성하도록 요구하며, 프론트엔드에서는 유닛테스트 통과가 의도한 동작을 보장하지 않기 때문입니다(이는 로직과 컴포넌트가 결합된 프론트엔드의 특성 때문이라고 생각합니다). 이에 대한 대안으로, 통합 테스트를 작성하는 것을 제안합니다. 이를 개념적으로 트로피 테스트 모형이라 부르며, 우리 프로젝트에서는 도메인 컴포넌트에 대한 테스트가 적합하다고 판단했습니다.

(번외): 마틴 파울러가 테스트 피라미드에 대한 논란에 답한 글이 있습니다. 그는 사람들이 통합 테스트에 관한 오해를 가지고 있다고 지적하며, 자신의 생각에 따르면 통합 테스트는 개발자가 아닌 다른 사람이 작성한 테스트 코드로 테스트하는 경우를 가리킨다고 밝혔습니다. 그리고 사람들이 일반적으로 이야기하는 통합테스트 (즉, 트로피 테스트에 해당하는 통합테스트)는 자신이 정의한 sociable unittest에 해당하며, 일반적으로 언급되는 유닛테스트는 solitary unittest에 해당한다고 설명했습니다. 관심 있으시면 아래 링크 읽어보세요!

https://martinfowler.com/articles/2021-test-shapes.html

테스트를 해야 하는 이유에 대해 확인했습니다!
추가적으로 저는 궁금한게 있는데, 도메인 컴포넌트에 대한 테스트 작성이 언제 적합하다고 생각하나요?
저는 컴포넌트에 대한 비즈니스 로직을 다 구현한 후에 진행하는 것이 적합하다고 생각합니다!
이유는 아래와 같습니다

개발을 하면서 tdd를 할만큼 검증해야 예외 케이스가 많지 않을 것 같습니다

해당 부분에 대해서 저의 의견을 말씀드리면 테스트는 개발과 동시에 이뤄져야 한다고 생각합니다. 이 부분은 회고 시간에 마저 논의하도록 합시다!

yougyung · 2024-02-24T23:12:00Z

app/business/taken-lectures/taken-lecture-list.query.ts

+type lectureInfo = {
+  id: number;
+  year: string;
+  semester: string;
+  lectureCode: string;
+  lectureName: string;
+  credit: number;
+};


두 번 이상 사용되는 공통된 타입에 대해서 파일로 분리하는 것은 어떨까 제안드려요 :)

lecture info 타입은 taken lecture에서 한 번 사용되는데 두 번 이상 사용된다는 게 어떤 말씀이신지 조금 더 알려주실 수 있나요1?

app/ui/taken-lectures/taken-lecture-list.tsx에서 사용되고 있는

const parseTakenLectures = (subjects: TakenLectures['takenLectures']) =>

부분에서 착각한 것 같습니다. 해당 부분은 그대로 진행해도 좋을 것 같습니다. 죄송합니다 :)

yougyung · 2024-02-24T23:37:14Z

app/ui/taken-lectures/taken-lecture-title.tsx

taken-lecture-title이라는 네이밍이 적절하지않다는 생각이 듭니다. 해당 컴포넌트가 결과 페이지에서도 재사용될 것으로 예상되는데, 기이수과목 뿐만아니라 미이수과목을 표시할때도 함께 노출되기 때문입니다.

결과페이지에서 해당 컴포넌트를 재사용해서 노출할 것으로 예상되는 (현재 들은 학점/
기준 학점)정보가 미이수 과목 표시에서 사용되는 경우에도 기이수를 기준으로 노출 하고 있지만, 이는 기이수에 대한 title이 아니라 table에 대한 정보를 노출하는 것이라고 생각합니다.

따라서,

table-info 네이밍을 변경하고

반드시 해당 이름일 필요는 없고, table에 의존한 네이밍

view컴포넌트로 위치
시키는것이 적절하다고 생각합니다.

미이수 과목 표시에도 노출되는 걸 생각하지 못했네요! taken lecture에서 lecture로 변경하겠습니다!
table에 의존한 네이밍을 말씀하셨는데 아래 같은 경우에도 taken-lecture-title 컴포넌트는 사용될 예정입니다!
table에 의존한 네이밍을 가져가기엔 애매하다고 생각하는데 혹시 어떻게 생각하시나요!? ( 저번에 카페에서 저희끼리 얘기했던 거,,,,,, 기억하시나여??)

모호한 부분이 있다는 의견에 대해서 공감합니다. table에서 의존성을 가져가긴 어려울 것 같네요.

저번에 카페에서 저희끼리 얘기했던 거,,,,,, 기억하시나여??

심심한 사과의 말씀...을 전하며, 정확한 대화의 흐름은 기억이 나지않지만 당시에도 비슷한 문제로 이야기를 나눴던 것으로 기억하고 있습니다. lecture-info 등 여러 후보군이 있었던 것으로 기억하는데 해당 네이밍은 내일 회의때 이야기해봐도 좋을 것 같습니다 !

github-actions · 2024-02-26T08:29:54Z

🚀storybook: https://65ccb85d5afe55a024495bc0-mxiftbdeys.chromatic.com/

github-actions · 2024-02-27T11:39:10Z

🚀storybook: https://65ccb85d5afe55a024495bc0-xfmkyaejxw.chromatic.com/

github-actions · 2024-02-27T13:26:04Z

🚀storybook: https://65ccb85d5afe55a024495bc0-bldkebbebp.chromatic.com/

gahyuun · 2024-02-27T13:32:28Z

@seonghunYang @yougyung
변경 사항 반영해서 푸시했습니다! 확인 부탁드립니다:)

taken lecture title domain component를 label container라는 view component로 변경
기이수 과목 리스트 테스트 코드 작성
taken lecture 폴더 네이밍을 lecture로 변경 ( 미이수 과목에 대해서도 포함될 것 같기 때문입니다)

jest에서 next js server component 를 지원하지 않는 것 같더라구요
revenue chart test code와 아래 링크를 참고해서 테스트 코드를 구현했습니다
https://stackoverflow.com/questions/75729282/testing-an-async-server-component-with-jest-in-next-13

seonghunYang

수고하셨습니다. comment 남겼으니 확인 부탁드려요.

seonghunYang · 2024-02-27T14:44:16Z

app/ui/lecture/taken-lecture-list.tsx

@@ -0,0 +1,15 @@
+import { fetchTakenLectures } from '@/app/business/lecture/taken-lecture-list.query';
+import { Table } from '../view/molecule/table';
+import TakenLectureTitle from './taken-lecture-title';


[comment]

파일이랑 컴포넌트 이름이 TakenLectureLabel로 아직 변경되지 않는 것 같네요!

수정했습니다! 알려주셔서 감사합니다
rename: taken lecture title -> taken lecture label

seonghunYang · 2024-02-27T14:46:58Z

app/__test__/ui/lecture/taken-lecture-list.test.tsx

+describe('Taken lecture list', () => {
+  it('기이수 과목 리스트를 보여준다.', async () => {
+    render(await TakenLectureList());
+    expect(await screen.findByTestId('table-data'));


[comment]

test-id로 테스트 작성하신 이유가 있을까요?

기이수 과목 리스트가 데이터를 호출해서 잘 렌더링 하는 지 확인해야 했습니다!
다른 테스트처럼 findByText 를 이용하기엔 어떤 데이터가 들어올 지 모르기 때문에 test id를 사용했습니다!
데이터가 문제 없이 호출되면 table이 정상적으로 렌더링 될 것이라고 판단했기 때문입니다~!

네, 감사합니다. 말씀하신대로 test-id가 적절해 보이네요!

아래는 테스트에서 요소를 선택하는 베스트 프랙티스를 정리한 문서입니다. 참고하시면 도움이 될 것 같아 공유합니다
https://docs.cypress.io/guides/references/best-practices#Selecting-Elements

github-actions · 2024-02-27T17:07:15Z

🚀storybook: https://65ccb85d5afe55a024495bc0-tixikokpcd.chromatic.com/

yougyung

수정사항 확인했습니다. 수고하셨어요 👍

yougyung · 2024-02-27T17:38:23Z

app/business/lecture/taken-lecture-list.query.ts

@@ -0,0 +1,20 @@
+import { API_PATH } from '../api-path';
+
+type lectureInfo = {


컨벤션 확인 부탁드립니다 :)

수정했습니다! 알려주셔서 감사합니다:)
chore: modifying type name to Pascal Case

github-actions · 2024-02-27T18:00:43Z

🚀storybook: https://65ccb85d5afe55a024495bc0-oimomfqbux.chromatic.com/

seonghunYang

수고하셨습니다.

seonghunYang · 2024-02-28T09:40:15Z

app/__test__/ui/lecture/taken-lecture-list.test.tsx

+describe('Taken lecture list', () => {
+  it('기이수 과목 리스트를 보여준다.', async () => {
+    render(await TakenLectureList());
+    expect(await screen.findByTestId('table-data'));


네, 감사합니다. 말씀하신대로 test-id가 적절해 보이네요!

아래는 테스트에서 요소를 선택하는 베스트 프랙티스를 정리한 문서입니다. 참고하시면 도움이 될 것 같아 공유합니다
https://docs.cypress.io/guides/references/best-practices#Selecting-Elements

gahyuun added 11 commits February 23, 2024 16:44

remove: delete table root page

2b3b861

remove: sample stories

45404ad

refactor: add list row key prop

61e8442

chore: change table type

ab4b204

feat: implement taken lecture list query

fe690cf

feat: implement taken lecture list mock data

03877b3

style: change table width style

1c42387

feat: implement taken lecture list component

4e1da15

feat: implement taken lecture title component

4005d3f

chore: merge branch main

26a7685

chore: interface -> type

0a1a557

gahyuun self-assigned this Feb 24, 2024

gahyuun requested review from yougyung and seonghunYang as code owners February 24, 2024 07:23

seonghunYang requested changes Feb 24, 2024

View reviewed changes

yougyung previously approved these changes Feb 24, 2024

View reviewed changes

refactor: change table data type

8a218ab

gahyuun dismissed yougyung’s stale review via 8a218ab February 26, 2024 08:28

rename: taken lecture -> lecture

5aa2967

gahyuun added 4 commits February 27, 2024 21:03

feat: implement label container component

45eae33

refactor: change taken lecture label role

5f3b5f6

chore: fix next link import

6d2461e

test: implement taken lecture list rendering test code

be454f7

seonghunYang reviewed Feb 27, 2024

View reviewed changes

rename: taken lecture title -> taken lecture label

ca6c51d

yougyung reviewed Feb 27, 2024

View reviewed changes

chore: modifying type name to Pascal Case

aa0c3aa

yougyung approved these changes Feb 27, 2024

View reviewed changes

seonghunYang approved these changes Feb 28, 2024

View reviewed changes

gahyuun merged commit 9d90007 into main Feb 29, 2024
3 checks passed

gahyuun deleted the completed-subjects-api/#35 branch February 29, 2024 13:35

		@@ -0,0 +1,20 @@
		import { API_PATH } from '../api-path';

		type lectureInfo = {

taken lectures api/#35 #36

taken lectures api/#35 #36

Conversation

gahyuun commented Feb 24, 2024 • edited Loading

📌 작업 내용

🤔 고민 했던 부분

github-actions bot commented Feb 24, 2024

seonghunYang 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

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

yougyung Feb 24, 2024 • edited Loading

Choose a reason for hiding this comment

seonghunYang Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

seonghunYang Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seonghunYang Feb 26, 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

Choose a reason for hiding this comment

github-actions bot commented Feb 26, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

gahyuun commented Feb 27, 2024

seonghunYang 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 27, 2024

yougyung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 27, 2024

seonghunYang left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gahyuun commented Feb 24, 2024 •

edited

Loading

yougyung Feb 24, 2024 •

edited

Loading

seonghunYang Feb 25, 2024 •

edited

Loading

seonghunYang Feb 25, 2024 •

edited

Loading

seonghunYang Feb 26, 2024 •

edited

Loading

seonghunYang left a comment •

edited

Loading