[Feat] #905 - 솝레터 게시물 수정, 삭제 API 연동#916
Open
kwonseokki12 wants to merge 7 commits into
Open
Conversation
Summary by CodeRabbit
Walkthrough솝레터 메시지 수정/삭제 기능이 API·서비스·리포지토리·유스케이스 전 계층에 걸쳐 추가되었습니다. 상세 화면에는 텍스트 편집 UI와 완료/취소 바인딩이 도입되었고, 메인 화면에는 새로고침 트리거가 추가되었으며, 코디네이터는 온보딩 완료 여부에 따른 분기와 편집/삭제 완료 후 목록 새로고침 라우팅을 지원하도록 변경되었습니다. Changes솝레터 메시지 수정/삭제 기능
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DetailVC as SoptletterDetailModalVC
participant DetailVM as SoptletterDetailViewModel
participant UseCase as DefaultSoptletterUseCase
participant Repository as SoptletterRepository
participant Service as DefaultSoptletterService
participant Coordinator as SoptletterCoordinator
participant MainVM as SoptletterMainViewModel
DetailVC->>DetailVM: editCompleteButtonTap(content) / deleteButtonTap(content)
DetailVM->>UseCase: editMessage(...) / deleteMessage(...)
UseCase->>Repository: editMessage(...) / deleteMessage(...)
Repository->>Service: editMessage(...) / deleteMessage(...)
Service-->>Repository: 결과 반환
Repository-->>UseCase: 결과 반환
UseCase-->>DetailVM: 결과 반환
DetailVM->>DetailVM: soptletterEditCompleted / soptletterDeleteCompleted 전송
DetailVM->>Coordinator: onEditCompleted() / onDeleteCompleted()
Coordinator->>MainVM: refreshMessagesTrigger()
MainVM->>MainVM: fetchMessages(output:)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (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: 9
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift (1)
24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
soptletterMain을 IUO 대신 안전한 옵셔널로 관리 권장
private var soptletterMain: SoptletterMainPresentable!로 선언되어, 초기화 전에presentSoptletterDetail의onEditCompleted/onDeleteCompleted(Line 146-152)에서 접근하면 크래시합니다. 현재 호출 순서상(showSoptletterMain()이 항상 먼저 실행되어onCellTap을 세팅) 즉각적인 위험은 낮지만, 향후 코드 변경 시 방어가 없어 취약합니다.♻️ 제안 예시
- private var soptletterMain: SoptletterMainPresentable! + private var soptletterMain: SoptletterMainPresentable?그리고 사용처는 옵셔널 체이닝으로 변경:
- soptletterDetail.vm.onEditCompleted = { [weak self] in - self?.soptletterMain.vm.refreshMessagesTrigger() - } + soptletterDetail.vm.onEditCompleted = { [weak self] in + self?.soptletterMain?.vm.refreshMessagesTrigger() + }🤖 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 `@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift` at line 24, `soptletterMain`이 IUO라서 초기화 전에 `presentSoptletterDetail`의 `onEditCompleted`/`onDeleteCompleted`에서 접근하면 크래시할 수 있습니다. `SoptletterCoordinator`의 `soptletterMain`을 안전한 옵셔널로 바꾸고, 이를 사용하는 `showSoptletterMain`, `presentSoptletterDetail`, 그리고 `onCellTap`/완료 클로저 연결부에서 옵셔널 바인딩 또는 옵셔널 체이닝으로 접근하도록 수정해 초기화 여부를 보장하세요.
🤖 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 `@SOPT-iOS/Projects/Core/Sources/Extension/Foundation`+/String+.swift:
- Around line 77-89: The toMMDDFormat() parsing currently assumes a fixed
yyyy-MM-dd'T'HH:mm:ss.SSSSSS input, which does not match the project’s ISO
formats used by createdAt. Update String+.swift so toMMDDFormat() parses the
same ISO variants as the rest of the codebase (for example by using
ISO8601DateFormatter or by aligning the input formatter with .iso and
.isoWithoutMillis), then keep the MM.dd. output formatting unchanged.
In `@SOPT-iOS/Projects/Domain/Sources/UseCase/SoptletterUseCase.swift`:
- Around line 70-73: `SoptletterUseCase.editMessage` is bypassing the same
content validation used by `writeMessage`, so add the `isWritable(content:)`
check before calling `repository.editMessage`. Reuse the existing validation
flow in `SoptletterUseCase` to reject empty content and messages over 250
characters, and only delegate to `editMessage(messageId:topicId:content:)` when
the content passes. Keep the behavior consistent with `writeMessage` so both
create and edit paths enforce the same domain rule.
In
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift`:
- Around line 41-46: 온보딩 종료 후 메인으로 넘어갈 때 `showSelectTopic()`에서
`showSoptletterMain()`을 바로 호출하면 `soptletterRootController`가 fullScreen으로 남아 메인
화면을 가립니다. `SoptletterCoordinator`의 `showSelectTopic()` 흐름에서 먼저 모달 코디네이터를
dismiss하고, dismiss completion에서 `showSoptletterMain()`을 호출하도록 수정하세요.
`showSoptletterMain()`이 push하는 대상이 `navigationController`임을 유지하되, 반드시
`soptletterRootController` 해제 이후에 실행되게 연결하세요.
In
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterDetailModalVC.swift`:
- Around line 207-216: The confirm action in SoptletterDetailModalVC’s
confirmButtonTap sink allows an empty editedContent to be sent because
updateConfirmButtonState only checks the maximum length. Add a non-empty
validation before calling editCompleteButtonTapPublisher.send in the editing
branch, and make sure the confirm button state in updateConfirmButtonState
reflects both the max-length and minimum-length requirements so “수정완료” cannot be
triggered with blank content.
- Around line 396-409: The cancelEditingMode() flow is missing restoration of
the dateLabel visibility, so it stays hidden after exiting edit mode. Update
SoptletterDetailModalVC.cancelEditingMode() to mirror exitEditingMode() by
explicitly un-hiding dateLabel along with the other non-editing UI elements,
using the existing dateLabel symbol in this view controller.
In
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swift`:
- Around line 84-92: `SoptletterDetailViewModel`의 삭제 완료 콜백이 `deleteButtonTap`의
`customAction`에서 `deleteMessage(...)` 직후 동기적으로 호출되어, 비동기 삭제 완료 전에
`onDeleteCompleted`가 실행되는 문제가 있습니다. `deleteMessage` 내부에서 삭제 Task가 성공한 뒤,
`output.soptletterDeleteCompleted.send()`가 발생한 시점에 `onDeleteCompleted?()`를 호출하도록
옮기고, `deleteButtonTap` 쪽에서는 콜백 호출을 제거해 `deleteMessage`, `onDeleteCompleted`,
`soptletterDeleteCompleted` 흐름이 삭제 성공 이후에만 이어지도록 수정하세요.
- Around line 66-82: In SoptletterDetailViewModel’s editCompleteButtonTap
handling, treat CancellationError as a silent return instead of calling
onError?(), and only invoke onError?() for real failures. Update the Task body
to use owner consistently rather than [weak self], since self is not needed
there, and keep the existing submitTask cancellation flow intact.
- Around line 98-125: `deleteMessage` and `fetchMessages` are accessing instance
properties inside `Task { [weak self] in ... }` without resolving `self`, which
prevents compilation. Update `SoptletterDetailViewModel` to safely unwrap `self`
at the start of each task (for example with `guard let self else { return }`)
and then access `useCase`, `messageId`, and `topicId` through `self` explicitly
throughout the task body.
In
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterMainViewModel.swift`:
- Line 39: The selected topicId is not being passed through to the main screen,
so SoptletterMainViewModel falls back to its default value. Update the flow from
showSelectTopic() through showSoptletterMain() and makeSoptletterMainVC() to
accept and forward topicId, and wire it into SoptletterMainViewModel so the
chosen topic is used instead of the default.
---
Nitpick comments:
In
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift`:
- Line 24: `soptletterMain`이 IUO라서 초기화 전에 `presentSoptletterDetail`의
`onEditCompleted`/`onDeleteCompleted`에서 접근하면 크래시할 수 있습니다.
`SoptletterCoordinator`의 `soptletterMain`을 안전한 옵셔널로 바꾸고, 이를 사용하는
`showSoptletterMain`, `presentSoptletterDetail`, 그리고 `onCellTap`/완료 클로저 연결부에서
옵셔널 바인딩 또는 옵셔널 체이닝으로 접근하도록 수정해 초기화 여부를 보장하세요.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6e83ffaf-7078-4901-8e91-6b3b397a15eb
📒 Files selected for processing (13)
SOPT-iOS/Projects/Core/Sources/Extension/Foundation+/String+.swiftSOPT-iOS/Projects/Data/Sources/Repository/SoptletterRepository.swiftSOPT-iOS/Projects/Domain/Sources/RepositoryInterface/SoptletterRepositoryInterface.swiftSOPT-iOS/Projects/Domain/Sources/UseCase/SoptletterUseCase.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Interface/Sources/SoptletterMainPresentable.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Interface/Sources/SoptletterPresentable.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterDetailModalVC.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterMainVC.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swiftSOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterMainViewModel.swiftSOPT-iOS/Projects/Modules/Networks/Sources/API/SoptletterAPI.swiftSOPT-iOS/Projects/Modules/Networks/Sources/Service/SoptletterService.swift
Comment on lines
+77
to
+89
| func toMMDDFormat() -> String { | ||
| let inputFormatter = DateFormatter() | ||
| inputFormatter.locale = Locale(identifier: "en_US_POSIX") | ||
| inputFormatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSSSSS" | ||
|
|
||
| guard let date = inputFormatter.date(from: self) else { | ||
| return self | ||
| } | ||
|
|
||
| let outputFormatter = DateFormatter() | ||
| outputFormatter.dateFormat = "MM.dd." | ||
| return outputFormatter.string(from: date) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Soptletter createdAt API date format ISO8601 fractional seconds timezone
💡 Result:
In the context of modern API design, particularly for services like Soptletter (which uses standard RESTful practices and camelCase) [1], the createdAt field typically adheres to the ISO 8601 standard [2][3]. When implementing ISO 8601 for an API, the standard approach is to use the RFC 3339 profile [2][4]. The recommended format for a createdAt field is as follows: - Format: YYYY-MM-DDTHH:MM:SS.sssZ [2][5] - Fractional Seconds: Expressed with a decimal mark (e.g.,.sss for milliseconds) [2][3]. While optional, they are recommended for precise event logging [3]. - Timezone: Must always be explicit [5][3]. For UTC, use the 'Z' suffix [2][6]. For other timezones, use a numeric offset (e.g., ±HH:MM) [2][5]. For Soptletter specifically, publicly available technical documentation confirms the use of UTC timestamps with the 'Z' suffix (e.g., 2026-06-29T15:33:31Z) [1]. Best practices for APIs include: - Consistency: Use the same format across all endpoints [5][3]. - Avoid Ambiguity: Never use bare date strings or raw Unix integers without explicit documentation of the unit (seconds vs. milliseconds) [5]. - Serialization: ISO 8601 strings are self-describing and JSON-friendly, making them the preferred choice for most web APIs over epoch timestamps [3].
Citations:
- 1: [FEAT/#1602] 솝레터 메인 1차 API 연동 sopt-android#1610
- 2: https://developer-docs.amazon.com/sp-api/docs/iso-8601
- 3: https://www.devtoolnow.com/guides/unix-timestamp-vs-iso-8601
- 4: https://datatracker.ietf.org/doc/html/rfc3339
- 5: https://timefyi.com/guides/tech-time/timestamps-in-apis/
- 6: https://fleetwork.vn/docs/en/introduction/date-formats
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== file outline ==\n'
ast-grep outline SOPT-iOS/Projects/Core/Sources/Extension/Foundation+/String+.swift --view expanded || true
printf '\n== relevant slice ==\n'
sed -n '1,140p' SOPT-iOS/Projects/Core/Sources/Extension/Foundation+/String+.swift | cat -n
printf '\n== usages of toMMDDFormat ==\n'
rg -n "toMMDDFormat\(\)" SOPT-iOS -g'*.swift' || true
printf '\n== date-like string literals around createdAt / timestamp parsing ==\n'
rg -n 'createdAt|timestamp|yyyy-MM-dd.T.HH:mm:ss|ISO8601|DateFormatter\(' SOPT-iOS -g'*.swift' || trueRepository: sopt-makers/SOPT-iOS
Length of output: 14526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== locate API models with date fields ==\n'
rg -n 'createdAt|updatedAt|date|time' SOPT-iOS/Projects -g'*.swift' | head -n 200
printf '\n== inspect formatter helpers ==\n'
rg -n 'ISO8601DateFormatter|DateFormatter' SOPT-iOS/Projects/Core/Sources -g'*.swift' | head -n 200Repository: sopt-makers/SOPT-iOS
Length of output: 27092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== DateFormatType.swift ==\n'
sed -n '1,120p' SOPT-iOS/Projects/Core/Sources/Enum/DateFormatType.swift | cat -n
printf '\n== DateFormatterManager.swift ==\n'
sed -n '1,140p' SOPT-iOS/Projects/Core/Sources/Utils/DateFormatterManager.swift | cat -n
printf '\n== SoptletterDetailModalVC.swift relevant slice ==\n'
sed -n '150,260p' SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterDetailModalVC.swift | cat -n
printf '\n== Soptletter response/entity fields ==\n'
sed -n '1,120p' SOPT-iOS/Projects/Modules/Networks/Sources/Entity/ResponseEntity/SoptletterItemResponseEntity.swift | cat -n
sed -n '1,120p' SOPT-iOS/Projects/Modules/Networks/Sources/Entity/ResponseEntity/SoptletterMessageResponseEntity.swift | cat -nRepository: sopt-makers/SOPT-iOS
Length of output: 12036
날짜 포맷을 기존 ISO 형식에 맞춰주세요. toMMDDFormat()은 정확히 6자리 소수초만 받아서, 프로젝트에서 쓰는 .iso(...SSS'Z') / .isoWithoutMillis와 형식이 맞지 않습니다. createdAt이 이 형식으로 들어오면 파싱이 실패해 원문이 그대로 노출될 수 있으니 ISO8601DateFormatter를 쓰거나 입력 포맷을 통일하세요.
🤖 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 `@SOPT-iOS/Projects/Core/Sources/Extension/Foundation`+/String+.swift around
lines 77 - 89, The toMMDDFormat() parsing currently assumes a fixed
yyyy-MM-dd'T'HH:mm:ss.SSSSSS input, which does not match the project’s ISO
formats used by createdAt. Update String+.swift so toMMDDFormat() parses the
same ISO variants as the rest of the codebase (for example by using
ISO8601DateFormatter or by aligning the input formatter with .iso and
.isoWithoutMillis), then keep the MM.dd. output formatting unchanged.
Comment on lines
+70
to
+73
|
|
||
| public func editMessage(messageId: Int, topicId: Int, content: String) async throws { | ||
| try await repository.editMessage(messageId: messageId, topicId: topicId, content: content) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
editMessage에 isWritable 유효성 검사 누락
writeMessage(라인 35-40)는 isWritable(content:)로 빈 문자열/최대 글자 수(250자) 초과를 검증한 후 저장소로 위임하지만, 새로 추가된 editMessage는 검증 없이 바로 repository.editMessage로 위임합니다. 동일한 콘텐츠 도메인 규칙(빈 값/글자 수 제한)이 수정 시에도 적용되어야 하며, 그렇지 않으면 빈 내용이나 250자를 초과하는 내용으로 수정이 가능해집니다.
🛠️ 제안 수정
public func editMessage(messageId: Int, topicId: Int, content: String) async throws {
+ guard isWritable(content: content) else {
+ throw SoptletterError.invalidContent
+ }
try await repository.editMessage(messageId: messageId, topicId: topicId, content: content)
}📝 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
| public func editMessage(messageId: Int, topicId: Int, content: String) async throws { | |
| try await repository.editMessage(messageId: messageId, topicId: topicId, content: content) | |
| } | |
| public func editMessage(messageId: Int, topicId: Int, content: String) async throws { | |
| guard isWritable(content: content) else { | |
| throw SoptletterError.invalidContent | |
| } | |
| try await repository.editMessage(messageId: messageId, topicId: topicId, content: content) | |
| } |
🤖 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 `@SOPT-iOS/Projects/Domain/Sources/UseCase/SoptletterUseCase.swift` around
lines 70 - 73, `SoptletterUseCase.editMessage` is bypassing the same content
validation used by `writeMessage`, so add the `isWritable(content:)` check
before calling `repository.editMessage`. Reuse the existing validation flow in
`SoptletterUseCase` to reject empty content and messages over 250 characters,
and only delegate to `editMessage(messageId:topicId:content:)` when the content
passes. Keep the behavior consistent with `writeMessage` so both create and edit
paths enforce the same domain rule.
Comment on lines
+41
to
46
| if UserDefaultKeyList.User.isCompleteSoptletterOnboarding == true { | ||
| showSoptletterMain() | ||
| } else { | ||
| showSoptletterOnboarding() | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# soptletterRootController dismiss 관련 처리가 다른 곳에 있는지 확인
rg -n 'soptletterRootController' SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift
rg -n 'dismiss' SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swiftRepository: sopt-makers/SOPT-iOS
Length of output: 1085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift'
echo '--- lines 45-75 ---'
sed -n '45,75p' "$FILE" | cat -n
echo '--- lines 150-175 ---'
sed -n '150,175p' "$FILE" | cat -nRepository: sopt-makers/SOPT-iOS
Length of output: 2595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift'
echo '--- lines 100-140 ---'
sed -n '100,140p' "$FILE" | cat -nRepository: sopt-makers/SOPT-iOS
Length of output: 1977
온보딩 종료 시 메인 진입 전에 모달 코디네이터를 닫아야 합니다
showSelectTopic()에서 showSoptletterMain()을 호출하면, 메인은 navigationController에 push되지만 온보딩 플로우는 여전히 fullScreen으로 present된 soptletterRootController 안에서 진행됩니다. 이 상태에선 메인 화면이 가려질 수 있으니, soptletterRootController를 dismiss한 뒤 push하거나 dismiss completion에서 메인을 띄워야 합니다.
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swift`
around lines 41 - 46, 온보딩 종료 후 메인으로 넘어갈 때 `showSelectTopic()`에서
`showSoptletterMain()`을 바로 호출하면 `soptletterRootController`가 fullScreen으로 남아 메인
화면을 가립니다. `SoptletterCoordinator`의 `showSelectTopic()` 흐름에서 먼저 모달 코디네이터를
dismiss하고, dismiss completion에서 `showSoptletterMain()`을 호출하도록 수정하세요.
`showSoptletterMain()`이 push하는 대상이 `navigationController`임을 유지하되, 반드시
`soptletterRootController` 해제 이후에 실행되게 연결하세요.
Comment on lines
207
to
+216
| confirmButtonTap | ||
| .withUnretained(self) | ||
| .sink { owner, _ in | ||
| owner.dismiss(animated: true) | ||
| if owner.isEditingContent { | ||
| let editedContent = owner.contentTextView.text ?? "" | ||
| owner.editCompleteButtonTapPublisher.send(editedContent) | ||
| } else { | ||
| owner.dismiss(animated: true) | ||
| } | ||
| }.store(in: cancelBag) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
빈 내용 수정에 대한 검증이 없습니다.
updateConfirmButtonState는 글자 수 초과만 비활성화하므로, 내용을 모두 지운 상태에서도 "수정완료"가 눌려 빈 문자열이 전송될 수 있습니다. 최소 길이(비어있지 않음) 검증 추가를 검토하세요.
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterDetailModalVC.swift`
around lines 207 - 216, The confirm action in SoptletterDetailModalVC’s
confirmButtonTap sink allows an empty editedContent to be sent because
updateConfirmButtonState only checks the maximum length. Add a non-empty
validation before calling editCompleteButtonTapPublisher.send in the editing
branch, and make sure the confirm button state in updateConfirmButtonState
reflects both the max-length and minimum-length requirements so “수정완료” cannot be
triggered with blank content.
Comment on lines
+396
to
+409
| func cancelEditingMode() { | ||
| isEditingContent = false | ||
| contentTextView.resignFirstResponder() | ||
| contentTextView.isEditable = false | ||
| contentTextView.isHidden = true | ||
| contentLabel.isHidden = false | ||
| charCountLabel.isHidden = true | ||
| confirmButton.setTitle("확인", for: .normal) | ||
| editDeleteStackView.isHidden = false | ||
| confirmButton.isEnabled = true | ||
| confirmButton.backgroundColor = DSKitAsset.Colors.gray800.color | ||
| cancelEditButton.isHidden = true | ||
| likeStackView.isHidden = false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
cancelEditingMode()에서 dateLabel 표시 복원이 누락되었습니다.
enterEditingMode()는 dateLabel.isHidden = true로 날짜를 숨기지만, cancelEditingMode()에는 복원 코드가 없습니다. exitEditingMode()와 달리 편집을 취소하면 날짜 라벨이 계속 숨겨진 상태로 남습니다.
🔧 제안 수정
func cancelEditingMode() {
isEditingContent = false
contentTextView.resignFirstResponder()
contentTextView.isEditable = false
contentTextView.isHidden = true
contentLabel.isHidden = false
charCountLabel.isHidden = true
+ dateLabel.isHidden = false
confirmButton.setTitle("확인", for: .normal)
editDeleteStackView.isHidden = false
confirmButton.isEnabled = true
confirmButton.backgroundColor = DSKitAsset.Colors.gray800.color
cancelEditButton.isHidden = true
likeStackView.isHidden = false
}📝 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
| func cancelEditingMode() { | |
| isEditingContent = false | |
| contentTextView.resignFirstResponder() | |
| contentTextView.isEditable = false | |
| contentTextView.isHidden = true | |
| contentLabel.isHidden = false | |
| charCountLabel.isHidden = true | |
| confirmButton.setTitle("확인", for: .normal) | |
| editDeleteStackView.isHidden = false | |
| confirmButton.isEnabled = true | |
| confirmButton.backgroundColor = DSKitAsset.Colors.gray800.color | |
| cancelEditButton.isHidden = true | |
| likeStackView.isHidden = false | |
| } | |
| func cancelEditingMode() { | |
| isEditingContent = false | |
| contentTextView.resignFirstResponder() | |
| contentTextView.isEditable = false | |
| contentTextView.isHidden = true | |
| contentLabel.isHidden = false | |
| charCountLabel.isHidden = true | |
| dateLabel.isHidden = false | |
| confirmButton.setTitle("확인", for: .normal) | |
| editDeleteStackView.isHidden = false | |
| confirmButton.isEnabled = true | |
| confirmButton.backgroundColor = DSKitAsset.Colors.gray800.color | |
| cancelEditButton.isHidden = true | |
| likeStackView.isHidden = false | |
| } |
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/VC/SoptletterDetailModalVC.swift`
around lines 396 - 409, The cancelEditingMode() flow is missing restoration of
the dateLabel visibility, so it stays hidden after exiting edit mode. Update
SoptletterDetailModalVC.cancelEditingMode() to mirror exitEditingMode() by
explicitly un-hiding dateLabel along with the other non-editing UI elements,
using the existing dateLabel symbol in this view controller.
Comment on lines
+66
to
82
| input.editCompleteButtonTap | ||
| .withUnretained(self) | ||
| .sink { owner, content in | ||
| owner.submitTask?.cancel() | ||
| owner.submitTask = Task { [weak self] in | ||
| do { | ||
| let result = try await owner.useCase.fetchSoptletterMessage(messageId: owner.messageId, topicId: owner.topicId) | ||
| await MainActor.run { output.soptletterMessage.send(result) } | ||
| try await owner.useCase.editMessage(messageId: owner.messageId, topicId: owner.topicId, content: content) | ||
| output.soptletterEditCompleted.send() | ||
| owner.onEditCompleted?() | ||
| } catch is CancellationError { | ||
| return | ||
| self?.onError?() | ||
| } catch { | ||
| self?.onError?() | ||
| } | ||
| } | ||
| }.store(in: cancelBag) | ||
|
|
||
| // TODO: 수정 API | ||
| input.editButtonTap | ||
| .withUnretained(self) | ||
| .sink { owner, _ in | ||
|
|
||
| } | ||
| } | ||
| .store(in: cancelBag) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swiftRepository: sopt-makers/SOPT-iOS
Length of output: 4246
취소는 조용히 반환하세요
editCompleteButtonTap에서도 CancellationError는 return으로 처리하고, onError?()는 실제 실패에서만 호출하세요. Task 안에서는 owner를 직접 쓰고 있어 [weak self]는 불필요합니다.
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swift`
around lines 66 - 82, In SoptletterDetailViewModel’s editCompleteButtonTap
handling, treat CancellationError as a silent return instead of calling
onError?(), and only invoke onError?() for real failures. Update the Task body
to use owner consistently rather than [weak self], since self is not needed
there, and keep the existing submitTask cancellation flow intact.
Comment on lines
84
to
92
| input.deleteButtonTap | ||
| .withUnretained(self) | ||
| .sink { owner, _ in | ||
| AlertUtils.presentAlertVC(type: .titleDescription, title: "솝레터 삭제하기", description: "해당 솝레터가 영구적으로 삭제되어요.\n그래도 삭제하시겠어요?", customButtonTitle: "삭제") { | ||
| print("삭제") | ||
| } | ||
| .sink { owner, content in | ||
| AlertUtils.presentAlertVC(type: .titleDescription, title: "솝레터 삭제하기", description: "해당 솝레터가 영구적으로 삭제되어요.\n그래도 삭제하시겠어요?", customButtonTitle: "삭제", customAction: { | ||
| owner.deleteMessage(output: output, content: content) | ||
| owner.onDeleteCompleted?() | ||
| }) | ||
| } | ||
| .store(in: cancelBag) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
삭제 완료 콜백이 비동기 삭제 완료 이전에 호출됩니다.
deleteMessage(...)는 Task를 시작만 하고 곧바로 owner.onDeleteCompleted?()가 동기적으로 실행됩니다. 결과적으로 메인 목록 새로고침(refreshMessagesTrigger)이 서버 삭제 완료 전에 트리거되어, 삭제된 메시지가 남아있는 stale 데이터를 다시 불러올 수 있습니다. 편집 경로처럼 삭제 성공 이후에 호출되도록 deleteMessage 내부(위 제안 diff의 output.soptletterDeleteCompleted.send() 직후)로 이동하세요.
🔧 제안 수정
input.deleteButtonTap
.withUnretained(self)
.sink { owner, content in
AlertUtils.presentAlertVC(type: .titleDescription, title: "솝레터 삭제하기", description: "해당 솝레터가 영구적으로 삭제되어요.\n그래도 삭제하시겠어요?", customButtonTitle: "삭제", customAction: {
- owner.deleteMessage(output: output, content: content)
- owner.onDeleteCompleted?()
+ owner.deleteMessage(output: output)
})
}
.store(in: cancelBag)🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swift`
around lines 84 - 92, `SoptletterDetailViewModel`의 삭제 완료 콜백이 `deleteButtonTap`의
`customAction`에서 `deleteMessage(...)` 직후 동기적으로 호출되어, 비동기 삭제 완료 전에
`onDeleteCompleted`가 실행되는 문제가 있습니다. `deleteMessage` 내부에서 삭제 Task가 성공한 뒤,
`output.soptletterDeleteCompleted.send()`가 발생한 시점에 `onDeleteCompleted?()`를 호출하도록
옮기고, `deleteButtonTap` 쪽에서는 콜백 호출을 제거해 `deleteMessage`, `onDeleteCompleted`,
`soptletterDeleteCompleted` 흐름이 삭제 성공 이후에만 이어지도록 수정하세요.
Comment on lines
+98
to
+125
| extension SoptletterDetailViewModel { | ||
| private func deleteMessage(output: Output, content: String) { | ||
| submitTask?.cancel() | ||
| submitTask = Task { [weak self] in | ||
| do { | ||
| try await useCase.deleteMessage(messageId: messageId, topicId: topicId) | ||
| output.soptletterDeleteCompleted.send() | ||
| } catch is CancellationError { | ||
| self?.onError?() | ||
| } catch { | ||
| self?.onError?() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public func fetchMessages(output: Output) { | ||
| submitTask?.cancel() | ||
| submitTask = Task { [weak self] in | ||
| do { | ||
| let messages = try await useCase.fetchSoptletterMessage(messageId: messageId, topicId: topicId) | ||
| await MainActor.run { output.soptletterMessage.send(messages) } | ||
| } catch is CancellationError { | ||
| return | ||
| } catch { | ||
| self?.onError?() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 파일에서 [weak self] 캡처 + 명시 self 없는 멤버 접근 여부 확인
fd -e swift SoptletterDetailViewModel.swift --exec sed -n '95,126p' {}Repository: sopt-makers/SOPT-iOS
Length of output: 1160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swift"
sed -n '1,180p' "$FILE" | cat -nRepository: sopt-makers/SOPT-iOS
Length of output: 5128
[weak self] 안에서는 인스턴스 멤버를 암시적으로 참조할 수 없습니다.
deleteMessage와 fetchMessages의 Task { [weak self] in ... }에서 useCase, messageId, topicId를 self 없이 읽고 있어 컴파일이 막힙니다. guard let self else { return }로 풀고 self.useCase...처럼 명시적으로 접근하세요.
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterDetailViewModel.swift`
around lines 98 - 125, `deleteMessage` and `fetchMessages` are accessing
instance properties inside `Task { [weak self] in ... }` without resolving
`self`, which prevents compilation. Update `SoptletterDetailViewModel` to safely
unwrap `self` at the start of each task (for example with `guard let self else {
return }`) and then access `useCase`, `messageId`, and `topicId` through `self`
explicitly throughout the task body.
| private let useCase: SoptletterUseCase | ||
| private let coordinator: AnyCoordinatorObject | ||
| private var submitTask: Task<Void, Never>? | ||
| private let topicId: Int |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# topicId가 실제로 전달되는지 확인
rg -n -C5 'makeSoptletterMainVC' --type=swift
rg -n -C5 'onCellTap' SOPT-iOS/Projects/Features/SoptletterFeature/Sources/Coordinator/SoptletterCoordinator.swiftRepository: sopt-makers/SOPT-iOS
Length of output: 158
선택한 topicId를 메인 화면까지 전달하세요
showSelectTopic()에서 선택된 토픽 ID를 showSoptletterMain()으로 넘기지 않아 SoptletterMainViewModel이 기본값 1로만 메시지를 조회합니다. makeSoptletterMainVC/showSoptletterMain에 topicId를 추가해 선택값이 끝까지 전달되도록 수정해야 합니다.
🤖 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
`@SOPT-iOS/Projects/Features/SoptletterFeature/Sources/SoptletterMainScene/ViewModel/SoptletterMainViewModel.swift`
at line 39, The selected topicId is not being passed through to the main screen,
so SoptletterMainViewModel falls back to its default value. Update the flow from
showSelectTopic() through showSoptletterMain() and makeSoptletterMainVC() to
accept and forward topicId, and wire it into SoptletterMainViewModel so the
chosen topic is used instead of the default.
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
1 participant
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.
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
📌 참고 사항
📸 스크린샷
📮 관련 이슈