Repository navigation
feat: 활동사진 업로드를 부원 전체에게 연다 - #402
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 681dccdd1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **업로드는 부원 누구나 한다** (2026-09-03, [#400](https://github.com/HackerKHU/hacker_HP/issues/400)). `ACTIVE`·`INACTIVE` 둘 다다 — 활동사진은 자료 갈래가 아니라 비활동 부원도 쓴다. | ||
| - **삭제는 본인이 올린 것만이다.** `ADMIN`은 전체를 지운다. 업로더가 비어 있는 사진(탈퇴한 회원의 것)은 `ADMIN`만 지운다. |
There was a problem hiding this comment.
일반 USER와 INACTIVE 회원은 API 권한이 열려도 실제 웹 화면에서는 이 기능을 사용할 수 없습니다. apps/web/src/App.tsx의 /admin/photos/new는 여전히 RequireActive requiredRole="ADMIN" 아래에 있고, PhotoGalleryPage.tsx도 isAdmin일 때만 업로드 링크와 삭제 버튼을 렌더링하므로 소모임장은 업로드 화면에 진입하지 못하고 업로더도 자기 사진을 삭제할 수 없습니다. fixtures.ts의 requirePhotoAdmin()도 새 계약과 반대로 일반 회원 요청을 403으로 거절합니다. 라우트·버튼의 소유자 판정과 웹 테스트를 함께 변경해야 이 커밋의 사용자 흐름이 실제로 열립니다.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
| - **업로드는 부원 누구나 한다** (2026-09-03, [#400](https://github.com/HackerKHU/hacker_HP/issues/400)). `ACTIVE`·`INACTIVE` 둘 다다 — 활동사진은 자료 갈래가 아니라 비활동 부원도 쓴다. | ||
| - **삭제는 본인이 올린 것만이다.** `ADMIN`은 전체를 지운다. 업로더가 비어 있는 사진(탈퇴한 회원의 것)은 `ADMIN`만 지운다. |
There was a problem hiding this comment.
새 정책은 일반 부원이 사진을 올리고 본인 사진을 삭제할 수 있다고 정하지만, 배포되는 apps/web/src/features/legal/termsContent.ts:61은 여전히 "활동사진은 운영진만 다룹니다"라고 안내합니다. 사용자가 동의하고 확인하는 이용약관이 실제 권한과 모순되므로, 이번 요구사항 변경과 함께 업로드·삭제 권한 문구를 갱신해야 합니다.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
| void aPlainMemberCanIssueUploadUrls() throws Exception { | ||
| mockMvc | ||
| .perform(issueUploadUrl(member)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$[0].uploadUrl").isNotEmpty()); |
There was a problem hiding this comment.
새 테스트가 T-604를 표방하지만 여기서는 POST /photos/upload-url까지만 호출하므로, 이번에 권한 검사가 바뀐 실제 POST /photos 등록과 uploader_id 저장은 여전히 관리자 테스트만 거칩니다. 또한 T-611이라고 적은 테스트도 S3 등록 도중의 정지를 재현하지 않고 DELETE만 호출하므로, registerOne()의 두 번째 requireWritable()이나 실패 시 자리표시자 정리가 제거되어도 새 테스트들이 모두 통과합니다. 일반 회원으로 원본 등록까지 수행하고, 저장소 왕복 사이에 정지시켜 행이 남지 않는지 검증해야 합니다.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
|
반영했습니다. CI 실패부터 — 이 PR과 무관한 고장이었습니다
Docker Hub의 MinIO의 공식 배포처인 new MinIOContainer(
DockerImageName.parse("quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z")
.asCompatibleSubstituteFor("minio/minio"))
1. 프런트엔드 가드 (P1) — 절반 반영, 절반은 #401입니다픽스처와 이용약관은 이 PR이 맞습니다. 둘 다 "규칙이 무엇인가" 를 적은 것이라 규칙이 바뀌는 여기서 함께 가야 합니다.
라우트와 버튼은 #401입니다. 일부러 나눈 것이고 그 이슈에 "#400이 머지된 뒤에 시작한다" 를 경고 블록으로 적어 뒀습니다 — 계절학기(#272·#273)와 같은 순서입니다.
2. 이용약관 (P2) — 맞습니다
→ 자료와 활동사진은 올리신 본인도 지우실 수 있고 운영진은 전체를 지울 수 있으며, 공지는 운영진만 다룹니다. 약관은 UI 버튼이 아니라 권리를 적는 자리라 규칙이 바뀌는 이 PR에서 고치는 것이 맞습니다. 3. 등록 경로·S3 왕복 (P2) — 맞습니다. 실제로 구멍이었습니다지적을 확인하려고 두 번째 업로드가 시작되는 순간에 정지시키도록 바꿨습니다. doAnswer(invocation -> {
jdbcTemplate.update("UPDATE users SET status = 'SUSPENDED' WHERE id = ?", member.getId());
return invocation.callRealMethod();
})
.when(storage).upload(anyString(), any(byte[].class), anyString());이제 두 번째 확인을 지우면 깨집니다. 확인했습니다. 등록 경로도 더했습니다 — 그 밖에 뒤집힌 사례 둘
검증
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e50296214
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| photos: { key: string; caption: string | null }[], | ||
| ): Promise<PhotoRegisterResult> { | ||
| const denied = requirePhotoAdmin() | ||
| const denied = requirePhotoWritable() |
There was a problem hiding this comment.
user·inactive 시나리오가 이 가드를 통과하도록 바뀌었지만, 아래에서 생성하는 사진은 여전히 USERS.admin을 uploaderId와 uploaderName으로 하드코딩합니다. 따라서 일반 부원이 픽스처 모드에서 등록한 사진은 관리자가 올린 것으로 표시되고, 이어서 fixtureRemovePhoto를 호출하면 본인 사진인데도 403 FORBIDDEN이 발생합니다. 현재 시나리오의 사용자를 업로더로 기록하고 등록 후 본인 삭제까지 검증해야 합니다.
AGENTS.md reference: apps/web/AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| void anInactiveMemberCanIssueUploadUrls() throws Exception { | ||
| User resting = | ||
| userRepository.saveAndFlush(Accounts.inactive("sub-i", "i@khu.ac.kr", "20240001")); | ||
|
|
||
| mockMvc.perform(issueUploadUrl(resting)).andExpect(status().isOk()); |
There was a problem hiding this comment.
T-605라고 명시한 이 테스트는 POST /photos/upload-url까지만 호출하므로, INACTIVE 회원이 실제 POST /photos 등록을 통과하는지는 검증하지 않습니다. 새로 추가된 전체 등록 테스트도 ACTIVE 회원만 사용하기 때문에 registerOne()의 requireWritable()이 INACTIVE를 거절하도록 바뀌거나 업로더 저장이 잘못되어도 T-605가 계속 통과합니다. 원본 업로드와 등록 응답·uploader_id 확인까지 포함해야 새 권한 동작을 보호할 수 있습니다.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
| | POST | `/photos/upload-url` | ACTIVE·INACTIVE | 원본 파일별 presigned PUT URL 발급 (다중) | | ||
| | POST | `/photos` | ACTIVE·INACTIVE | 메타데이터 등록 (JSON) — body에 업로드 완료된 원본 파일 키 목록. 서버가 그 키들을 S3에서 읽어 리사이즈한 뒤 최종 위치에 저장하고 사진마다 행을 만든다 | | ||
| | DELETE | `/photos/{id}` | 본인/ADMIN | 삭제 — **본인이 올린 것만.** `ADMIN`은 전체 | |
There was a problem hiding this comment.
API 표는 업로드를 ACTIVE·INACTIVE로 바꿨지만 같은 계약의 업로드 절차(spec/3-2-DESIGN-CONTRACT.md:886,888)는 여전히 관리자만 두 엔드포인트를 호출한다고 명시합니다. 현재 요구사항 문서의 spec/2-1-USER-STORIES.md:195,337과 테스트 기준의 spec/5-TESTING.md:421,435,438,1209도 업로드·삭제가 ADMIN 전용이라고 남아 있어, 후속 #401 구현이나 회귀 테스트가 이 문구를 따르면 기존 관리자 가드를 다시 만들게 됩니다. 역사적 ADR과 달리 현재 계약·요구사항·테스트 기준에 남은 표현은 새 업로드 및 소유자 삭제 정책으로 함께 갱신해야 합니다.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
|
셋 다 반영했습니다. 1. 픽스처 등록자 — 맞습니다. 실제로 모순이었습니다가드는 열어 놓고 등록은 지금 시나리오의 사람을 업로더로 기록하고, 등록 → 본인 삭제를 한 사례에서 이어 봅니다. 따로 재면 그 조합이 드러나지 않습니다. 2. INACTIVE 등록 경로 — 맞습니다. 여기도 구멍이었습니다확인해 봤습니다. 기존 T-605( 원본 업로드 → 실제 3. 스펙에 남은 관리자 문구 — 맞습니다표만 고치고 본문을 놓쳤습니다. 그대로 두면 #401이 그 문구를 읽고 관리자 가드를 다시 만듭니다.
T-571의 근거 문단은 과거형으로 바꿨습니다. 그 필터 매처는 #400이 걷어냈지만 함정은 그대로라, 다음에 사진 경로에 규칙을 더할 때 같은 일이 벌어진다는 것을 T-612와 이어 적었습니다.
검증
|
Closes #400
소모임장이 활동사진을 올릴 수 있게 업로드를 부원 전체에게 연다. 화면은 #401이다.
정한 대로
ADMINACTIVE·INACTIVE— 조회·좋아요와 같은 줄ADMINADMIN은 전체비로그인·
PENDING은 열지 않았다 — 그 둘은 갤러리를 볼 수조차 없다.SUSPENDED는 따로 막을 것이 없다 — 매트릭스에 그 열이 없는 것이 곧 답이다.권한이 걸린 네 자리를 전부 고쳤다
SecurityConfigPOST /photos·/upload-url,DELETE /photos/*)PhotoController@PreAuthorize셋 →isAuthenticated()PhotoServicerequireActiveAdmin→requireWritable+requireOwnerOrAdminrequireActiveAdmin을 지우지 않고 조건만 바꿨다. 그 메서드가 있는 이유는 권한 등급이 아니라 세션과 실제 상태가 어긋나는 창이다 — 등록이 S3와 왕복하는 동안 관리자가 정지시킬 수 있다. 조건이 느슨해져도 그 창은 그대로 있다 (T-611이 그것을 잰다).판단은
RequesterCheck.requireActive에 이미 있었다 —INACTIVE는 통과하고SUSPENDED·PENDING은 각자의 코드로 거절된다. 새로 만들지 않았다.소유자 판단은 자료를 그대로 따랐다
NoteEditService.requireOwnerOrAdmin과 같은 모양이다.ADMIN은 전체ADMIN만 — 주인이 없어 "본인"이 성립하지 않는다SecurityConfig경고를 코드에 남겼다매처를 걷어내면서 #346 리뷰가 남긴 함정을 주석으로 옮겨 적었다.
매처가 사라졌다고 함정이 없어지지 않는다 — 다음에 그 자리에 규칙을 더할 때 그대로 걸린다. T-612가 그것을 지킨다.
스펙
O가 아니라본인 것만이다uploader_id서술("ADMIN만 채워진다" 가 거짓이 됐다), 좋아요 절의 권한 비교테스트 (
PhotoWritePermissionIntegrationTest, 9개)이빨을 확인했다.
requireOwnerOrAdmin호출 한 줄을 지우니 T-608·T-610 둘이 정확히 깨졌다. 나머지 일곱은 그대로 통과했다 — "올릴 수 있나" 만 보는 사례로는 아무나 남의 사진을 지우는 것을 못 잡는다는 뜻이다.403, 사진은 남는다403,ADMIN은204. 널 처리를 빠뜨리면 둘 다 비었을 때 통과한다낡은 사례 하나를 뒤집었다
PhotoApiIntegrationTest.memberCannotIssueUploadUrls가 정확히 이번에 뒤집은 규칙을 단언하고 있었다.aMemberCanIssueUploadUrls로 바꾸고 왜 뒤집혔는지를 주석에 남겼다 — 값만 바꾸면 다음 사람이 실수로 읽는다.검증
./gradlew check— 885 tests, 전부 통과.다음
#401(화면) 이다. 업로드 버튼이 지금
ADMIN에게만 보여서, 서버만 열면 부원이 쓸 길이 없다.