Feature/#85 - 만료 배지 tooltip 추가 / 임계값(3/7/30일) SecretExpiryPolicy로 통합 - #97
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
SecretExpiryStatus.init의 expiresAt > now 가드는 목록뿐 아니라 DetailExpireDateFieldView에도 그대로 적용됩니다. DetailExpireDateFieldView.swift:26-36 — emphasis가 nil이 되면서 textColor가 .gray900으로, leadingIcon도 사라집니다. 즉 3개월 뒤 만료되는 시크릿과 이미 3개월 전에 만료된 시크릿의 Expire Date 필드가 화면상 완전히 동일해집니다. PR 본문의 근거는 "Expired 탭이 '이미 지남'을 전담하므로 다른 탭에서 중복 표시할 필요 없음"인데, 조회 화면은 탭이 아닙니다. All 탭에서 만료된 시크릿을 열면 어디에도 만료됐다는 신호가 없습니다. 목록 배지를 빼는 건 (섹션 헤더가 대신 말해주므로) 논리가 서지만, 상세 화면까지 같이 빠지는 건 의도한 결과인지 확실치 않아 보입니다. 제안 — 배지 제외 정책 자체는 목록의 관심사이므로, 판정 타입이 아니라 소비처에서 거르는 쪽이 맞습니다. // SecretExpiryStatus는 expired 케이스를 되살리고 // SecretListView.row(for:)에서만 제외 이러면 조회 화면은 기존대로 강조를 유지하고, 목록만 중복을 피합니다. 지금 구조는 "목록의 UX 판단"이 판정 타입에 박혀서 다른 소비처가 선택권 없이 끌려가는 형태입니다. 아니면 별도로 status 나누기(조회화면에서는 expired도 필요합니다) |
SecretListView.swift:295-299 case .within7Days: return "Expires in 7 days" contains(_:referenceDate:)의 경계 계산만 SecretExpiryPolicy로 바뀌고, 바로 위 화면에 찍히는 숫자는 리터럴로 남았습니다. upcomingWindowDays를 7→10으로 바꾸면 버킷은 10일치를 담는데 헤더는 "Expires in 7 days"라고 표시됩니다. 이 PR이 없애려던 drift가 같은 enum 안에 남아 있는 셈이라 아쉽습니다. tooltipText처럼 보간으로 처리하면 됩니다. (참고로 이 두 문자열은 .module(_:)을 안 거쳐서 로컬라이즈도 안 되는데, 그건 기존 상태라 이 PR 범위는 아닙니다 — 다만 어차피 손대는 김에 같이 정리하는 게 좋아 보입니다.) |
ScheduleSecretExpiryNotificationsUseCaseImpl.cancel(secretID:)는 현재 daysBeforeExpiry 값으로 identifier를 재구성해서 취소합니다. let identifiers = Self.daysBeforeExpiry.map { Self.notificationID(secretID: secretID, daysBefore: $0) } upcomingWindowDays가 7→10으로 바뀌면 이미 secret-expiry--7d로 예약된 알림은 영원히 취소되지 않고 그대로 발사됩니다. 시크릿을 삭제해도 마찬가지입니다. 이 취약점 자체는 기존([7, 3] 리터럴)에도 있었으니 이 PR이 만든 버그는 아닙니다. 다만 이 PR이 그 값을 "배지 임계값"과 묶으면서, UI 배지를 조정하려는 사람이 알림 스케줄까지 건드리게 되는 경로가 새로 생겼습니다. 값이 바뀔 확률이 올라간 만큼 위험도 올라갔습니다. PR 본문이 스스로 "값이 우연히 일치할 뿐" 이라고 적었는데, 우연히 일치하는 값을 하나로 묶으면 한쪽 요구사항 변경이 다른 쪽을 조용히 끌고 갑니다. 최소한 두 가지 중 하나는 필요해 보입니다.
개인적으로는 (a)를 추천합니다 — criticalWindowDays/upcomingWindowDays라는 표현 계층 어휘가 DVDomain에 올라간 것도 이 결합의 부작용으로 보입니다. |
#expect(SecretExpiryPolicy.criticalWindowDays < SecretExpiryPolicy.upcomingWindowDays) 순서 검증은 사실상 깨질 수 없는 명제라 회귀를 잡아주지 못합니다. 실제 값을 고정하는 테스트(SecretExpiryStatusTests의 criticalWindow == 3 * 86_400처럼)가 더 유용한데, DVDomain 쪽엔 listingWindowDays == 30을 고정하는 테스트가 없습니다. 순서 대신 값을 박아두는 편이 나아 보입니다. |
|
🔴 크리티컬 — 말풍선이 화면에 영구히 박제될 수 있다 Projects/DVDesign/Sources/Foundations/Tooltip/HoverTooltip.swift:73 private func showTooltip() { mouseEntered가 mouseExited 없이 두 번 들어오면 이전 패널을 잃어버립니다. addChildWindow로 부모 윈도우가 패널을 retain하고 있는데, 유일한 참조인 tooltipPanel이 덮어써지면서 그 패널을 orderOut할 방법이 사라집니다. ignoresMouseEvents = true, level = .popUpMenu라 클릭으로 치울 수도 없고, 앱 창을 닫을 때까지 화면 위에 회색 말풍선이 떠 있게 됩니다. 재현 경로가 실재합니다. updateTrackingAreas()가 tracking area를 매번 제거 후 재등록하는데: override func updateTrackingAreas() { .inVisibleRect 옵션 때문에 스크롤·레이아웃 변경·창 리사이즈마다 이 메서드가 호출되고, 마우스가 영역 안에 있는 상태에서 tracking area를 재등록하면 AppKit이 mouseEntered:를 다시 보냅니다(이게 tracking area 재등록 시 NSEvent.mouseLocation으로 직접 확인해야 한다고들 하는 이유입니다). 배지에 커서를 올려둔 채 목록을 한 칸 스크롤하면 바로 걸립니다. 수정 — showTooltip() 첫 줄에서 무조건 정리하면 끝입니다. private func showTooltip() { 같이 넣으면 좋은 안전장치 두 가지: deinit { hideTooltip() } // 뷰가 먼저 사라지는 경로 대비 |
|
🟠 확인 필요 — 배지 위 클릭이 행 선택/컨텍스트 메뉴를 막는지 hoverTooltip은 overlay로 TrackingView(NSView)를 아이콘 위에 얹습니다. TrackingView는 hitTest를 오버라이드하지 않아서 마우스 히트 타겟이 됩니다. mouseDown을 구현하지 않았으니 이벤트가 responder chain을 타고 올라가 List의 행 선택까지 도달할 가능성이 높긴 합니다. 다만 .contextMenu(우클릭)나 SwiftUI 자체 제스처는 AppKit 서브뷰가 히트테스트를 가로채면 막히는 사례가 있어서, 배지 아이콘을 정확히 클릭/우클릭했을 때 행 선택과 컨텍스트 메뉴가 뜨는지 직접 확인이 필요합니다. hover 전용 오버레이라 히트테스트에 참여할 이유가 없으니, 확인 여부와 무관하게 아래 한 줄을 넣는 게 안전합니다. tracking area는 hitTest와 독립적으로 동작해서 툴팁은 그대로 뜹니다. override func hitTest(_ point: NSPoint) -> NSView? { nil } |
만료 관련 기간 상수가 네 곳(SecretExpiryStatus의 배지 임계값, 만료 알림 스케줄러의 daysBeforeExpiry, ExpiryBucket의 Expired 탭 섹션 경계, SecretQuery.Collection의 쿼리 window)에 각각 리터럴로 적혀 있었다. 값이 우연히 일치할 뿐이라 한 곳만 바뀌면 나머지가 조용히 어긋난다. DVDomain에 SecretExpiryPolicy를 두고 네 소비처가 값만 공유하도록 정리했다. 배지·섹션 분류처럼 목적이 다른 판정 로직은 그대로 각자 소유한다 — 통합한 것은 숫자뿐이다. 배지 표시 자체(critical/upcoming 2단계, 아이콘 형태로 색맹 대응)는 이미 구현되어 있어 새로 만들지 않았다. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
목록 행의 만료 배지(critical/upcoming)에 hover 시 "Expires within N
days" tooltip이 뜨도록 DVVaultContainer에 trailingIconTooltip을
추가했다. 문구는 SecretExpiryPolicy 값을 그대로 참조해 만든다.
기존엔 이미 만료된 시크릿도 critical 배지가 떴는데("조치가 같으므로
구분할 이유 없음"이 원래 정책이었다), Expired 탭이 "이미 지남"을
전담하므로 다른 탭에서까지 배지로 중복 표시할 필요가 없다는 쪽으로
정책을 바꿨다. SecretExpiryStatus.init이 이미 지난 만료일을 nil로
판정하도록 수정하고, 관련 테스트·프리뷰 주석을 갱신했다.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SwiftUI .help(_:)를 List 행 안의 배지에 붙였더니 완전히 무반응이었다. AppKit NSView.toolTip으로 직접 우회해도 마찬가지였다 — mouseEntered/ mouseExited는 정상적으로 도달하는데(직접 로그로 검증) 시스템 tooltip 렌더링 자체가 이 SwiftUI 호스팅 컨텍스트에서 안 됐다. hover 상태를 NSTrackingArea로 직접 감지해 별도 borderless NSPanel에 말풍선을 그리는 방식으로 바꿨다. 화면 절대 좌표에 띄우므로 List가 자기 폭 밖 콘텐츠를 잘라내는 문제도 함께 해결된다(처음엔 SwiftUI overlay로 그렸다가 텍스트가 잘려서 이 방식으로 갔다). 디자인은 macOS 시스템 tooltip과 맞춰 밝은 회색 배경 + 어두운 텍스트, 위치는 배지 아래쪽으로 뒀다.
- SecretExpiryStatus에 .expired 복원. "이미 지난 건 배지 제외"는 판정 타입이 아니라 소비처(SecretListView.row)가 결정하게 옮겨서, 조회 화면(DetailExpireDateFieldView)의 강조가 다시 뜬다. - ExpiryBucket.title의 7/30일 리터럴을 SecretExpiryPolicy 보간으로. 로컬라이즈도 함께 처리(.module). - 알림 스케줄러가 SecretExpiryPolicy 대신 독립 상수 notificationLeadDays를 쓰도록 분리. cancel이 그 값으로 identifier를 재구성해서, 참조하면 값 변경 시 이전 예약을 못 지운다. - stale 주석 정리 (SecretExpiryStatus, SecretListView, SecretExpiryPolicy). - SecretExpiryPolicyTests에 실제 값(3/7/30) 고정 테스트 추가 — 순서 검증만으로는 값이 바뀌어도 항상 통과한다.
- showTooltip()이 기존 패널을 안 지우고 덮어써서, mouseExited 없이 mouseEntered가 다시 오면(트래킹 재등록 시) 이전 패널이 참조를 잃고 화면에 영구히 남을 수 있었다. 첫 줄에서 hideTooltip() 호출, deinit 에서도 정리, isReleasedWhenClosed = false 추가. - TrackingView가 hitTest를 안 막아서 배지 클릭이 List 행 선택/ 컨텍스트 메뉴로 못 갈 수 있었다. hover 전용이라 hitTest에서 뺐다.
093a78a to
fdbf735
Compare
✨ What's this PR?
📌 관련 이슈 (Related Issue)
🧶 주요 변경 내용 (Summary)
1. 만료 관련 기간 상수를
SecretExpiryPolicy로 통합이슈 등록 시점엔 세 곳이었지만, 그사이 머지된 PR #82(시크릿 조회 화면)가 3/7일 배지(
SecretExpiryStatus/DVExpiryEmphasis)를 이미 구현하면서 네 곳으로 늘어나 있었습니다.SecretExpiryStatus(DVPresentation)ScheduleSecretExpiryNotificationsUseCaseImpl.daysBeforeExpiry(DVDomain)[7, 3]ExpiryBucket(SecretListView, private)SecretQuery.Collection.expiringSoonWindowDays(DVDomain)값이 우연히 일치할 뿐 각자 리터럴로 적혀 있어, 한 곳만 바뀌면 나머지가 조용히 어긋나는 구조였습니다.
DVDomain에SecretExpiryPolicyenum을 신설해criticalWindowDays(3) /upcomingWindowDays(7) /listingWindowDays(30) 세 상수를 단일 소유하게 하고, 네 소비처가 전부 이 값을 참조하도록 변경했습니다.판정 로직은 통합하지 않았습니다. "뭘 배지로 보여줄지"(critical/upcoming 2단계)와 "뭘 섹션으로 나눌지"(expired/7일/30일 3단계)는 목적이 달라 억지로 하나의 타입으로 합치면 오히려 부자연스럽습니다. 통합한 건 숫자뿐입니다.
2. 만료 배지에 tooltip 추가, 이미 만료된 항목은 배지 대상에서 제외
목록 행의 만료 배지에 hover 시 "Expires within N days" 문구가 뜨도록
DVVaultContainer에trailingIconTooltip을 추가했습니다. 문구는SecretExpiryPolicy값을 그대로 참조해 만듭니다.이 작업 중 기존 정책(PR #82, "이미 만료됐어도 critical 배지로 함께 보여준다 — 조치가 같으므로")을 재검토했습니다. Expired 탭이 "이미 지남"을 이미 전담해서 보여주므로, All/Star 같은 다른 탭에서까지 배지로 또 강조하는 건 중복입니다.
SecretExpiryStatus.init이 이미 지난 만료일을nil(표시 없음)로 판정하도록 바꿨습니다.📸 스크린샷 (Optional)
🧪 테스트 / 검증 내역
xcodebuild testDVDomain 타깃 — 180 tests passed (신규SecretExpiryPolicyTests포함)xcodebuild testDVPresentation 타깃 — 161 tests passed (판정 변경·tooltip 테스트 포함)xcodebuild buildDVDesign 타깃 — BUILD SUCCEEDEDgit stash로 develop 기준 재현 확인 → 이 PR과 무관한 기존 이슈입니다💬 기타 공유 사항
SecretExpiryStage라는 판정 로직 포함 타입을 제안했지만, PR #82에서 이미 같은 목적의 타입(SecretExpiryStatus+DVExpiryEmphasis)이 만들어져 있어 중복 구현하지 않고 "값의 단일 소스화"로 범위를 좁혔습니다."Expires within %lld days")이Localizable.xcstrings에 아직 자동 추출되지 않았습니다. CLI 빌드(xcodebuild)로는 String Catalog 추출이 트리거되지 않는 것으로 보이며, Xcode IDE에서 빌드하면 채워질 것으로 예상합니다. 번역 자체는 Feature: Localization (ko/en) 적용 #87(Localization) 범위입니다.Calendar.date(byAdding:.day:), DST 인식)와 배지/섹션(86_400초 고정 계산)이 값은 같은 소스를 보게 됐지만 계산 방식은 여전히 다릅니다. 이번 PR 범위 밖이라 건드리지 않았습니다.SecretExpiryPolicy를DVDomain/Sources/Entity/에 뒀는데,Secret/Project같은 실제 엔티티와 성격이 달라(정책 상수) 위치가 다소 어색할 수 있습니다. 더 나은 위치 제안 있으면 반영하겠습니다.🙇🏻♀️ 리뷰 가이드 (선택)
SecretExpiryStatus.swift의 판정 로직 변경(expiresAt > now조건 추가) — 이게 이번 PR에서 가장 논쟁적일 수 있는 지점입니다. 정책 변경이 맞는지 짚어주세요.DVVaultContainer.swift—trailingIcon과trailingIconTooltip을 별도 옵셔널 파라미터 두 개로 둔 설계. 항상 짝을 이뤄야 하는 값이라 하나의 타입으로 묶는 게 나을지 의견 주시면 좋겠습니다.SecretExpiryPolicy.swift— 상수만 소유하고 판정 로직을 의도적으로 배제한 설계가 타당한지 확인 부탁드립니다.