feat: edit photo note #49
Conversation
โ Deploy Preview for timely-pudding-5af14b ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
์ํฌ์ค๋ฃจ์ฌ์ง ๊ธฐ๋ก ํธ์ง ๊ธฐ๋ฅ์ ์ถ๊ฐํฉ๋๋ค. PhotoEditOverlay ์ปดํฌ๋ํธ์ usePhotoEdit ํ ์ ๋์ ํด ๋ฉ๋ชจยท์จ๋ฒยท์์น ํธ์ง UI๋ฅผ ์ ๊ณตํ๊ณ ํ์ด์ง์ ํตํฉํ์ผ๋ฉฐ, ๊ด๋ จ ์คํ์ผยทํ ยท๋ชฉ ๋ฐ์ดํฐ ๋ฐ ์ผ๋ถ UI ์ปดํฌ๋ํธ๋ฅผ ์ ๋ฐ์ดํธํ์ต๋๋ค. ๋ณ๊ฒฝ์ฌํญ
์ํ์ค ๋ค์ด์ด๊ทธ๋จsequenceDiagram
actor User
participant Page as ์ฌ์ง ์กฐํ ํ์ด์ง
participant Hook as usePhotoEdit ํ
participant Overlay as PhotoEditOverlay
participant MemoModal as MemoModal
participant AlbumModal as AlbumSelectOverlay
participant LocationModal as LocationSelectOverlay
User->>Page: "๊ธฐ๋ก ์์ ํ๊ธฐ" ํด๋ฆญ
Page->>Hook: openEditOverlay(photoId)
Hook-->>Page: isEditing = true
Page->>Overlay: ๋ ๋(AnimatePresence, photoId)
User->>Overlay: ๋ฉ๋ชจ ํธ์ง ํด๋ฆญ
Overlay->>MemoModal: open()
MemoModal->>User: ๋ฉ๋ชจ ์
๋ ฅ UI
User->>MemoModal: ์ ์ฅ
MemoModal-->>Overlay: memo ์ ์ถ
User->>Overlay: ์จ๋ฒ ์ ํ ํด๋ฆญ
Overlay->>AlbumModal: open()
AlbumModal->>User: ์จ๋ฒ ์ ํ UI
User->>AlbumModal: ์ ํ
AlbumModal-->>Overlay: albumId ์ ์ถ
User->>Overlay: ์์น ์ ํ ํด๋ฆญ
Overlay->>LocationModal: open()
LocationModal->>User: ์์น ์ ํ UI
User->>LocationModal: ์ ํ
LocationModal-->>Overlay: location ์ ์ถ
User->>Overlay: ์ ์ฅ ํด๋ฆญ
Overlay->>Hook: saveEdit({photoId,memo,albumId,location})
Hook-->>Page: onSave ์ฝ๋ฐฑ
Page->>Hook: closeEditOverlay()
Hook-->>Page: isEditing = false
Page->>User: ์ค๋ฒ๋ ์ด ๋ซํ
์ถ์ ์ฝ๋ ๋ฆฌ๋ทฐ ๋ ธ๋ ฅ๐ฏ 3 (์ค๊ฐ) | โฑ๏ธ ~25๋ถ ์
๐ฅ Pre-merge checks | โ 3 | โ 2โ Failed checks (1 warning, 1 inconclusive)
โ Passed checks (3 passed)
โ๏ธ Tip: You can configure your own custom pre-merge checks in the settings. โจ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @salmonco, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! ์ด PR์ ์ฌ์ฉ์๊ฐ ์ฌ์ง ์์ธ ํ์ด์ง์์ ์ฌ์ง์ ๋ฉ๋ชจ, ์จ๋ฒ, ์์น ์ ๋ณด๋ฅผ ์ง์ ํธ์งํ ์ ์๋ ๊ธฐ๋ฅ์ ๋์ ํฉ๋๋ค. ์๋ก์ด ํธ์ง ์ค๋ฒ๋ ์ด ์ปดํฌ๋ํธ์ ์ด๋ฅผ ๊ด๋ฆฌํ๋ ํ ์ ํตํด ๊ธฐ์กด ์ฌ์ง ์ ๋ณด์ ๋ํ ์์ ์ ๊ทผ์ฑ์ ๋์ด๊ณ , ์ฌ์ฉ์์๊ฒ ๋์ฑ ์ ์ฐํ ๋ฐ์ดํฐ ๊ด๋ฆฌ ๊ฒฝํ์ ์ ๊ณตํฉ๋๋ค. ๋ํ, ๊ฐ๋ฐ ํธ์์ฑ์ ์ํด ๋ชฉ(mock) ๋ฐ์ดํฐ ํธ๋ค๋ง ๋ก์ง๋ ๊ฐ์ ๋์์ต๋๋ค. Highlights
๐ง New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with ๐ and ๐ on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
์ฌ์ง ๊ธฐ๋ก์ ์์ ํ๋ ๊ธฐ๋ฅ์ ์ถ๊ฐํ PR ์ ๋ณด์์ต๋๋ค. ์ ๋ฐ์ ์ผ๋ก ์๋ก์ด PhotoEditOverlay ์ปดํฌ๋ํธ์ ๊ด๋ จ ํ
๋ค์ด ์ ๊ตฌํ๋์์ต๋๋ค. ์ฝ๋์ ์์ ์ฑ๊ณผ ๊ฐ๋
์ฑ์ ๋์ผ ์ ์๋ ๋ช ๊ฐ์ง ์ ์ ์ฌํญ์ ๋ฆฌ๋ทฐ์ ๋จ๊ฒผ์ต๋๋ค.
ํนํ, useMemoModal ํ
์ ์ํ ๋๊ธฐํ ๋ก์ง๊ณผ onSave ํธ๋ค๋ฌ์ ์ต์
๋ ์์ฑ ์ฒ๋ฆฌ ๋ถ๋ถ์ ์ ์ฌ์ ์ธ ๋ฒ๊ทธ๋ฅผ ์ ๋ฐํ ์ ์์ด ํ์ธ์ด ํ์ํด ๋ณด์
๋๋ค. ๋ํ, ์คํ์ผ ๊ฐ์ด๋์ ๋ฐ๋ผ ๋งค์ง ๋๋ฒ์ ํ๋์ฝ๋ฉ๋ ์์ ๊ฐ์ ํ
๋ง ์์๋ก ๋์ฒดํ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์๋ฉด ์ข๊ฒ ์ต๋๋ค.
apps/web/src/app/photo/[photoId]/_components/PhotoEditOverlay.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/photo/[photoId]/_components/PhotoEditOverlay.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const saveEdit = (data: EditData) => { | ||
| // TODO: ์ฌ์ง ์์ API ์ฐ๋ | ||
| console.log('Save photo edit:', data); |
There was a problem hiding this comment.
์ถํ API ์ฐ๋ ์์ ์ด์์
| svg path { | ||
| fill: ${({ theme }) => theme.colors.gray[100]}; | ||
| } |
There was a problem hiding this comment.
| <AnimatePresence> | ||
| {isEditing && editingPhotoId && ( | ||
| <PhotoEditOverlay | ||
| photoId={editingPhotoId} | ||
| onClose={closeEditOverlay} | ||
| onSave={saveEdit} | ||
| /> | ||
| )} | ||
| </AnimatePresence> |
There was a problem hiding this comment.
์ฌ์ง ์์ธ๋ณด๊ธฐ ํ๋ฉด์์ ์ค๋ฒ๋ ์ด๋ฅผ ๋์์ ์ฌ์ง ๊ธฐ๋ก ์์ ๊ฐ๋ฅํ๊ฒ๋ ํ์ด์
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
๐ค Fix all issues with AI agents
In `@apps/web/src/app/photo/`[photoId]/_components/PhotoEditOverlay.tsx:
- Around line 83-85: The code in PhotoEditOverlay.tsx uses non-null assertions
on selectedLocation.latitude! and selectedLocation.longitude!, which can throw
at runtime; update the logic where location is built (the object using
selectedLocation) to first check that selectedLocation.latitude and
selectedLocation.longitude are not null/undefined (e.g., guard with if/&& or a
helper like isFinite number check) and only set location to { latitude:
selectedLocation.latitude, longitude: selectedLocation.longitude } when both
values are valid; otherwise set location to undefined or handle the missing
coordinates appropriately to avoid using the ! operator.
- Around line 120-123: The isModified check incorrectly treats any non-null
selectedAlbum/selectedLocation as a change; update it to compare actual values:
compute whether memo.trim() differs from (photoDetail.description || '').trim(),
and for albums/locations compare selectedAlbum?.id (or selectedAlbum) against
photoDetail.albumId (or photoDetail.album?.id) and selectedLocation?.id against
photoDetail.locationId (or photoDetail?.id) instead of checking !== null; then
set isModified to true only if any of those comparisons show a real difference
(use optional chaining and null-safe checks like selectedAlbum?.id !==
photoDetail.albumId).
In `@apps/web/src/app/photo/`[photoId]/_hooks/usePhotoEdit.ts:
- Around line 24-28: The saveEdit function currently logs EditData to the
console which can leak sensitive info; remove the raw console.log and instead
either (a) guard debug logging behind a runtime dev check (e.g., only log when
process.env.NODE_ENV === 'development' or an app-level isDebug flag) or (b)
replace with a structured logger that redacts sensitive fields before emitting;
update the saveEdit implementation (refer to saveEdit, EditData, setIsEditing,
setEditingPhotoId) to stop unguarded console output and ensure any retained logs
explicitly omit or redact memo/location fields.
In `@apps/web/src/app/photo/`[photoId]/page.tsx:
- Line 87: The "์ฌ์ง ์ญ์ ํ๊ธฐ" MenuHeader.Item is missing an onClick handler; add an
onClick prop that calls a delete handler (e.g., handleDeletePhoto) similar to
the edit item at line 84. Implement handleDeletePhoto to call the photo deletion
API (or mutation) for the current photoId, handle errors, and then update UI
(navigate away or call router.refresh / revalidate) and close any open menus.
Attach this handler to the MenuHeader.Item (MenuHeader.Item
onClick={handleDeletePhoto}) and reuse any existing modal/confirmation logic if
present.
In `@apps/web/src/app/photo/add/note/_hooks/useMemoModal.ts`:
- Around line 3-16: The current useEffect guard with isInitialized prevents
subsequent changes to initialMemo from syncing into the hook; update the
synchronization so useMemoModal reacts to new initialMemo values instead of
permanently blocking after the first setโe.g., remove or adjust the
isInitialized ref and add a useEffect that compares initialMemo to memo/tempMemo
(or runs when isOpen changes) and sets memo and tempMemo when initialMemo
differs; touch the useMemoModal function, the isInitialized ref, and the
existing useEffect to ensure memo and tempMemo are updated on relevant
initialMemo updates.
In `@apps/web/src/mocks/handlers/photos/index.ts`:
- Around line 10-14: The dataResolver in createHandler(mockGetPhotoDetail...)
may produce id: NaN because Number(params.photoId) can return NaN for undefined
or string[]; update the resolver to safely extract and parse photoId (e.g., if
Array.isArray(params.photoId) take first element, if missing treat as fallback
id or return ์ฌ์ง_์์ธ_์กฐํ_์ฑ๊ณต) and only use the parsed numeric id when
Number.isFinite(parsedId); otherwise return the safe default object
(์ฌ์ง_์์ธ_์กฐํ_์ฑ๊ณต) or a fallback id so ์ฌ์ง_์์ธ_๋ชฉ๋ก lookup never receives NaN.
๐งน Nitpick comments (3)
apps/web/src/mocks/createHandler.ts (1)
6-28: dataResolver ๋ฐํ ํ์ ์ ์๋ต ํ์ ๊ณผ ์ฐ๊ฒฐํด ์ฃผ์ธ์.
ํ์ฌCreateHandlerOptions<JsonBodyType>๋ก ๊ณ ์ ๋์ด ์์ด resolver๊ฐ ์ค์ ์๋ต ์คํค๋ง์ ๋ค๋ฅธ ํํ๋ฅผ ๋ฐํํด๋ ํ์ ์ด ํต๊ณผํฉ๋๋ค.responseKey์ ๋์ํ๋responseItem.dataํ์ ๊ณผ ์ฐ๊ฒฐํ๋ฉด ์์ ์ฑ์ด ์ฌ๋ผ๊ฐ๋๋ค.โป๏ธ ์ ์ ๋ณ๊ฒฝ (ํ์ ์ฐ๊ฒฐ)
-export const createHandler = <T extends MockApiResponse<string, JsonBodyType>>( - config: T, - responseKey: ResponseKey<T>, - options?: CreateHandlerOptions<JsonBodyType> | ((request: Request) => string | null) -) => { +export const createHandler = < + T extends MockApiResponse<string, JsonBodyType>, + K extends ResponseKey<T> +>( + config: T, + responseKey: K, + options?: CreateHandlerOptions<T['response'][K]['data']> | ((request: Request) => string | null) +) => {apps/web/src/app/photo/[photoId]/_components/PhotoEditOverlay.tsx (2)
202-207: MapPreviewButton์ onClick ํธ๋ค๋ฌ๊ฐ ์์ต๋๋ค."์ง๋๋ทฐ ๋ฏธ๋ฆฌ๋ณด๊ธฐ" ๋ฒํผ์ ํด๋ฆญ ํธ๋ค๋ฌ๊ฐ ๊ตฌํ๋์ด ์์ง ์์ต๋๋ค. ํฅํ ๊ตฌํ ์์ ์ธ์ง ํ์ธ์ด ํ์ํฉ๋๋ค.
์ด ๊ธฐ๋ฅ ๊ตฌํ์ ์ถ์ ํ๊ธฐ ์ํ ์ด์๋ฅผ ์์ฑํ์๊ฒ ์ต๋๊น?
91-110: ์ธ๋ผ์ธ ์คํ์ผ์ styled-component๋ก ์ถ์ถํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.๋ก๋ฉ ์ํ์ ๋ฉ์ธ ์ค๋ฒ๋ ์ด์์ ์ ์ฌํ ์ธ๋ผ์ธ ์คํ์ผ์ด ๋ฐ๋ณต๋๊ณ ์์ต๋๋ค.
PhotoEditOverlay.styles.ts์ styled-component๋ก ์ถ์ถํ๋ฉด ์ผ๊ด์ฑ๊ณผ ์ ์ง๋ณด์์ฑ์ด ํฅ์๋ฉ๋๋ค.
apps/web/src/app/photo/[photoId]/_components/PhotoEditOverlay.tsx
Outdated
Show resolved
Hide resolved
| const isModified = | ||
| memo !== (photoDetail.description || '') || | ||
| selectedAlbum !== null || | ||
| selectedLocation !== null; |
There was a problem hiding this comment.
isModified ๋ก์ง์ด ์ค์ ๋ณ๊ฒฝ ์ฌ๋ถ๋ฅผ ์ ํํ ๋ฐ์ํ์ง ์์ต๋๋ค.
ํ์ฌ ๋ก์ง์ selectedAlbum !== null ๋๋ selectedLocation !== null์ธ ๊ฒฝ์ฐ ์์ ๋ ๊ฒ์ผ๋ก ํ๋จํฉ๋๋ค. ๊ทธ๋ฌ๋ ์ฌ์ฉ์๊ฐ ๊ธฐ์กด๊ณผ ๋์ผํ ์จ๋ฒ์ด๋ ์์น๋ฅผ ์ ํํด๋ ์์ ๋ ๊ฒ์ผ๋ก ํ์๋ฉ๋๋ค.
โป๏ธ ๊ฐ์ ์ ์
const isModified =
memo !== (photoDetail.description || '') ||
- selectedAlbum !== null ||
- selectedLocation !== null;
+ (selectedAlbum !== null && selectedAlbum.id !== photoDetail.albumId) ||
+ (selectedLocation !== null && selectedLocation.placeName !== photoDetail.address);๐ Committable suggestion
โผ๏ธ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isModified = | |
| memo !== (photoDetail.description || '') || | |
| selectedAlbum !== null || | |
| selectedLocation !== null; | |
| const isModified = | |
| memo !== (photoDetail.description || '') || | |
| (selectedAlbum !== null && selectedAlbum.id !== photoDetail.albumId) || | |
| (selectedLocation !== null && selectedLocation.placeName !== photoDetail.address); |
๐ค Prompt for AI Agents
In `@apps/web/src/app/photo/`[photoId]/_components/PhotoEditOverlay.tsx around
lines 120 - 123, The isModified check incorrectly treats any non-null
selectedAlbum/selectedLocation as a change; update it to compare actual values:
compute whether memo.trim() differs from (photoDetail.description || '').trim(),
and for albums/locations compare selectedAlbum?.id (or selectedAlbum) against
photoDetail.albumId (or photoDetail.album?.id) and selectedLocation?.id against
photoDetail.locationId (or photoDetail?.id) instead of checking !== null; then
set isModified to true only if any of those comparisons show a real difference
(use optional chaining and null-safe checks like selectedAlbum?.id !==
photoDetail.albumId).
| const saveEdit = (data: EditData) => { | ||
| // TODO: ์ฌ์ง ์์ API ์ฐ๋ | ||
| console.log('Save photo edit:', data); | ||
| setIsEditing(false); | ||
| setEditingPhotoId(null); |
There was a problem hiding this comment.
ํธ์ง ๋ฐ์ดํฐ console.log๋ ํ๋ก๋์
์์ ์ ๊ฑฐ/๊ฐ๋๊ฐ ํ์ํฉ๋๋ค.
๋ฉ๋ชจ/์์น๊ฐ ์ฝ์์ ๋จ์ผ๋ฉด ๋๋ฒ๊ทธ ๋
ธ์ด์ฆ + ๋ฏผ๊ฐ ์ ๋ณด ๋
ธ์ถ ์ฐ๋ ค๊ฐ ์์ต๋๋ค.
๐ ๏ธ ์ ์ ์์
- console.log('Save photo edit:', data);
+ if (process.env.NODE_ENV !== 'production') {
+ console.log('Save photo edit:', data);
+ }๐ Committable suggestion
โผ๏ธ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const saveEdit = (data: EditData) => { | |
| // TODO: ์ฌ์ง ์์ API ์ฐ๋ | |
| console.log('Save photo edit:', data); | |
| setIsEditing(false); | |
| setEditingPhotoId(null); | |
| const saveEdit = (data: EditData) => { | |
| // TODO: ์ฌ์ง ์์ API ์ฐ๋ | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.log('Save photo edit:', data); | |
| } | |
| setIsEditing(false); | |
| setEditingPhotoId(null); |
๐ค Prompt for AI Agents
In `@apps/web/src/app/photo/`[photoId]/_hooks/usePhotoEdit.ts around lines 24 -
28, The saveEdit function currently logs EditData to the console which can leak
sensitive info; remove the raw console.log and instead either (a) guard debug
logging behind a runtime dev check (e.g., only log when process.env.NODE_ENV ===
'development' or an app-level isDebug flag) or (b) replace with a structured
logger that redacts sensitive fields before emitting; update the saveEdit
implementation (refer to saveEdit, EditData, setIsEditing, setEditingPhotoId) to
stop unguarded console output and ensure any retained logs explicitly omit or
redact memo/location fields.
H0ngJu
left a comment
H0ngJu
left a comment
There was a problem hiding this comment.
๋น ๋ฅธ ์์ ๊ฐ์ฌํฉ๋๋ค ใ ใ !! ์๊ณ ํ์ จ์ต๋๋ค~!
๐ 1. ์ฐ๊ด ์ด์
๐ 2. ์์ ๋ด์ฉ
๐ 3. ์ฐธ๊ณ ์ฌํญ
ํผ๊ทธ๋ง
๐ 4. ๋ฆฌ๋ทฐ ์ค์ ์ฌํญ
๐งช 5. ํ ์คํธ
ํ ์คํธ ๋ฐฉ๋ฒ
2026-01-29.1.40.08.mov
Summary by CodeRabbit
์๋ก์ด ๊ธฐ๋ฅ
์คํ์ผ
โ๏ธ Tip: You can customize this high-level summary in your review settings.