Skip to content

일기 작성 시 미연결된 추억사진 diary 연결 처리 - #49

Merged
nariming merged 1 commit into
developfrom
fix/diary-memory-photo-link
Aug 7, 2026
Merged

nariming merged 1 commit into
developfrom
fix/diary-memory-photo-link

Conversation

@nariming

@nariming nariming commented Aug 6, 2026 •

Copy link
Copy Markdown
Member

📌 관련 이슈

  • close #00

🔎 What is this PR?

  • Diary API 명세서 참고 (별도 이슈 없이 코드 리뷰 중 발견하여 수정한 버그 수정 PR입니다)

✨ Changes

  • 투데이 추억 저장 모드에서 저장한 사진(MemoryPhoto)이 diary와 연결되지 않아, 일기 사진 조회(DIA-004)와 날짜별 추억 조회(DIA-002)의 사진 목록이 항상 빈 배열로 반환되는 버그를 발견해 수정했습니다.
  • MemoryPhotoRepository에 diary와 아직 연결되지 않은 사진을 찾는 findAllByCourse_CourseIdAndDiaryIsNull 쿼리를 추가하고, DiaryService.writeDiary()에서 일기 작성 시점에 미연결 사진을 해당 일기로 연결하도록 했습니다.
  • 추가로 피그마 화면(기록 탭)을 확인한 결과, 추억 사진은 코스 멤버 전원이 함께 보는 공용 사진(같은 사진에 여러 멤버가 메모를 남김)으로 설계되어 있었습니다. 기존 getPhotosByCourse는 요청자 본인의 diary에 연결된 사진만 보여주는 구조라, 다른 멤버가 먼저 일기를 쓰면 그 뒤에 조회하는 멤버는 사진이 안 보이는 문제가 있었습니다.
    이를 diary 소유 여부가 아닌 코스 멤버십 기준으로 접근을 검증하고, 사진도 course 단위로 조회하도록 수정했습니다 (CourseMemberRepository 활용).
  • DiaryServiceTest에 관련 케이스 전부 추가 (미연결 사진 연결, 연결 대상 없음, 완료되지 않은 코스 예외, 코스 멤버는 조회 가능, 비멤버는 접근 거부).
  • CourseQueryValidationTest에 신규 쿼리 실행 검증 라인 추가.

📷 Result

  • DiaryServiceTest, CourseQueryValidationTest 전체 통과 확인

💬 To. Reviewer

  • "이 사진이 어느 일기 작성 시점에 확정됐는지" 메타데이터로는 의미가 있을 것 같아 diary_id 연결 로직(linkUnlinkedMemoryPhotos)은 그대로 유지했는데 불필요하다고 판단되 제거해도 될 것 같습니다

✅ 체크 리스트

  • base 브랜치(develop 또는 main) 최신 상태 pull 및 충돌 확인 완료
  • Reviewers 설정
  • Assignees 설정
  • Labels 설정

Summary by CodeRabbit

  • New Features

    • Course members can view all photos associated with a course, regardless of diary ownership.
    • Course photos are consistently displayed in photo-order sequence.
  • Bug Fixes

    • Restricted course photo and memory access to joined members.
    • Prevented diary creation for incomplete courses.
    • Removed diary-specific photo association requirements.
  • Tests

    • Added coverage for shared photo access, membership restrictions, diary updates, and empty photo results.

@nariming nariming added the 🐛 fix develop 브랜치에서 발생한 버그를 수정하는 작업 label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Memory photos are now associated only with courses. The repository retrieves them in photoOrder order. Diary service photo access requires JOINED membership and supports course-wide retrieval. Tests cover access control, diary behavior, and course completion validation.

Changes

Course memory photo access

Layer / File(s) Summary
Course photo entity and query contract
src/main/java/com/daytodo/domain/course/entity/MemoryPhoto.java, src/main/java/com/daytodo/domain/course/repository/MemoryPhotoRepository.java
MemoryPhoto no longer stores a diary association or exposes linkDiary. The repository now retrieves ordered photos by course.
Joined-member photo access
src/main/java/com/daytodo/domain/course/service/DiaryService.java
getPhotosByCourse and getMemoryByDate require JOINED membership and retrieve photos by course.
Photo access and diary validation tests
src/test/java/com/daytodo/domain/course/repository/CourseQueryValidationTest.java, src/test/java/com/daytodo/domain/course/service/DiaryServiceTest.java
Tests cover repository queries, diary creation and updates, joined-member access, former-member denial, and incomplete-course rejection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: hyeoniss, yxpjseo

Sequence Diagram(s)

