Conversation
…조직 세부 정보 조회, 조직 이름으로 검색해 해당 이름 가진 조직 조회)
…보 조회, 조직 이름으로 검색해 해당 이름 가진 조직 조회)
…조직의 세부정보 조회 메서드" 회원 관련 데이터 제거
# Conflicts: # src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/dto/response/OrgResponse.java # src/main/java/com/whereyouad/WhereYouAd/global/exception/GlobalExceptionHandler.java
Walkthrough조직 조회 API 2가지를 구현합니다. 사용자가 속한 모든 조직을 목록으로 조회하는 엔드포인트와 특정 조직의 상세 정보를 조회하는 엔드포인트를 추가합니다. SimpleInfo, MyOrganizations, OrgDetail DTO를 정의하고 매퍼, 서비스, 컨트롤러 계층을 연동합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant OrgController
participant OrgService
participant OrgMemberRepository
participant Database as DB
Client->>OrgController: GET /api/orgs/my
OrgController->>OrgService: getMyOrganizations(userId)
OrgService->>OrgMemberRepository: findOrgMemberByUserId(userId)
OrgMemberRepository->>Database: Query OrgMember + Organization<br/>(status = ACTIVE)
Database-->>OrgMemberRepository: List<OrgMember>
OrgMemberRepository-->>OrgService: List<OrgMember>
OrgService->>OrgService: toOrgSimpleInfo() 변환 (각 OrgMember)
OrgService->>OrgService: toMyOrganizations() 래핑
OrgService-->>OrgController: MyOrganizations
OrgController-->>Client: DataResponse<MyOrganizations>
sequenceDiagram
participant Client
participant OrgController
participant OrgService
participant OrgRepository
participant Database as DB
Client->>OrgController: GET /api/orgs/{orgId}
OrgController->>OrgService: getOrganizationDetail(orgId)
OrgService->>OrgRepository: findById(orgId)
OrgRepository->>Database: Query Organization
Database-->>OrgRepository: Organization
OrgRepository-->>OrgService: Organization
OrgService->>OrgService: 소프트 삭제 상태 검증
OrgService->>OrgService: toOrgDetail() 변환
OrgService-->>OrgController: OrgDetail
OrgController-->>Client: DataResponse<OrgDetail>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
📝 상세 검토 포인트✅ 잘 설계된 부분
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 34.78% 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 |
|---|---|---|
| Title check | ✅ Passed | PR 제목이 주요 변경사항을 명확하게 요약합니다. '조직 조회 API 추가'는 간결하고 구체적이며, 핵심 변경사항과 정확히 일치합니다. |
| Description check | ✅ Passed | PR 설명이 템플릿의 모든 주요 섹션(관련 이슈, 개요, 작업 내용, 테스트 결과, 체크리스트, 리뷰 포인트)을 완벽하게 충족하고 있습니다. 스크린샷과 예외 처리 테스트까지 상세하게 포함되어 있습니다. |
| Linked Issues check | ✅ Passed | PR의 모든 주요 코드 변경사항이 Issue #33의 요구사항을 완벽하게 충족합니다. 로그인한 회원의 조직 목록 조회 API와 특정 조직 상세 조회 API가 모두 구현되었으며, Soft Delete 처리도 추가되어 있습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 코드 변경사항이 Issue #33의 범위 내에 있습니다. 추가된 메서드, DTO, 에러 코드, 리포지토리 쿼리 등이 조직 조회 기능에만 집중되어 있으며, 불필요한 변경사항은 없습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/#33
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.java (1)
46-54:⚠️ Potential issue | 🟠 Major
getOrganizationDetail엔드포인트에 인가(Authorization) 검사가 누락되어 있습니다 — 정보 유출 취약점 🚨현재 코드를 확인한 결과, 이 엔드포인트는
@AuthenticationPrincipal이 전혀 없고userId를 사용하지 않습니다. 즉:
- 모든 인증된 사용자가
GET /api/org/{orgId}를 호출하면 어떤 조직의 상세 정보든 조회 가능합니다.orgId가 순차적 Long이므로 1, 2, 3... 순서로 요청해 모든 조직 정보를 열거(enumeration) 공격할 수 있습니다.modifyOrganization메서드처럼 오너 검증이 없습니다 (line 108-110 참조).현재 API 문서(
OrgControllerDocs)도 이 엔드포인트의 공개/비공개 의도를 명확히 하지 않고 있습니다. 다른 엔드포인트들(getMyOrganizations)과 달리 인증 실패 시나리오(401)를 문서화하지 않았으므로, 의도가 불명확합니다.수정 방향:
- 조직 멤버/오너만 조회 가능하려면:
userId를 추가하고 서비스 레이어에서orgMemberRepository.findByUserIdAndOrgId(userId, orgId)검증 추가- 의도상 공개 정보라면: 명확히 문서화하고
SecurityConfig의.permitAll()처리 검토`@GetMapping`("/{orgId}") - public ResponseEntity<DataResponse<OrgResponse.OrgDetail>> getOrganizationDetail(`@PathVariable` Long orgId) + public ResponseEntity<DataResponse<OrgResponse.OrgDetail>> getOrganizationDetail( + `@AuthenticationPrincipal`(expression = "userId") Long userId, + `@PathVariable` Long orgId) { - OrgResponse.OrgDetail response = orgService.getOrganizationDetail(orgId); + OrgResponse.OrgDetail response = orgService.getOrganizationDetail(userId, orgId); ... }그리고
OrgServiceImpl에서:// 멤버십 검증 orgMemberRepository.findByUserIdAndOrgId(userId, orgId) .orElseThrow(() -> new OrgException(OrgErrorCode.ORG_FORBIDDEN));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.java` around lines 46 - 54, getOrganizationDetail currently lacks authorization checks: add an authenticated user parameter to the controller (use `@AuthenticationPrincipal` to inject userId into OrgController.getOrganizationDetail), pass that userId into the OrgService.getOrganizationDetail call, and enforce membership/owner validation inside OrgServiceImpl by calling orgMemberRepository.findByUserIdAndOrgId(userId, orgId).orElseThrow(() -> new OrgException(OrgErrorCode.ORG_FORBIDDEN));; alternatively, if the endpoint is intended to be public, update OrgControllerDocs and SecurityConfig to explicitly permit public access (rather than leaving it ambiguous).
🧹 Nitpick comments (2)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java (1)
26-26: 읽기 전용 메서드에@Transactional(readOnly = true)를 추가하면 성능이 개선됩니다.현재 클래스 레벨의
@Transactional은readOnly = false(기본값)입니다.getMyOrganizations와getOrganizationDetail은 데이터를 변경하지 않는 조회 전용 메서드인데, 이 경우:
- Hibernate가 dirty checking(변경 감지)을 스킵하므로 불필요한 오버헤드가 줄어듭니다.
- 읽기 전용 DB 레플리카를 라우팅하도록 설정한 경우 자동으로 활용됩니다.
♻️ 개선 제안
`@Service` `@Transactional` `@RequiredArgsConstructor` public class OrgServiceImpl implements OrgService { ... + `@Transactional`(readOnly = true) public OrgResponse.MyOrganizations getMyOrganizations(Long userId) { ... } + `@Transactional`(readOnly = true) public OrgResponse.OrgDetail getOrganizationDetail(Long orgId) { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java` at line 26, Class-level `@Transactional` currently defaults to readOnly = false; mark read-only service methods to improve performance by skipping dirty-check and allowing replica routing: add `@Transactional`(readOnly = true) to the methods getMyOrganizations and getOrganizationDetail in OrgServiceImpl (or annotate those methods to override the class-level transactional setting) so these two query-only methods run in read-only transactions.src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java (1)
20-22:join fetch로 N+1 문제를 깔끔하게 방지했네요! 👍 다만o.status = 'ACTIVE'문자열 리터럴은 개선이 필요합니다.
join fetch om.organization은 서비스 계층에서om.getOrganization()을 호출할 때 추가 쿼리가 발생하지 않도록 잘 처리했습니다. 그런데o.status = 'ACTIVE'처럼 enum 값을 JPQL 내 문자열 리터럴로 비교하는 방식은 표준 JPQL 스펙에 어긋나고,@Enumerated(EnumType.ORDINAL)로 변경되거나 enum 값 이름이 바뀌는 순간 런타임에 조용히 깨질 수 있습니다. 같은 파일의findByOrganizationIdWithCursor처럼 파라미터로 바인딩하는 방식이 훨씬 안전합니다.예를 들어:
// ❌ 현재: 문자열 리터럴 (fragile) ... and o.status = 'ACTIVE' // ✅ 개선: enum 파라미터 바인딩 (type-safe) ... and o.status = :status그리고 메서드 시그니처에 파라미터를 추가하면 됩니다:
♻️ 개선 제안
-@Query(value = "select om from OrgMember om join fetch om.organization o where om.user.id = :userId and o.status = 'ACTIVE'") -List<OrgMember> findOrgMemberByUserId(`@Param`("userId") Long userId); +@Query(value = "select om from OrgMember om join fetch om.organization o where om.user.id = :userId and o.status = :status") +List<OrgMember> findOrgMemberByUserId(`@Param`("userId") Long userId, `@Param`("status") OrgStatus status);호출부(
OrgServiceImpl66줄)에서는:-List<OrgMember> orgMembers = orgMemberRepository.findOrgMemberByUserId(userId); +List<OrgMember> orgMembers = orgMemberRepository.findOrgMemberByUserId(userId, OrgStatus.ACTIVE);JPQL 공식 스펙에서도 "리터럴 대신 파라미터를 사용하면 쿼리를 한 번만 컴파일하고 다양한 값으로 재사용할 수 있어 더 효율적"이라고 권장하고 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java` around lines 20 - 22, The JPQL in OrgMemberRepository.findOrgMemberByUserId uses a hard-coded string literal o.status = 'ACTIVE' which is fragile; change the query to bind a status parameter (e.g. ... and o.status = :status) and add a method parameter (annotated `@Param`("status")) so the repository method accepts the enum value instead of a string; update callers (e.g. OrgServiceImpl where findOrgMemberByUserId is invoked) to pass the appropriate Organization status enum (like Organization.Status.ACTIVE) and follow the same parameter-binding approach used in findByOrganizationIdWithCursor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/mapper/OrgConverter.java`:
- Around line 47-61: The role determination in OrgConverter.toOrgSimpleInfo is
incorrect because it infers ADMIN solely from
organization.getOwnerUserId().equals(userId); change the method to accept the
caller's OrgMember (or directly an OrgRole) instead of userId/Organization-only,
and use orgMember.getRole() (or the passed OrgRole) to populate the
SimpleInfo.myRole; update callers (e.g., OrgServiceImpl.getMyOrganizations) to
pass the OrgMember or its role when calling toOrgSimpleInfo so the real
DB-stored role is used rather than comparing ownerUserId.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 64-85: getMyOrganizations currently extracts Organization objects
and discards OrgMember.role, causing role info to be lost and roles to be
re-inferred by ownership; change OrgServiceImpl.getMyOrganizations to
iterate/stream over the List<OrgMember> returned by
orgMemberRepository.findOrgMemberByUserId and call OrgConverter.toOrgSimpleInfo
with the OrgMember (not Organization) so the converter can use OrgMember.role
directly (update OrgConverter.toOrgSimpleInfo signature accordingly), remove the
intermediate organizations list and produce infos directly from the OrgMember
stream.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/exception/code/OrgErrorCode.java`:
- Around line 24-25: The enum constant ORG_SOFT_DELETED has a missing space
before the parenthetical; update its message string from "해당 조직은 삭제된 조직입니다.(Soft
Delete)" to include a space before "(Soft Delete)" so it reads "해당 조직은 삭제된
조직입니다. (Soft Delete)" in the ORG_SOFT_DELETED definition.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgRepository.java`:
- Around line 12-14: The method findOrganizationsByName in OrgRepository is dead
code (not referenced from OrgService or OrgServiceImpl); remove the method
declaration and related imports (Page, Pageable, Query, Param) from
OrgRepository, and if you keep similar queries elsewhere replace the literal
status check with the OrgStatus enum (use OrgStatus.ACTIVE) to ensure type-safe
filtering; update OrgRepository to cleanly compile after removing
findOrganizationsByName.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.java`:
- Around line 37-45: The Swagger docs for getOrganizationDetail in
OrgControllerDocs are missing the 410 response for soft-deleted orgs; update the
`@ApiResponses` on the getOrganizationDetail method to include an ApiResponse for
responseCode = "410" (description like "해당 조직은 소프트 삭제됨 / ORG_SOFT_DELETED") so
the API docs match the behavior thrown by OrgServiceImpl (ORG_SOFT_DELETED).
Ensure the new ApiResponse is added alongside the existing 200 and 404 entries
and references getOrganizationDetail/OrgControllerDocs so consumers see the 410
Gone case.
---
Outside diff comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.java`:
- Around line 46-54: getOrganizationDetail currently lacks authorization checks:
add an authenticated user parameter to the controller (use
`@AuthenticationPrincipal` to inject userId into
OrgController.getOrganizationDetail), pass that userId into the
OrgService.getOrganizationDetail call, and enforce membership/owner validation
inside OrgServiceImpl by calling
orgMemberRepository.findByUserIdAndOrgId(userId, orgId).orElseThrow(() -> new
OrgException(OrgErrorCode.ORG_FORBIDDEN));; alternatively, if the endpoint is
intended to be public, update OrgControllerDocs and SecurityConfig to explicitly
permit public access (rather than leaving it ambiguous).
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Line 26: Class-level `@Transactional` currently defaults to readOnly = false;
mark read-only service methods to improve performance by skipping dirty-check
and allowing replica routing: add `@Transactional`(readOnly = true) to the methods
getMyOrganizations and getOrganizationDetail in OrgServiceImpl (or annotate
those methods to override the class-level transactional setting) so these two
query-only methods run in read-only transactions.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java`:
- Around line 20-22: The JPQL in OrgMemberRepository.findOrgMemberByUserId uses
a hard-coded string literal o.status = 'ACTIVE' which is fragile; change the
query to bind a status parameter (e.g. ... and o.status = :status) and add a
method parameter (annotated `@Param`("status")) so the repository method accepts
the enum value instead of a string; update callers (e.g. OrgServiceImpl where
findOrgMemberByUserId is invoked) to pass the appropriate Organization status
enum (like Organization.Status.ACTIVE) and follow the same parameter-binding
approach used in findByOrganizationIdWithCursor.
...ain/java/com/whereyouad/WhereYouAd/domains/organization/application/mapper/OrgConverter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java
Show resolved
Hide resolved
Comment on lines
+24
to
+25
| //410 | ||
| ORG_SOFT_DELETED(HttpStatus.GONE, "ORG_410_1", "해당 조직은 삭제된 조직입니다.(Soft Delete)"), |
There was a problem hiding this comment.
410 Gone 사용은 의미론적으로 정확합니다! 👍 메시지 문자열에 공백 하나가 빠졌습니다.
소프트 딜리트된 리소스에 HttpStatus.GONE을 사용한 건 HTTP 스펙에 맞는 좋은 선택입니다. 다만 현재 메시지 "해당 조직은 삭제된 조직입니다.(Soft Delete)"는 (Soft Delete) 앞에 공백이 없어서 PR 스크린샷에 보이는 응답 예시("해당 조직은 삭제된 조직입니다. (Soft Delete)")와 다릅니다.
✏️ 메시지 수정 제안
-ORG_SOFT_DELETED(HttpStatus.GONE, "ORG_410_1", "해당 조직은 삭제된 조직입니다.(Soft Delete)"),
+ORG_SOFT_DELETED(HttpStatus.GONE, "ORG_410_1", "해당 조직은 삭제된 조직입니다. (Soft Delete)"),📝 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.
Suggested change
| //410 | |
| ORG_SOFT_DELETED(HttpStatus.GONE, "ORG_410_1", "해당 조직은 삭제된 조직입니다.(Soft Delete)"), | |
| //410 | |
| ORG_SOFT_DELETED(HttpStatus.GONE, "ORG_410_1", "해당 조직은 삭제된 조직입니다. (Soft Delete)"), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/exception/code/OrgErrorCode.java`
around lines 24 - 25, The enum constant ORG_SOFT_DELETED has a missing space
before the parenthetical; update its message string from "해당 조직은 삭제된 조직입니다.(Soft
Delete)" to include a space before "(Soft Delete)" so it reads "해당 조직은 삭제된
조직입니다. (Soft Delete)" in the ORG_SOFT_DELETED definition.
...ava/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgRepository.java
Outdated
Show resolved
Hide resolved
...java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.java
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java (1)
77-90: Soft Delete 체크 로직이 깔끔합니다! 🎉 삭제된 조직에 대해 명확하게 예외를 던져주는 패턴이 좋습니다.
@Transactional(readOnly = true)적용을 권장합니다.현재
getOrganizationDetail과 위의getMyOrganizations는 모두 조회만 수행하는 메서드인데, 클래스 레벨@Transactional(기본값readOnly = false)을 그대로 상속받고 있습니다.
readOnly = true를 메서드 레벨에 명시하면:
- Hibernate가 dirty checking을 건너뛰어 성능이 향상됩니다.
- DB가 read-replica를 사용하는 환경이라면 읽기 전용 커넥션으로 라우팅될 수 있습니다.
- 코드 의도도 명확하게 전달됩니다 (이 메서드는 데이터를 변경하지 않는다!).
♻️ 수정 제안
+ `@Transactional`(readOnly = true) public OrgResponse.MyOrganizations getMyOrganizations(Long userId) { ... } + `@Transactional`(readOnly = true) public OrgResponse.OrgDetail getOrganizationDetail(Long orgId) { ... }As per coding guidelines, "JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지, FetchType.LAZY가 잘 적용되었는지 체크하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java` around lines 77 - 90, Annotate the read-only service methods (getOrganizationDetail and getMyOrganizations) with `@Transactional`(readOnly = true) to override the class-level transactional default; add the import for org.springframework.transaction.annotation.Transactional if missing. While updating, quickly inspect how Organization and related entities are accessed in these methods (e.g., lazy collections/relations) to ensure no N+1 queries are triggered—eagerly fetch needed associations via repository fetch joins or adjust repository methods if necessary rather than changing transactional semantics. Ensure the methods' behavior and exception handling (OrgHandler/OrgErrorCode) remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 77-90: Annotate the read-only service methods
(getOrganizationDetail and getMyOrganizations) with `@Transactional`(readOnly =
true) to override the class-level transactional default; add the import for
org.springframework.transaction.annotation.Transactional if missing. While
updating, quickly inspect how Organization and related entities are accessed in
these methods (e.g., lazy collections/relations) to ensure no N+1 queries are
triggered—eagerly fetch needed associations via repository fetch joins or adjust
repository methods if necessary rather than changing transactional semantics.
Ensure the methods' behavior and exception handling (OrgHandler/OrgErrorCode)
remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java (1)
62-74: 스트림 기반 매핑으로 깔끔하게 수정된 것 확인! ✅ —@Transactional(readOnly = true)추가를 권장합니다이전 코드의 중간 리스트 문제가 스트림으로 깔끔하게 해결됐네요! 👍
다만 클래스 레벨의
@Transactional은 기본적으로readOnly = false이기 때문에, 순수 조회 메서드인getMyOrganizations도 불필요하게 쓰기 트랜잭션으로 실행됩니다.
readOnly = true를 지정하면 Hibernate의 dirty checking이 비활성화되고 flush mode가NEVER로 설정되어 불필요한 오버헤드를 줄일 수 있습니다.OrgQueryServiceImpl을 보면 클래스 레벨에서@Transactional(readOnly = true)를 사용하는 일관된 패턴을 이미 따르고 있습니다.♻️ 개선 제안
+ `@Transactional`(readOnly = true) public OrgResponse.MyOrganizations getMyOrganizations(Long userId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java` around lines 62 - 74, The getMyOrganizations method in OrgServiceImpl is a read-only DB query but currently runs under the default (read-write) transaction; annotate it (or the OrgServiceImpl class if you prefer consistent behavior) with `@Transactional`(readOnly = true) so Hibernate disables dirty-checking and avoids unnecessary flushes—apply the annotation to the getMyOrganizations method or add it at the class level in OrgServiceImpl, keeping the method body (orgMemberRepository.findOrgMemberByUserId, OrgConverter::toOrgSimpleInfo, OrgConverter.toMyOrganizations) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 77-89: The getOrganizationDetail method currently only checks
soft-deletion; add membership/access control: obtain the current user id (via
your auth component, e.g., SecurityContextHolder or an injected AuthService
method such as getCurrentUserId()), then use
orgMemberRepository.existsByOrganizationIdAndUserId(orgId, currentUserId) (or
equivalent) to verify the requester is a member (or otherwise allowed) and if
not throw new OrgHandler(OrgErrorCode.ORG_FORBIDDEN); keep the existing
orgRepository lookup and DELETED check and perform the membership check before
converting to DTO in OrgConverter.toOrgDetail.
- Around line 77-89: getOrganizationDetail in OrgServiceImpl is a read-only
lookup but runs under the class-level `@Transactional` (readOnly=false); annotate
the method getOrganizationDetail(...) with `@Transactional`(readOnly = true) to
enforce a read-only transaction, preventing unnecessary dirty-checking and
signaling intent, leaving the existing logic that uses
orgRepository.findById(...) and OrgConverter.toOrgDetail(...) unchanged.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 62-74: The getMyOrganizations method in OrgServiceImpl is a
read-only DB query but currently runs under the default (read-write)
transaction; annotate it (or the OrgServiceImpl class if you prefer consistent
behavior) with `@Transactional`(readOnly = true) so Hibernate disables
dirty-checking and avoids unnecessary flushes—apply the annotation to the
getMyOrganizations method or add it at the class level in OrgServiceImpl,
keeping the method body (orgMemberRepository.findOrgMemberByUserId,
OrgConverter::toOrgSimpleInfo, OrgConverter.toMyOrganizations) unchanged.
Comment on lines
+77
to
89
| public OrgResponse.OrgDetail getOrganizationDetail(Long orgId) { | ||
| //해당 조직 id 로 Organization 조회 | ||
| Organization organization = orgRepository.findById(orgId) | ||
| .orElseThrow(() -> new OrgHandler(OrgErrorCode.ORG_NOT_FOUND)); | ||
|
|
||
| //Soft Delete 된 조직이면 예외처리 | ||
| if (organization.getStatus() == OrgStatus.DELETED) { | ||
| throw new OrgHandler(OrgErrorCode.ORG_SOFT_DELETED); | ||
| } | ||
|
|
||
| //DTO 로 변환 | ||
| return OrgConverter.toOrgDetail(organization); | ||
| } |
There was a problem hiding this comment.
조직 비공개 여부에 따라 멤버십 접근 제어 검토 필요
현재 구현은 삭제된 조직 여부(소프트 딜리트)만 확인하고, 요청한 사용자가 해당 조직의 멤버인지는 검증하지 않습니다. 즉, 인증된 사용자라면 조직 id만 알면 본인이 속하지 않은 조직의 상세 정보도 조회할 수 있습니다.
비공개 조직이나 B2B 테넌트 격리가 요구사항에 있다면 아래와 같은 멤버십 검증 로직을 추가하는 것을 권장합니다:
// 예시: orgMemberRepository를 통해 멤버십 확인
boolean isMember = orgMemberRepository.existsByOrganizationIdAndUserId(orgId, currentUserId);
if (!isMember) {
throw new OrgHandler(OrgErrorCode.ORG_FORBIDDEN);
}요구사항상 조직 상세 정보가 인증된 사용자 누구에게나 공개되는 것이라면 현재 구현도 문제없습니다. 의도된 설계인지 확인 부탁드립니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`
around lines 77 - 89, The getOrganizationDetail method currently only checks
soft-deletion; add membership/access control: obtain the current user id (via
your auth component, e.g., SecurityContextHolder or an injected AuthService
method such as getCurrentUserId()), then use
orgMemberRepository.existsByOrganizationIdAndUserId(orgId, currentUserId) (or
equivalent) to verify the requester is a member (or otherwise allowed) and if
not throw new OrgHandler(OrgErrorCode.ORG_FORBIDDEN); keep the existing
orgRepository lookup and DELETED check and perform the membership check before
converting to DTO in OrgConverter.toOrgDetail.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find OrgService interface and OrgServiceImpl, then examine structure
echo "=== Finding OrgService interface (excluding Impl) ==="
fd -e java "OrgService\.java"
echo ""
echo "=== Finding OrgServiceImpl ==="
fd -e java "OrgServiceImpl\.java"
echo ""
echo "=== Finding OrgController ==="
fd -e java "OrgController\.java"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 470
🏁 Script executed:
#!/bin/bash
# Examine the three key files
echo "=== OrgService.java (interface) ==="
cat -n src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgService.java
echo ""
echo "=== OrgServiceImpl.java (class-level annotations + methods) ==="
head -100 src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java
echo ""
echo "=== OrgController.java (injection check) ==="
head -50 src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 7754
🏁 Script executed:
#!/bin/bash
# Verify the exact class-level `@Transactional` annotation in OrgServiceImpl
echo "=== Class-level `@Transactional` in OrgServiceImpl ==="
sed -n '20,28p' src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java
echo ""
echo "=== Check getOrganizationDetail method signature (lines 75-90) ==="
sed -n '75,90p' src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 945
순수 조회 메서드에 @Transactional(readOnly = true) 추가 필요
이 메서드는 Organization을 조회하고 상태를 확인한 후 DTO로 변환하는 순수 읽기 작업입니다. 현재는 클래스 레벨의 @Transactional(기본값: readOnly = false)에서 실행되고 있는데, 이는 쓰기 작업을 위한 설정입니다.
readOnly = true를 추가하면:
- 데이터베이스 레벨에서 읽기 전용 트랜잭션으로 최적화됨
- Dirty checking 오버헤드 제거 (변경사항 감지 안 함)
- 명확한 의도 전달 (이 메서드는 조회만 한다)
♻️ 개선 제안
+ `@Transactional`(readOnly = true)
public OrgResponse.OrgDetail getOrganizationDetail(Long orgId) {현재 코드는 OrgService 인터페이스에 메서드가 잘 선언되어 있고(12번 줄), OrgController도 인터페이스 타입으로 주입받기 때문에 정상 작동합니다. 좋은 DI 패턴이네요. 👍
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`
around lines 77 - 89, getOrganizationDetail in OrgServiceImpl is a read-only
lookup but runs under the class-level `@Transactional` (readOnly=false); annotate
the method getOrganizationDetail(...) with `@Transactional`(readOnly = true) to
enforce a read-only transaction, preventing unnecessary dirty-checking and
signaling intent, leaving the existing logic that uses
orgRepository.findById(...) and OrgConverter.toOrgDetail(...) unchanged.
kingmingyu
approved these changes
Feb 18, 2026
Collaborator
kingmingyu
left a comment
kingmingyu
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다!! soft delete 고려해서 잘 작성된 것 같습니다!!👍
Collaborator
|
P4: 고생하셨습니다! 단순 조회만 생각했었는데 삭제 후 status에 따라 조회되는 내용을 잘 구분지어주셔서 명확하고 좋은 것 같습니다!! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
🚀 개요
조직 CRUD API 중 조회 API (내 조직 전체 조회, 특정 조직 상세 조회) 추가
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
===예외 처리===
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit