-
Notifications
You must be signed in to change notification settings - Fork 4
[Refactor] experience-detail 구조 정리 및 내부 API 범위 축소 #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e87a9e0
936647b
0b9aaa0
7287fe0
8763a8b
77a6cfc
7219d53
d028bb7
ace0d11
42c6251
7adcf48
c82f429
6808eb4
381f218
561f754
6b447f6
cb8e344
248aee2
68f98fb
bf52d76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,103 +1,15 @@ | ||
| export { DatePicker } from "./ui/date-picker/date-picker"; | ||
| export { ExperienceForm } from "./ui/experience-form/experience-form"; | ||
| export { ExperienceViewer } from "./ui/experience-viewer/experience-viewer"; | ||
| export { ExperienceAlertRenderer } from "./ui/experience-alert-renderer/experience-alert-renderer"; | ||
|
|
||
| export { | ||
| postExperience, | ||
| usePostExperience, | ||
| type PostExperienceResponse, | ||
| } from "./api/use-post-experience.query"; | ||
| export { useGetExperienceDetail } from "./api/use-get-experience-detail.query"; | ||
|
|
||
| export { | ||
| getExperienceDetail, | ||
| useGetExperienceDetail, | ||
| type GetExperienceDetailResponse, | ||
| } from "./api/use-get-experience-detail.query"; | ||
|
|
||
| export { | ||
| patchExperience, | ||
| usePatchExperience, | ||
| } from "./api/use-patch-experience.query"; | ||
|
|
||
| export { | ||
| deleteExperience, | ||
| useDeleteExperience as useDeleteExperienceMutation, | ||
| } from "./api/use-delete-experience.query"; | ||
|
|
||
| export { | ||
| patchExperienceDefault, | ||
| usePatchExperienceDefault, | ||
| type PatchDefaultResponse, | ||
| } from "./api/use-patch-experience-default.query"; | ||
|
|
||
| export { | ||
| useExperienceDetailStore, | ||
| initialDraft, | ||
| } from "./store/experience.store"; | ||
|
|
||
| export { | ||
| useExperienceMode, | ||
| useExperienceCurrent, | ||
| useExperienceDraft, | ||
| useDefaultExperienceId, | ||
| useExperienceActions, | ||
| useIsDraftDefault, | ||
| useDefaultButtonLabel, | ||
| useShowEditDeleteButtons, | ||
| useShowSubmitButton, | ||
| useCurrentExperienceId, | ||
| } from "./store/use-experience-hooks"; | ||
|
|
||
| export { | ||
| useExperienceSubmit, | ||
| useExperienceHeaderActions, | ||
| useDeleteExperience, | ||
| } from "./model/use-actions"; | ||
|
|
||
| export { useExperienceDateField } from "./model/use-experience-date-field"; | ||
|
|
||
| export { formatDateDash, parseYMD } from "@/shared/lib/format-date"; | ||
|
|
||
| export { | ||
| showExperienceError, | ||
| showExperienceSuccess, | ||
| showExperienceInfo, | ||
| showExperienceWarning, | ||
| showValidationError, | ||
| showSaveError, | ||
| showDeleteError, | ||
| showDefaultSettingError, | ||
| showSaveSuccess, | ||
| showDeleteSuccess, | ||
| useExperienceAlerts, | ||
| useExperienceAlertActions, | ||
| } from "./model/use-alert"; | ||
| export { useExperienceMode } from "./store/use-experience-hooks"; | ||
|
|
||
| export { useLeaveConfirm } from "./model/use-leave-confirm"; | ||
| export { initExperienceDetail } from "./model/use-init-experience-detail"; | ||
| export { applyExperienceDetailFromApi } from "./model/use-hydrate-experience"; | ||
|
|
||
| export { | ||
| useInitExperienceDetail, | ||
| useResetExperienceDetail, | ||
| initExperienceDetail, | ||
| resetExperienceDetail, | ||
| } from "./model/use-init-experience-detail"; | ||
|
|
||
| export { | ||
| useHydrateExperienceFromApi, | ||
| hydrateExperienceFromApi, | ||
| } from "./model/use-hydrate-experience"; | ||
|
|
||
| export { toExperienceEntity } from "./lib/to-experience-entity"; | ||
|
|
||
| export { validateExperienceDraft } from "./lib/validation"; | ||
|
|
||
| export type { | ||
| ExperienceMode, | ||
| ExperienceType, | ||
| ExperienceUpsertBody, | ||
| ExperienceEntity, | ||
| DefaultExperience, | ||
| } from "./types/experience-detail.types"; | ||
| export type { ExperienceMode } from "./types/experience-detail.types"; | ||
|
|
||
| export { EXPERIENCE_MESSAGES, DEFAULT_BUTTON_LABELS } from "./config/messages"; | ||
| export { EXPERIENCE_MESSAGES } from "./config/messages"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,90 +16,74 @@ interface ExperienceAlertState { | |
| actions: { | ||
| show: (variant: AlertVariant, title: string, description: string) => void; | ||
| close: (id: string) => void; | ||
| closeAll: () => void; | ||
| }; | ||
| } | ||
|
|
||
| let alertIdCounter = 0; | ||
|
|
||
| export const useExperienceAlertStore = create<ExperienceAlertState>( | ||
| (set, get) => ({ | ||
| alerts: [], | ||
| actions: { | ||
| show: (variant, title, description) => { | ||
| const { alerts } = get(); | ||
| const isDuplicate = alerts.some( | ||
| (a) => | ||
| a.variant === variant && | ||
| a.title === title && | ||
| a.description === description | ||
| ); | ||
| if (isDuplicate) return; | ||
|
|
||
| const id = `exp-alert-${++alertIdCounter}`; | ||
| set((state) => ({ | ||
| alerts: [...state.alerts, { id, variant, title, description }], | ||
| })); | ||
| }, | ||
| close: (id) => { | ||
| set((state) => ({ | ||
| alerts: state.alerts.filter((a) => a.id !== id), | ||
| })); | ||
| }, | ||
| closeAll: () => { | ||
| set({ alerts: [] }); | ||
| }, | ||
| const useExperienceAlertStore = create<ExperienceAlertState>((set, get) => ({ | ||
| alerts: [], | ||
| actions: { | ||
| show: (variant, title, description) => { | ||
| const { alerts } = get(); | ||
|
Comment on lines
+24
to
+28
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 modal과 마찬가지로
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 포인트 짚어주셔서 감사합니다 🙇 말씀해주신 것처럼 현재 다만 이번 PR에서는 “기존 동작을 유지한 상태에서 내부 구조를 정리하는 것”을 목표로 잡고 작업을 진행하다 보니, alert를 shared 레벨로 올리는 작업까지 포함하기에는 범위가 다소 커진다고 판단했습니다. 그래서 이번 단계에서는 experience-detail 내부에서 불필요한 abstraction을 줄이고 export 범위를 정리하는 수준까지만 반영하였고, 말씀해주신 shared/store 분리 및 전역 renderer 구조로의 개선은 다음 스프린트에서 조금 더 구조적으로 고민해보겠습니다 ㅠ!! 좋은 방향 제안해주셔서 감사합니다! 👍🖤 |
||
| const lastAlert = alerts[alerts.length - 1]; | ||
|
|
||
| const isDuplicate = | ||
| lastAlert != null && | ||
| lastAlert.variant === variant && | ||
| lastAlert.title === title && | ||
| lastAlert.description === description; | ||
|
|
||
| if (isDuplicate) return; | ||
|
|
||
| const id = `exp-alert-${++alertIdCounter}`; | ||
| set((state) => ({ | ||
| alerts: [...state.alerts, { id, variant, title, description }], | ||
| })); | ||
| }, | ||
| }) | ||
| ); | ||
|
|
||
| export const showExperienceError = (message: string) => { | ||
| const { show } = useExperienceAlertStore.getState().actions; | ||
| show("error", "오류", message); | ||
| }; | ||
|
|
||
| export const showExperienceSuccess = (message: string, title = "완료") => { | ||
| const { show } = useExperienceAlertStore.getState().actions; | ||
| show("success", title, message); | ||
| }; | ||
|
|
||
| export const showExperienceInfo = (message: string) => { | ||
| const { show } = useExperienceAlertStore.getState().actions; | ||
| show("info", "안내", message); | ||
| }; | ||
|
|
||
| export const showExperienceWarning = (message: string) => { | ||
| const { show } = useExperienceAlertStore.getState().actions; | ||
| show("warning", "주의", message); | ||
| close: (id) => { | ||
| set((state) => ({ | ||
| alerts: state.alerts.filter((a) => a.id !== id), | ||
| })); | ||
| }, | ||
| }, | ||
| })); | ||
|
|
||
| const showAlert = ( | ||
| variant: AlertVariant, | ||
| title: string, | ||
| description: string | ||
| ) => { | ||
| useExperienceAlertStore.getState().actions.show(variant, title, description); | ||
| }; | ||
|
|
||
| export const showValidationError = (title: string, description: string) => { | ||
| const { show } = useExperienceAlertStore.getState().actions; | ||
| show("error", title, description); | ||
| showAlert("error", title, description); | ||
| }; | ||
|
|
||
| export const showSaveError = () => { | ||
| showExperienceError(EXPERIENCE_MESSAGES.API.SAVE_FAILED); | ||
| showAlert("error", "오류", EXPERIENCE_MESSAGES.API.SAVE_FAILED); | ||
| }; | ||
|
|
||
| export const showDeleteError = () => { | ||
| showExperienceError(EXPERIENCE_MESSAGES.API.DELETE_FAILED); | ||
| showAlert("error", "오류", EXPERIENCE_MESSAGES.API.DELETE_FAILED); | ||
| }; | ||
|
|
||
| export const showDefaultSettingError = () => { | ||
| showExperienceError(EXPERIENCE_MESSAGES.API.DEFAULT_SETTING_FAILED); | ||
| showAlert("error", "오류", EXPERIENCE_MESSAGES.API.DEFAULT_SETTING_FAILED); | ||
| }; | ||
|
|
||
| export const showSaveSuccess = (title?: string) => { | ||
| showExperienceSuccess(EXPERIENCE_MESSAGES.SUCCESS.SAVED, title); | ||
| showAlert("success", title ?? "완료", EXPERIENCE_MESSAGES.SUCCESS.SAVED); | ||
| }; | ||
|
|
||
| export const showDeleteSuccess = () => { | ||
| showExperienceSuccess(EXPERIENCE_MESSAGES.SUCCESS.DELETED); | ||
| showAlert("success", "완료", EXPERIENCE_MESSAGES.SUCCESS.DELETED); | ||
| }; | ||
|
Comment on lines
+66
to
83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 오류/완료 타이틀 문자열은 상수로 추출해 중복을 줄여주세요. 동일 리터럴이 여러 곳에 반복되어 문구 변경 시 누락 위험이 있습니다. ♻️ 제안 diff let alertIdCounter = 0;
+const ERROR_TITLE = "오류";
+const SUCCESS_TITLE = "완료";
@@
export const showSaveError = () => {
- showAlert("error", "오류", EXPERIENCE_MESSAGES.API.SAVE_FAILED);
+ showAlert("error", ERROR_TITLE, EXPERIENCE_MESSAGES.API.SAVE_FAILED);
};
@@
export const showDeleteError = () => {
- showAlert("error", "오류", EXPERIENCE_MESSAGES.API.DELETE_FAILED);
+ showAlert("error", ERROR_TITLE, EXPERIENCE_MESSAGES.API.DELETE_FAILED);
};
@@
export const showDefaultSettingError = () => {
- showAlert("error", "오류", EXPERIENCE_MESSAGES.API.DEFAULT_SETTING_FAILED);
+ showAlert("error", ERROR_TITLE, EXPERIENCE_MESSAGES.API.DEFAULT_SETTING_FAILED);
};
@@
export const showSaveSuccess = (title?: string) => {
- showAlert("success", title ?? "완료", EXPERIENCE_MESSAGES.SUCCESS.SAVED);
+ showAlert("success", title ?? SUCCESS_TITLE, EXPERIENCE_MESSAGES.SUCCESS.SAVED);
};
@@
export const showDeleteSuccess = () => {
- showAlert("success", "완료", EXPERIENCE_MESSAGES.SUCCESS.DELETED);
+ showAlert("success", SUCCESS_TITLE, EXPERIENCE_MESSAGES.SUCCESS.DELETED);
};As per coding guidelines, "Use UPPER_SNAKE_CASE for constants (e.g., 🤖 Prompt for AI Agents |
||
|
|
||
| export const useExperienceAlerts = () => | ||
| useExperienceAlertStore((s) => s.alerts); | ||
|
|
||
| export const useExperienceAlertActions = () => | ||
| useExperienceAlertStore((s) => s.actions); | ||
| useExperienceAlertStore((s) => s.actions); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,9 @@ | ||
| import { useCallback } from "react"; | ||
|
|
||
| import { toExperienceEntity } from "@/features/experience-detail/lib/to-experience-entity"; | ||
| import { useExperienceDetailStore } from "@/features/experience-detail/store/experience.store"; | ||
|
|
||
| import type { GetExperienceDetailResponse } from "@/features/experience-detail/api/use-get-experience-detail.query"; | ||
|
|
||
| export const useHydrateExperienceFromApi = () => { | ||
| const { setCurrent, setDefaultExperienceId, hydrateDraftFromCurrent } = | ||
| useExperienceDetailStore((s) => s.actions); | ||
|
|
||
| const hydrate = useCallback( | ||
| (data: GetExperienceDetailResponse) => { | ||
| const entity = toExperienceEntity(data); | ||
|
|
||
| setCurrent(entity); | ||
| setDefaultExperienceId(entity.isDefault ? entity.experienceId : null); | ||
| hydrateDraftFromCurrent(); | ||
| }, | ||
| [setCurrent, setDefaultExperienceId, hydrateDraftFromCurrent] | ||
| ); | ||
|
|
||
| return hydrate; | ||
| }; | ||
|
|
||
| export const hydrateExperienceFromApi = (data: GetExperienceDetailResponse) => { | ||
| export const applyExperienceDetailFromApi = (data: GetExperienceDetailResponse) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 이제 훅이 아닌 만큼 모듈명도 함께 정리하는 편이 좋겠습니다. export는 일반 함수로 바뀌었는데 파일명이 여전히 🤖 Prompt for AI Agents |
||
| const { actions } = useExperienceDetailStore.getState(); | ||
| const entity = toExperienceEntity(data); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index.ts는 지금처럼 features 외부에서 참조하는 파일에 대해서만 export하고 그 외는 내부에서 상대경로로 참조하는 것으로 충분하다고 생각합니다. 해당 방향이 저희가 정리한 컨벤션이기도 하고요!한 가지 현재 단계에서 수정하면 좋을 점은
experience-detail-page.tsx에서 이미index.ts에서 mode에 대한 type을 호출하여 사용하고 있는데 한번 더 props 타입으로 감쌀 필요는 없다고 생각해요!ExperienceMode로 대체해서 사용해도 충분하다고 생각합니다!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그러네요..! 불필요한 wrapping이 한 단계 더 생긴 셈이라 말씀해주신 방향대로 ExperienceMode를 직접 사용하는 형태로 수정해보겠습니다 :)