sequenceDiagram
  participant DiaryService
  participant CourseMemberRepository
  participant MemoryPhotoRepository

  DiaryService->>CourseMemberRepository: Check JOINED membership
  CourseMemberRepository-->>DiaryService: Return membership status
  DiaryService->>MemoryPhotoRepository: Query ordered photos by course
  MemoryPhotoRepository-->>DiaryService: Return course photos
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 일기 작성 시 미연결 추억사진을 diary에 연결하는 주요 변경 사항을 구체적으로 설명합니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/diary-memory-photo-link

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/com/daytodo/domain/course/service/DiaryService.java`:
- Around line 111-113: Update getMemoryByDate to call requireCourseMember with
userId and diary.getCourse().getCourseId() before
findAllByCourse_CourseIdOrderByPhotoOrderAsc. Add a regression test covering a
diary owner whose course membership is not JOINED, ensuring shared photos are
not returned.
- Around line 168-170: Update linkUnlinkedMemoryPhotos to use an atomic
conditional repository update that assigns photos only while diary is null,
ensuring first-writer-wins behavior instead of selecting entities and linking
them in memory. Add a concurrent-write test verifying that each photo remains
linked to only the transaction that claims it first.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b6db5a73-d054-41ba-8901-0e1678ab6333

📥 Commits

Reviewing files that changed from the base of the PR and between dd51338 and 7de0ebe.

📒 Files selected for processing (4)
  • src/main/java/com/daytodo/domain/course/repository/MemoryPhotoRepository.java
  • src/main/java/com/daytodo/domain/course/service/DiaryService.java
  • src/test/java/com/daytodo/domain/course/repository/CourseQueryValidationTest.java
  • src/test/java/com/daytodo/domain/course/service/DiaryServiceTest.java

Comment thread src/main/java/com/daytodo/domain/course/service/DiaryService.java
Comment thread src/main/java/com/daytodo/domain/course/service/DiaryService.java Outdated
@nariming
nariming force-pushed the fix/diary-memory-photo-link branch 2 times, most recently from fc0249c to 9fd4d71 Compare August 6, 2026 15:27

@kwonwnsduf kwonwnsduf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 연결은 일기 작성 시점에만 수행되므로, 일기를 먼저 작성한 뒤 저장되거나 UPDATE와 동시에 INSERT되는 사진은 diary_id=null로 계속 남습니다. saveMemoryPhotos()는 코스 상태나 기존 일기를 확인하지 않기 때문에 실제 API 호출로 가능한 순서인거 같습니다. 일기 이후 업로드를 금지하거나 사진 저장 경로에서도 기존 일기에 연결하고 동일한 코스 락으로 직렬화하는 방법도 한 번 생각해주시면 감사하겠습니다. 공용 사진 조회에 diary_id가 필요하지 않다면 이 연관관계 자체를 제거하는 편도 검토할 수 있을 거 같습니다. 개인적인 의견이나 참고만 해주세요! 수고하셨습니다

@nariming
nariming force-pushed the fix/diary-memory-photo-link branch from 9fd4d71 to 1c37e71 Compare August 7, 2026 12:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/daytodo/domain/course/entity/MemoryPhoto.java (1)

43-55: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

memory_photo.diary_id 관련 스키마 마이그레이션을 추가하십시오.

레버리지에서는 해당 열, 인덱스, 외래 키를 처리하는 마이그레이션이 없습니다. 엔티티가 더 이상 diary_id를 저장하지 않으므로, 기존 데이터베이스의 메모리 사진 연결 값과 외래 키 제약 조건/열을 마이그레이션에서 함께 처리하십시오.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/daytodo/domain/course/entity/MemoryPhoto.java` around lines
43 - 55, MemoryPhoto no longer persists diary_id, so add a schema migration for
the existing memory_photo table that preserves or transfers current diary
associations as required, then removes the diary_id foreign-key constraint,
related index, and column. Ensure the migration handles existing rows safely and
leaves the schema consistent with the course/imageUrl/photoOrder fields used by
MemoryPhoto.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/main/java/com/daytodo/domain/course/entity/MemoryPhoto.java`:
- Around line 43-55: MemoryPhoto no longer persists diary_id, so add a schema
migration for the existing memory_photo table that preserves or transfers
current diary associations as required, then removes the diary_id foreign-key
constraint, related index, and column. Ensure the migration handles existing
rows safely and leaves the schema consistent with the course/imageUrl/photoOrder
fields used by MemoryPhoto.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e15bfc7-6db2-4bb1-9930-f7b388a65da5

📥 Commits

Reviewing files that changed from the base of the PR and between 7de0ebe and 1c37e71.

📒 Files selected for processing (5)
  • src/main/java/com/daytodo/domain/course/entity/MemoryPhoto.java
  • src/main/java/com/daytodo/domain/course/repository/MemoryPhotoRepository.java
  • src/main/java/com/daytodo/domain/course/service/DiaryService.java
  • src/test/java/com/daytodo/domain/course/repository/CourseQueryValidationTest.java
  • src/test/java/com/daytodo/domain/course/service/DiaryServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/daytodo/domain/course/repository/CourseQueryValidationTest.java

@kwonwnsduf kwonwnsduf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다

@nariming
nariming merged commit e992dc6 into develop Aug 7, 2026
1 check passed
@nariming
nariming deleted the fix/diary-memory-photo-link branch August 7, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 fix develop 브랜치에서 발생한 버그를 수정하는 작업

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants