Conversation
WalkthroughAPI 버전 관리 체계를 재구성하고, 감정 시스템을 다층 구조(PrimaryEmotion, DetailEmotion)로 개편하며, RecordInfo의 페이지 번호를 선택형으로 변경하고 UI 컴포넌트와 프레젠테이션 계층을 이에 맞게 업데이트합니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant Presentation
participant Domain
participant Data
participant Network
User->>Presentation: 기록 상세 화면 진입
Presentation->>Domain: FetchDetailEmotionsUseCase.execute(primaryEmotion)
Domain->>Data: EmotionRepository.getDetailEmotions(primaryEmotion)
Data->>Network: EmotionAPI.fetchEmotions()
Network-->>Data: EmotionListResponseDTO
Data->>Data: DTO → 도메인 모델 변환 (toDomain())
Data->>Data: 메모리 캐시에 저장
Data-->>Domain: AnyPublisher<[EmotionGroup], DomainError>
Domain-->>Presentation: [DetailEmotion]
Presentation->>Presentation: BKBottomSheetViewController.makeDetailEmotionSheet 구성
Presentation-->>User: 감정 선택 UI 표시
User->>Presentation: 세부 감정 선택 및 확인
Presentation->>Domain: RecordInfo(primaryEmotion, detailEmotions, ...)
Domain->>Data: InsertRecordRequestDTO 변환
Data->>Network: POST /api/v1/records (primaryEmotion, detailEmotionTagIds)
Network-->>Data: InsertRecordResponseDTO
Data-->>Presentation: RecordInfo
Presentation-->>User: 기록 저장 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/Projects/BKData/Sources/DTO/Response/EmotionResponseDTO.swift`:
- Around line 46-53: PrimaryEmotionDTO.toDomain() currently returns nil on
unknown codes which is inconsistent with PrimaryEmotionResponseDTO.toDomain()
that uses PrimaryEmotion(rawValue: code) ?? .other; update
PrimaryEmotionDTO.toDomain() to use the same fallback by returning
PrimaryEmotion(rawValue: code) ?? .other so both DTOs map unknown/unsupported
codes to .other consistently (refer to PrimaryEmotionDTO, toDomain(),
PrimaryEmotionResponseDTO, and PrimaryEmotion).
In `@src/Projects/BKDesign/Sources/Components/Chip/BKChip.swift`:
- Around line 71-83: The layout currently adds
titleLabel.trailing.equalToSuperview() only when count == 0 in setupLayout(),
which will conflict if count changes later; capture the trailing constraint into
a stored property (e.g., titleTrailingConstraint) when creating constraints in
setupLayout() and do not create conflicting constraints for countLabel, then
toggle titleTrailingConstraint.isActive (or activate/deactivate it) inside the
count property's didSet to enable the trailing constraint when count == 0 and
disable it when count != 0 so constraints remain consistent as count changes;
update references to titleLabel, countLabel, setupLayout(), and the count
property accordingly.
- Line 55: Remove the redundant swiftlint directive "swiftlint:disable
empty_count" at the duplicate location in BKChip.swift (the second occurrence
near the BKChip component) since the rule is already disabled earlier in the
file; locate the duplicate directive string "swiftlint:disable empty_count" and
delete that line so only the first disabling (earlier in the file) remains.
In
`@src/Projects/BKPresentation/Sources/Common/Extension/BKBottomSheetViewController`+.swift:
- Around line 276-278: The hardcoded Korean particle "을" in the
BKBottomSheetViewController title should be chosen dynamically based on
primaryEmotion.displayName (handle cases like "기타" which require "를"); update
the call that constructs the title in BKBottomSheetViewController to use a small
utility (e.g., a josa/postposition helper) that inspects the last syllable of
primaryEmotion.displayName (or checks for .other) and returns the correct
particle ("을" vs "를"), then use the resulting particle in the interpolated title
string so the grammar is correct for all displayName values.
- Around line 298-308: The closure assigned to chip.onTap currently captures
chip strongly and creates a retain cycle; update the closure capture list to
capture chip (and sheet) weakly (e.g., [weak chip, weak sheet]), then inside the
closure unwrap chip (guard let chip = chip else { return }) before mutating
chip.isSelected and updating selectedDetailEmotions, and use sheet? when calling
sheet.button?.setPrimaryButtonState(!selectedDetailEmotions.isEmpty); this
removes the strong retain on chip while preserving the existing logic in the
chip.onTap handler.
In
`@src/Projects/BKPresentation/Sources/Common/Extension/PrimaryEmotion`+UI.swift:
- Around line 123-131: PrimaryEmotion.from(seedName:) is missing handling for
the "etc"/"기타" seed names causing it to return nil when EmotionSeed would map to
.etc; update the switch in PrimaryEmotion.from(seedName:) to include case "etc",
"기타" and return .other (PrimaryEmotion.other) so the mapping matches
EmotionSeed.from(seedName:) and Korean label "기타" is handled.
In
`@src/Projects/BKPresentation/Sources/MainFlow/BookDetail/View/SentenceCardView.swift`:
- Around line 118-122: The configure logic in SentenceCardView (inside the
method configuring emotionBackgroundImageView) leaves previous state when
switching between emotions; update the branch so when data.primaryEmotion !=
.other you set emotionBackgroundImageView.image = data.primaryEmotion.cardImage
and also reset emotionBackgroundImageView.backgroundColor = .clear (or nil), and
when data.primaryEmotion == .other set
emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary) and also
set emotionBackgroundImageView.image = nil; modify the configure method (or the
surrounding code that sets emotionBackgroundImageView) to explicitly clear the
unused property in each branch to avoid residual images or colors.
In
`@src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/CollectedSentenceView.swift`:
- Around line 83-87: The pageLabel.isHidden flag is only set to true when page
is nil and not restored when apply is later called with a non-nil page; update
the apply (or the method that contains this diff) to explicitly set
pageLabel.isHidden = false when page is non-nil (and still set text via
pageLabel.setText(text: "\(page)p")), or else always set pageLabel.isHidden =
(page == nil) so the visibility is correctly toggled whenever apply/page
changes; refer to pageLabel, page and the apply method/CollectedSentenceView to
locate the change.
🧹 Nitpick comments (14)
src/Projects/BKData/Sources/API/RecordAPI.swift (1)
11-11: enum case 이름과 실제 HTTP 메서드 불일치
.patchcase가 실제로는.put을 반환합니다. 주석으로 V2 API 변경 사항을 설명하고 있지만, case 이름 자체가 의미와 맞지 않아 혼동을 줄 수 있습니다. case 이름을.update등으로 변경하거나, 최소한 Line 11의 파라미터 레이블(readingRecordId)과 함께 리네이밍을 고려해 주세요.Also applies to: 43-44
src/Projects/BKData/Sources/Repository/DefaultRecordRepository.swift (1)
50-52: 더 견고한 감정 매핑 방식 권장
Emotion(rawValue: $0.displayName)은displayName이 사용자 표시용 한글 문자열이므로, API 응답 스펙 변경이나 현지화 시 깨질 수 있습니다. 대신code필드를 사용하여PrimaryEmotion으로 먼저 변환한 후Emotion으로 변환하세요:let emotion: Emotion? = $0.representativeEmotion.flatMap { PrimaryEmotion(rawValue: $0.code)?.toEmotion() }또는
PrimaryEmotionResponseDTO의toDomain()메서드를 확장하여 직접Emotion을 반환하도록 할 수 있습니다.src/Projects/BKPresentation/Sources/MainFlow/BookDetail/View/BookDetailView.swift (1)
22-22: 페이지가 동일하거나 nil인 경우 정렬 순서가 불안정합니다
page가nil인 항목들은 모두0으로 치환되어 동일한 정렬 키를 갖게 되며, 기존의createdAt기반 tie-break 로직이 제거되어 상대적 순서가 비결정적입니다. 안정적인 정렬을 위해 보조 정렬 기준을 유지하는 것을 고려해 주세요.♻️ 보조 정렬 기준 추가 제안
case .pageDescending: - return { ($0.page ?? 0) > ($1.page ?? 0) } + return { + let lhs = $0.page ?? 0 + let rhs = $1.page ?? 0 + if lhs != rhs { return lhs > rhs } + return $0.createdAt > $1.createdAt + }src/Projects/BKPresentation/Sources/MainFlow/BookDetail/Models/EmotionSeed.swift (1)
15-33:.etc의image와circleImage가 다른 감정의 에셋을 재사용하고 있습니다.
image는BKImage.Graphics.sad를,circleImage는BKImage.Graphics.insightCircle을 사용 중입니다. 디자인 에셋이 아직 준비되지 않은 것이라면, 향후 교체가 필요하다는 TODO 코멘트를 남겨두면 추적에 도움이 될 것 같습니다.src/Projects/BKData/Sources/DTO/Response/PrimaryEmotionResponseDTO.swift (1)
5-12:PrimaryEmotionResponseDTO와PrimaryEmotionDTO(EmotionResponseDTO.swift 내)가 유사한 역할을 하고 있습니다.
PrimaryEmotionResponseDTO.toDomain()은.other폴백이 내장된 non-optional을 반환하고,PrimaryEmotionDTO.toDomain()은 optional을 반환합니다. 동일 필드(code) 기반이므로, 하나의 DTO로 통합하면 매핑 일관성이 개선될 수 있습니다.#!/bin/bash # PrimaryEmotionResponseDTO와 PrimaryEmotionDTO의 사용처 확인 rg -n "PrimaryEmotionResponseDTO\|PrimaryEmotionDTO" --type swift -C 2src/Projects/BKData/Sources/API/EmotionAPI.swift (1)
28-32: GET 요청에Content-Type: application/json헤더는 불필요합니다.Body가 없는 GET 요청에는
Content-Type헤더가 의미가 없습니다. 다른 API 타겟들도 동일한 패턴을 사용하고 있다면 프로젝트 관례로 무시해도 됩니다.src/Projects/BKDomain/Sources/Entity/PrimaryEmotion.swift (2)
25-33:description프로퍼티가CustomStringConvertible.description을 섀도잉할 수 있습니다.
Stringraw value를 가진 enum은 자동으로CustomStringConvertible을 따르며,description이 raw value를 반환합니다. 여기서description을 재정의하면print(PrimaryEmotion.warmth)또는 문자열 보간 시 raw value("WARMTH") 대신 한글 설명("공감과 위로가 된 순간")이 출력됩니다. 의도된 동작이라면 괜찮지만, 디버깅 시 혼란을 줄 수 있으므로emotionDescription등 별도 이름 사용을 권장합니다.
36-44:toEmotion()과PrimaryEmotion+UI.swift의toLegacyEmotion이 동일한 매핑을 중복 구현합니다.
PrimaryEmotion+UI.swiftLine 108-116의toLegacyEmotioncomputed property가 이toEmotion()메서드와 완전히 동일한 로직입니다. 하나로 통일하면 유지보수 시 불일치 위험을 줄일 수 있습니다.src/Projects/BKDesign/Sources/Components/Chip/BKRemovableChip.swift (1)
52-56: 하드코딩된 배경색이 디자인 시스템 패턴을 벗어납니다.
UIColor(hex: "#E3F8E9")가 직접 사용되고 있지만, 기존BKChip은.bkBaseColor(.primary)같은 시맨틱 컬러를 사용합니다. 디자인 시스템에 해당 색상을 추가하거나, 기존 시맨틱 컬러를 활용하는 것이 일관성과 다크모드 대응에 유리합니다.src/Projects/BKPresentation/Sources/Common/Extension/BKBottomSheetViewController+.swift (1)
360-418:layoutSubviews와intrinsicContentSize에 행 그룹핑 로직이 중복되어 있습니다.칩을 행별로 그룹화하는 알고리즘이
layoutSubviews(Line 370-389)와intrinsicContentSize(Line 425-444)에서 거의 동일하게 반복됩니다. 공통 헬퍼 메서드로 추출하면 유지보수성이 개선됩니다.또한,
layoutSubviews끝(Line 417)에서invalidateIntrinsicContentSize()를 호출하면 레이아웃 사이클이 반복될 위험이 있습니다.bounds변경 시에만 호출하도록 가드를 추가하는 것을 권장합니다.Also applies to: 420-454
src/Projects/BKPresentation/Sources/Common/Extension/PrimaryEmotion+UI.swift (1)
82-90: 공유 배경색이 하드코딩된 hex 값을 사용합니다.인스타 스토리 공유 전용이라 이해되지만, 디자인 시스템의 시맨틱 컬러로 관리하면 향후 디자인 변경 시 일괄 수정이 용이합니다. 당장은 아니더라도 참고해 주세요.
src/Projects/BKData/Sources/DTO/Response/DetailRecordResponseDTO.swift (1)
43-77: V2 DTO 매핑도 정상입니다.
PrimaryEmotionResponseDTO.toDomain()이 non-optional을 반환하므로 폴백 없이 직접 사용하는 것이 맞습니다.detailEmotions가[DetailEmotionResponseDTO?]에서[DetailEmotionResponseDTO]로 변경된 것도 API 스펙 정리에 부합합니다.다만, V1(
PrimaryEmotionDTO/DetailEmotionDTO)과 V2(PrimaryEmotionResponseDTO/DetailEmotionResponseDTO)의 네이밍 규칙이 다릅니다. 장기적으로 네이밍을 통일하면 가독성이 향상됩니다.src/Projects/BKData/Sources/Repository/DefaultEmotionRepository.swift (2)
24-28:.map내부에서cacheQueue.sync사용 시 잠재적 데드락 주의
networkProvider.request의 결과가cacheQueue와 동일한 스레드에서 전달될 경우sync호출이 데드락을 유발할 수 있습니다. 현실적으로 네트워크 응답이 이 큐에서 전달될 가능성은 낮지만, 안전하게async로 변경하는 것을 고려해 주세요. 캐시 쓰기는 반환값에 영향을 주지 않으므로async로도 충분합니다.♻️ 제안
.map { [weak self] response in let emotions = response.emotions.compactMap { $0.toDomain() } - self?.cacheQueue.sync { + self?.cacheQueue.async { self?.cachedEmotions = emotions } return emotions
34-47: 캐시 미스 시 동시 다중 fetch 가능성
getEmotions()에서 캐시가nil일 때fetchEmotions()를 호출하는데, 여러 곳에서 동시에getEmotions()를 호출하면 중복 네트워크 요청이 발생할 수 있습니다. 현재 규모에서 큰 문제는 아니지만, 향후 필요 시 진행 중인 요청을 공유하는 패턴(share()또는 request deduplication)을 적용하면 좋습니다.
| struct PrimaryEmotionDTO: Decodable { | ||
| let code: String | ||
| let displayName: String | ||
|
|
||
| func toDomain() -> PrimaryEmotion? { | ||
| return PrimaryEmotion(rawValue: code) | ||
| } | ||
| } |
There was a problem hiding this comment.
PrimaryEmotionDTO.toDomain()의 fallback 처리 불일치
PrimaryEmotionResponseDTO.swift의 toDomain()은 PrimaryEmotion(rawValue: code) ?? .other로 fallback을 제공하지만, 이 파일의 PrimaryEmotionDTO.toDomain()은 nil을 반환합니다. 같은 도메인 타입을 변환하는데 fallback 전략이 다르면 호출부에서 혼란이 생길 수 있습니다. 의도된 차이인지 확인해 주세요.
🤖 Prompt for AI Agents
In `@src/Projects/BKData/Sources/DTO/Response/EmotionResponseDTO.swift` around
lines 46 - 53, PrimaryEmotionDTO.toDomain() currently returns nil on unknown
codes which is inconsistent with PrimaryEmotionResponseDTO.toDomain() that uses
PrimaryEmotion(rawValue: code) ?? .other; update PrimaryEmotionDTO.toDomain() to
use the same fallback by returning PrimaryEmotion(rawValue: code) ?? .other so
both DTOs map unknown/unsupported codes to .other consistently (refer to
PrimaryEmotionDTO, toDomain(), PrimaryEmotionResponseDTO, and PrimaryEmotion).
| updateAppearance() | ||
| } | ||
|
|
||
| // swiftlint:disable empty_count |
There was a problem hiding this comment.
중복된 swiftlint:disable 지시문
Line 23에서 이미 empty_count 규칙이 비활성화되어 있으므로, Line 55의 중복 swiftlint:disable은 불필요합니다. 정적 분석 도구에서도 이를 경고하고 있습니다.
🧹 수정 제안
- // swiftlint:disable empty_count
private func setupViews() {📝 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.
| // swiftlint:disable empty_count | |
| private func setupViews() { |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 55-55: The disabled 'empty_count' rule was already disabled
(blanket_disable_command)
[Warning] 55-55: Use 'next', 'this' or 'previous' instead to disable the 'empty_count' rule once, or re-enable it as soon as possible`
(blanket_disable_command)
🤖 Prompt for AI Agents
In `@src/Projects/BKDesign/Sources/Components/Chip/BKChip.swift` at line 55,
Remove the redundant swiftlint directive "swiftlint:disable empty_count" at the
duplicate location in BKChip.swift (the second occurrence near the BKChip
component) since the rule is already disabled earlier in the file; locate the
duplicate directive string "swiftlint:disable empty_count" and delete that line
so only the first disabling (earlier in the file) remains.
| private func setupLayout() { | ||
| titleLabel.snp.makeConstraints { | ||
| $0.leading.top.bottom.equalToSuperview() | ||
| if count == 0 { | ||
| $0.trailing.equalToSuperview() | ||
| } | ||
| } | ||
|
|
||
| countLabel.snp.makeConstraints { | ||
| $0.leading.equalTo(titleLabel.snp.trailing).offset(BKSpacing.spacing1) | ||
| $0.trailing.top.bottom.equalToSuperview() | ||
| $0.trailing.equalToSuperview() | ||
| $0.centerY.equalTo(titleLabel) | ||
| } |
There was a problem hiding this comment.
count가 동적으로 변경될 때 레이아웃 충돌 가능성
setupLayout()에서 count == 0일 때만 titleLabel에 trailing.equalToSuperview() 제약을 추가합니다(Line 74-76). 이 제약은 초기화 시점에 한 번만 설정되므로, 이후 count가 0에서 non-zero로 변경되면 titleLabel.trailing == superview와 countLabel.leading == titleLabel.trailing + countLabel.trailing == superview 제약이 동시에 활성화되어 충돌이 발생할 수 있습니다.
count가 초기화 이후 변경되지 않는다면 문제없지만, count 프로퍼티에 didSet이 있으므로 동적 변경이 의도된 것으로 보입니다. 제약을 참조로 저장하고 count 변경 시 활성화/비활성화하는 방식을 고려해 주세요.
🤖 Prompt for AI Agents
In `@src/Projects/BKDesign/Sources/Components/Chip/BKChip.swift` around lines 71 -
83, The layout currently adds titleLabel.trailing.equalToSuperview() only when
count == 0 in setupLayout(), which will conflict if count changes later; capture
the trailing constraint into a stored property (e.g., titleTrailingConstraint)
when creating constraints in setupLayout() and do not create conflicting
constraints for countLabel, then toggle titleTrailingConstraint.isActive (or
activate/deactivate it) inside the count property's didSet to enable the
trailing constraint when count == 0 and disable it when count != 0 so
constraints remain consistent as count changes; update references to titleLabel,
countLabel, setupLayout(), and the count property accordingly.
| let sheet = BKBottomSheetViewController( | ||
| title: "어떤 '\(primaryEmotion.displayName)'을 느꼈나요?", | ||
| subtitle: "더 자세한 감정을 선택 기록할 수 있어요.", |
There was a problem hiding this comment.
"기타" 감정일 때 한글 조사 "을"이 문법적으로 맞지 않습니다.
"어떤 '\(primaryEmotion.displayName)'을 느꼈나요?" — 현재 모든 displayName이 받침(ㅁ)으로 끝나 "을"이 맞지만, "기타"는 모음으로 끝나므로 "를"이어야 합니다. 이 시트에 .other가 전달되지 않는다면 문제없지만, 방어적으로 조사 처리를 고려해 주세요.
🤖 Prompt for AI Agents
In
`@src/Projects/BKPresentation/Sources/Common/Extension/BKBottomSheetViewController`+.swift
around lines 276 - 278, The hardcoded Korean particle "을" in the
BKBottomSheetViewController title should be chosen dynamically based on
primaryEmotion.displayName (handle cases like "기타" which require "를"); update
the call that constructs the title in BKBottomSheetViewController to use a small
utility (e.g., a josa/postposition helper) that inspects the last syllable of
primaryEmotion.displayName (or checks for .other) and returns the correct
particle ("을" vs "를"), then use the resulting particle in the interpolated title
string so the grammar is correct for all displayName values.
| chip.onTap = { [weak sheet] in | ||
| if selectedDetailEmotions.contains(detailEmotion) { | ||
| selectedDetailEmotions.remove(detailEmotion) | ||
| chip.isSelected = false | ||
| } else { | ||
| selectedDetailEmotions.insert(detailEmotion) | ||
| chip.isSelected = true | ||
| } | ||
| // 1개 이상 선택되어야 버튼 활성화 | ||
| sheet?.button?.setPrimaryButtonState(!selectedDetailEmotions.isEmpty) | ||
| } |
There was a problem hiding this comment.
chip.onTap 클로저에서 chip을 강하게 캡처하여 순환 참조가 발생합니다.
chip.onTap 클로저가 chip 자체를 강하게 참조하고 있습니다. chip → onTap 클로저 → chip 형태의 retain cycle이 형성되어 메모리 누수가 발생합니다.
🔧 순환 참조 수정 제안
- chip.onTap = { [weak sheet] in
- if selectedDetailEmotions.contains(detailEmotion) {
- selectedDetailEmotions.remove(detailEmotion)
- chip.isSelected = false
- } else {
- selectedDetailEmotions.insert(detailEmotion)
- chip.isSelected = true
- }
+ chip.onTap = { [weak sheet, weak chip] in
+ guard let chip else { return }
+ if selectedDetailEmotions.contains(detailEmotion) {
+ selectedDetailEmotions.remove(detailEmotion)
+ chip.isSelected = false
+ } else {
+ selectedDetailEmotions.insert(detailEmotion)
+ chip.isSelected = true
+ }🤖 Prompt for AI Agents
In
`@src/Projects/BKPresentation/Sources/Common/Extension/BKBottomSheetViewController`+.swift
around lines 298 - 308, The closure assigned to chip.onTap currently captures
chip strongly and creates a retain cycle; update the closure capture list to
capture chip (and sheet) weakly (e.g., [weak chip, weak sheet]), then inside the
closure unwrap chip (guard let chip = chip else { return }) before mutating
chip.isSelected and updating selectedDetailEmotions, and use sheet? when calling
sheet.button?.setPrimaryButtonState(!selectedDetailEmotions.isEmpty); this
removes the strong retain on chip while preserving the existing logic in the
chip.onTap handler.
| static func from(seedName: String) -> Self? { | ||
| switch seedName.lowercased() { | ||
| case "warmth", "따뜻함": return .warmth | ||
| case "joy", "즐거움": return .joy | ||
| case "sad", "sadness", "슬픔": return .sadness | ||
| case "insight", "깨달음": return .insight | ||
| default: return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find PrimaryEmotion.from(seedName:) call sites
rg "from\(seedName:" --type=swift -C 3
# Also search for variations
rg "\.from\(" --type=swift | grep -i emotion | head -20Repository: YAPP-Github/Reed-iOS
Length of output: 3579
🏁 Script executed:
# Look at EmotionSeed.swift to understand its from(seedName:) implementation
fd "EmotionSeed" --type=swift -x cat {}Repository: YAPP-Github/Reed-iOS
Length of output: 236
🏁 Script executed:
# Search for "etc" and "기타" usage in the codebase
rg "\"etc\"|\"기타\"" --type=swift -C 2Repository: YAPP-Github/Reed-iOS
Length of output: 1877
🏁 Script executed:
# Check EmotionSeed.swift to see the complete from(seedName:) implementation
cat src/Projects/BKPresentation/Sources/MainFlow/BookDetail/Models/EmotionSeed.swift | head -100Repository: YAPP-Github/Reed-iOS
Length of output: 3119
🏁 Script executed:
# Check if there's any usage of PrimaryEmotion.from(seedName:) directly (not from(seed:))
rg "PrimaryEmotion.from\(seedName:" --type-list | head -30Repository: YAPP-Github/Reed-iOS
Length of output: 453
🏁 Script executed:
# Check what the Seed entity looks like and its possible name values
rg "struct Seed|class Seed|enum Seed" -A 10Repository: YAPP-Github/Reed-iOS
Length of output: 4398
🏁 Script executed:
# Check if PrimaryEmotion.other is used anywhere
rg "\.other" src/Projects/BKDomain/Sources/Entity/PrimaryEmotion.swift -C 3Repository: YAPP-Github/Reed-iOS
Length of output: 574
from(seedName:)에서 "etc" / "기타" 케이스가 누락되었습니다.
EmotionSeed.from(seedName:)은 "etc", "기타"를 처리하여 .etc를 반환하지만, PrimaryEmotion.from(seedName:)에는 해당 케이스가 없어 nil을 반환합니다. 동일한 seed name이 전달되는 상황에서 불일치가 발생하며, PrimaryEmotion.other는 한국어 레이블 "기타"를 가지므로 이를 처리해야 합니다.
누락 케이스 추가 제안
static func from(seedName: String) -> Self? {
switch seedName.lowercased() {
case "warmth", "따뜻함": return .warmth
case "joy", "즐거움": return .joy
case "sad", "sadness", "슬픔": return .sadness
case "insight", "깨달음": return .insight
+ case "etc", "other", "기타": return .other
default: return nil
}
}🤖 Prompt for AI Agents
In `@src/Projects/BKPresentation/Sources/Common/Extension/PrimaryEmotion`+UI.swift
around lines 123 - 131, PrimaryEmotion.from(seedName:) is missing handling for
the "etc"/"기타" seed names causing it to return nil when EmotionSeed would map to
.etc; update the switch in PrimaryEmotion.from(seedName:) to include case "etc",
"기타" and return .other (PrimaryEmotion.other) so the mapping matches
EmotionSeed.from(seedName:) and Korean label "기타" is handled.
| if data.primaryEmotion != .other { | ||
| emotionBackgroundImageView.image = data.primaryEmotion.cardImage | ||
| } else { | ||
| emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary) | ||
| } |
There was a problem hiding this comment.
뷰 재설정 시 이전 상태가 남을 수 있습니다.
configure가 다시 호출될 경우, .other가 아닌 감정에서 .other로 변경되면 이전에 설정된 image가 남아있고, 반대의 경우 backgroundColor가 잔존합니다. 각 분기에서 사용하지 않는 속성을 초기화해야 합니다.
🐛 수정 제안
public func configure(_ data: BookDetailItem) {
sentenceLabel.setText(text: data.note)
titleLabel.setText(text: data.bookTitle.withCornerBrackets())
if data.primaryEmotion != .other {
emotionBackgroundImageView.image = data.primaryEmotion.cardImage
+ emotionBackgroundImageView.backgroundColor = .clear
} else {
+ emotionBackgroundImageView.image = nil
emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary)
}
}📝 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.
| if data.primaryEmotion != .other { | |
| emotionBackgroundImageView.image = data.primaryEmotion.cardImage | |
| } else { | |
| emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary) | |
| } | |
| if data.primaryEmotion != .other { | |
| emotionBackgroundImageView.image = data.primaryEmotion.cardImage | |
| emotionBackgroundImageView.backgroundColor = .clear | |
| } else { | |
| emotionBackgroundImageView.image = nil | |
| emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary) | |
| } |
🤖 Prompt for AI Agents
In
`@src/Projects/BKPresentation/Sources/MainFlow/BookDetail/View/SentenceCardView.swift`
around lines 118 - 122, The configure logic in SentenceCardView (inside the
method configuring emotionBackgroundImageView) leaves previous state when
switching between emotions; update the branch so when data.primaryEmotion !=
.other you set emotionBackgroundImageView.image = data.primaryEmotion.cardImage
and also reset emotionBackgroundImageView.backgroundColor = .clear (or nil), and
when data.primaryEmotion == .other set
emotionBackgroundImageView.backgroundColor = .bkBaseColor(.secondary) and also
set emotionBackgroundImageView.image = nil; modify the configure method (or the
surrounding code that sets emotionBackgroundImageView) to explicitly clear the
unused property in each branch to avoid residual images or colors.
| if let page = page { | ||
| pageLabel.setText(text: "\(page)p") | ||
| } else { | ||
| pageLabel.isHidden = true | ||
| } |
There was a problem hiding this comment.
pageLabel.isHidden이 다시 false로 복원되지 않는 문제
page가 nil일 때 pageLabel.isHidden = true로 설정하지만, 이후 apply가 non-nil page로 다시 호출될 경우 isHidden이 false로 복원되지 않습니다.
🛡️ 수정 제안
if let page = page {
+ pageLabel.isHidden = false
pageLabel.setText(text: "\(page)p")
} else {
pageLabel.isHidden = true
}📝 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.
| if let page = page { | |
| pageLabel.setText(text: "\(page)p") | |
| } else { | |
| pageLabel.isHidden = true | |
| } | |
| if let page = page { | |
| pageLabel.isHidden = false | |
| pageLabel.setText(text: "\(page)p") | |
| } else { | |
| pageLabel.isHidden = true | |
| } |
🤖 Prompt for AI Agents
In
`@src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/CollectedSentenceView.swift`
around lines 83 - 87, The pageLabel.isHidden flag is only set to true when page
is nil and not restored when apply is later called with a non-nil page; update
the apply (or the method that contains this diff) to explicitly set
pageLabel.isHidden = false when page is non-nil (and still set text via
pageLabel.setText(text: "\(page)p")), or else always set pageLabel.isHidden =
(page == nil) so the visibility is correctly toggled whenever apply/page
changes; refer to pageLabel, page and the apply method/CollectedSentenceView to
locate the change.
doyeonk429
left a comment
doyeonk429
left a comment
There was a problem hiding this comment.
508 리뷰로 한번에 리뷰 완료하였습니다. 토끼 리뷰만 선택적으로 반영하시면 될 것 같습니다. 수고하셨습니다 👍
2 participants
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
🧪 테스트 내역
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항