[Feat] QR 스캔 미지원 브랜드, 신속한 파싱 전략 수립을 위한 디스코드 웹훅 설정 #188#189
Merged
Remaked-Swain merged 6 commits intodevelopfrom Mar 24, 2026
Merged
[Feat] QR 스캔 미지원 브랜드, 신속한 파싱 전략 수립을 위한 디스코드 웹훅 설정 #188#189Remaked-Swain merged 6 commits intodevelopfrom
Remaked-Swain merged 6 commits intodevelopfrom
Conversation
Remaked-Swain
added
Add ✚
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughQR 스캔 흐름에 사용자 컨텍스트를 전파하도록 상태와 시그니처를 변경하고, 미지원 브랜드 검출 시 Discord 웹훅으로 비동기 알림을 전송하는 로직을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant QRFeature as QRCodeScanFeature
participant Client as QRScannerClient
participant Repo as DefaultQRCodeScanRepository
participant Strategy as QRCodeParsingStrategy
participant Network as NetworkProvider
participant Webhook as DiscordWebhook
User->>QRFeature: QR 스캔(데이터)
QRFeature->>Client: parse(urlString, user)
Client->>Repo: parse(url, user)
Repo->>Strategy: 파싱 시도
Strategy-->>Repo: 매칭 실패
Repo->>Network: detached background task로<br/>notifyUnsupportedBrand(url,user) 호출
Network->>Webhook: POST (embed: URL, host, user)
Webhook-->>Network: 응답
Network-->>Repo: 완료/오류(로깅)
Repo-->>Client: throw .unsupportedBrand
Client-->>QRFeature: throw QRParseError
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🧹 Nitpick comments (2)
Neki-iOS/Features/QRCodeScanner/Sources/Domain/Sources/Interfaces/QRCodeScanRepository.swift (1)
23-23:parse계약을User에 묶지 않는 편이 낫습니다.지원 브랜드 파싱 자체는
URL만 필요해서, unsupported-brand 리포팅 때문에User를 필수 인자로 만들면 파싱 레이어가 인증 모델에 결합됩니다. 리포터 의존성이나 최소 진단용 컨텍스트로 분리해 두면 테스트와 재사용성이 좋아집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/QRCodeScanner/Sources/Domain/Sources/Interfaces/QRCodeScanRepository.swift` at line 23, The parse signature on QRCodeScanRepository currently couples parsing to authentication by requiring a User; change the contract so func parse(_ url: URL) async throws(QRParseError) -> ParsedQRResult (remove the User parameter from parse in QRCodeScanRepository and all implementations), and move any user-dependent reporting to a separate method or injected reporter (e.g., add a reportUnsupportedBrand(_ result: ParsedQRResult, by user: User) or inject an UnsupportedBrandReporter into the higher-level use case). Update all callers of QRCodeScanRepository.parse, repository implementations, and unit tests to call the new parse(url) and perform user-scoped actions via the new reporter or reporting method.Neki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DefaultQRCodeScanRepository.swift (1)
31-35:NetworkProvider에Sendable명시 추가 검토 (선택사항)현재는
DefaultNetworkProvider가actor로 선언되어 자동으로Sendable을 준수하므로 문제가 없습니다. 다만 프로토콜NetworkProvider자체에Sendable준수를 명시하면, 향후 strict concurrency를 완전히 활성화할 때 타입 안정성을 더욱 명확히 할 수 있습니다:public protocol NetworkProvider: Sendable { func requestVoid(endpoint: Endpoint) async throws -> Void func request(endpoint: Endpoint) async throws -> BaseResponseDTO<EmptyData> func request<T: Decodable>(endpoint: Endpoint) async throws -> BaseResponseDTO<T> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DefaultQRCodeScanRepository.swift` around lines 31 - 35, Add explicit Sendable conformance to the NetworkProvider protocol so the compiler enforces concurrency safety (this clarifies usage when DefaultNetworkProvider is an actor); update the protocol declaration for NetworkProvider to conform to Sendable and ensure its async methods (requestVoid(endpoint:), request(endpoint:), and request<T: Decodable>(endpoint:)) remain compatible with Sendable so Task.detached usage with DefaultNetworkProvider is type-safe under stricter concurrency checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Neki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DTOs/DiscordWebhookDTO.swift`:
- Around line 34-36: Remove direct personal identifiers and full QR URLs from
the Discord payload in DiscordWebhookDTO: stop sending user.nickname and user.id
and the unsupportedURL.absoluteString; instead send only non-identifying minimal
diagnostics (e.g., host/domain and a high-level event type) or a
truncated/hashed identifier if you must correlate internally, and strip any
query string or token from URLs (collect full URL only on a protected backend
endpoint). Update the Field entries (the lines constructing "👤 User", "🔗 Full
URL", etc.) accordingly so no raw PII or full URLs are emitted.
In `@Neki-iOS/Info.plist`:
- Around line 5-6: The QR_WEBHOOK_URL key in Info.plist (QR_WEBHOOK_URL) exposes
the Discord webhook in the final app bundle; remove this key/value and stop
calling the webhook directly from the client (e.g., any code referencing
QR_WEBHOOK_URL), and instead implement a server-side endpoint that the app calls
(e.g., POST /api/send-qr-webhook) which performs the actual Discord webhook
request; update the client to call that secure server API and ensure the server
stores the webhook secret in its environment/config rather than embedding it in
the app bundle.
---
Nitpick comments:
In
`@Neki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DefaultQRCodeScanRepository.swift`:
- Around line 31-35: Add explicit Sendable conformance to the NetworkProvider
protocol so the compiler enforces concurrency safety (this clarifies usage when
DefaultNetworkProvider is an actor); update the protocol declaration for
NetworkProvider to conform to Sendable and ensure its async methods
(requestVoid(endpoint:), request(endpoint:), and request<T:
Decodable>(endpoint:)) remain compatible with Sendable so Task.detached usage
with DefaultNetworkProvider is type-safe under stricter concurrency checks.
In
`@Neki-iOS/Features/QRCodeScanner/Sources/Domain/Sources/Interfaces/QRCodeScanRepository.swift`:
- Line 23: The parse signature on QRCodeScanRepository currently couples parsing
to authentication by requiring a User; change the contract so func parse(_ url:
URL) async throws(QRParseError) -> ParsedQRResult (remove the User parameter
from parse in QRCodeScanRepository and all implementations), and move any
user-dependent reporting to a separate method or injected reporter (e.g., add a
reportUnsupportedBrand(_ result: ParsedQRResult, by user: User) or inject an
UnsupportedBrandReporter into the higher-level use case). Update all callers of
QRCodeScanRepository.parse, repository implementations, and unit tests to call
the new parse(url) and perform user-scoped actions via the new reporter or
reporting method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e812a49f-0331-4491-b0f1-de1d006a6729
📒 Files selected for processing (8)
Neki-iOS/APP/Sources/MainTab/MainTabCoordinator.swiftNeki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DTOs/DiscordWebhookDTO.swiftNeki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DefaultQRCodeScanRepository.swiftNeki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/QRCodeScannerEndpoint.swiftNeki-iOS/Features/QRCodeScanner/Sources/Domain/Sources/Clients/QRCodeScannerClient.swiftNeki-iOS/Features/QRCodeScanner/Sources/Domain/Sources/Interfaces/QRCodeScanRepository.swiftNeki-iOS/Features/QRCodeScanner/Sources/Presentation/Sources/Features/QRCodeScanFeature.swiftNeki-iOS/Info.plist
Comment on lines
+34
to
+36
| Field(name: "👤 User", value: "\(user.nickname) (ID: \(user.id))", inline: true), | ||
| Field(name: "🌐 Host", value: host, inline: true), | ||
| Field(name: "🔗 Full URL", value: unsupportedURL.absoluteString, inline: false) |
There was a problem hiding this comment.
개인 식별자와 QR 원문 URL을 디스코드로 직접 보내면 개인정보 유출입니다.
nickname, 내부 id, 전체 QR URL 조합이면 사용자를 재식별할 수 있고, query/token이 붙은 URL이면 접근 정보까지 외부 시스템에 남을 수 있습니다. 최소 진단 정보만 보내고, 원문 URL이 꼭 필요하면 접근 제어되는 백엔드에서 수집하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/QRCodeScanner/Sources/Data/Sources/Repositories/DTOs/DiscordWebhookDTO.swift`
around lines 34 - 36, Remove direct personal identifiers and full QR URLs from
the Discord payload in DiscordWebhookDTO: stop sending user.nickname and user.id
and the unsupportedURL.absoluteString; instead send only non-identifying minimal
diagnostics (e.g., host/domain and a high-level event type) or a
truncated/hashed identifier if you must correlate internally, and strip any
query string or token from URLs (collect full URL only on a protected backend
endpoint). Update the Field entries (the lines constructing "👤 User", "🔗 Full
URL", etc.) accordingly so no raw PII or full URLs are emitted.
Member
Author
There was a problem hiding this comment.
토큰 등을 사용하지 않는 URL이어서 원문 URL을 써도 보안상 문제될 여지는 없으므로 기각.
There was a problem hiding this comment.
@Remaked-Swain, 이해했습니다. 해당 QR URL에 인증 토큰이나 민감 파라미터가 포함되지 않는다면 원문 URL 전송은 문제 없습니다. 다만 user.nickname + user.id 조합은 서비스 내 사용자 재식별이 가능한 정보이므로, 향후 백엔드 이관 시 GDPR/개인정보보호법 관점에서 수집 정책을 함께 검토하시면 좋겠습니다. 결정 존중합니다.
Comment on lines
+5
to
+6
| <key>QR_WEBHOOK_URL</key> | ||
| <string>$(QR_WEBHOOK_URL)</string> |
There was a problem hiding this comment.
XCConfig를 거쳐도 웹훅 URL은 최종 앱 번들에 평문으로 포함됩니다.
디스코드 웹훅 URL 자체가 인증 비밀값이라 IPA/앱 번들에서 추출되면 아무나 채널에 임의 메시지를 보낼 수 있습니다. 클라이언트는 자체 API만 호출하고, 실제 웹훅 호출은 서버에서 중계해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Neki-iOS/Info.plist` around lines 5 - 6, The QR_WEBHOOK_URL key in Info.plist
(QR_WEBHOOK_URL) exposes the Discord webhook in the final app bundle; remove
this key/value and stop calling the webhook directly from the client (e.g., any
code referencing QR_WEBHOOK_URL), and instead implement a server-side endpoint
that the app calls (e.g., POST /api/send-qr-webhook) which performs the actual
Discord webhook request; update the client to call that secure server API and
ensure the server stores the webhook secret in its environment/config rather
than embedding it in the app bundle.
Member
Author
There was a problem hiding this comment.
이런 하꼬앱은 괜찮음. 나중에 고려해볼것.
There was a problem hiding this comment.
@Remaked-Swain, 알겠습니다! 나중에 백엔드로 이전하실 때 함께 처리하시면 좋을 것 같습니다. 혹시 나중에 놓치지 않도록 GitHub 이슈로 등록해 드릴까요?
( T_T)\ ( ヽ♡
OneTen19
approved these changes
Mar 24, 2026
|
|
||
| import Foundation | ||
|
|
||
| public enum DiscordWebhookDTO { |
Member
There was a problem hiding this comment.
p3
이 dto는 어디 나와있죠??? 맘대로 정할 수 있는건감??
| struct Field: Encodable { | ||
| let name: String | ||
| let value: String | ||
| let inline: Bool |
Member
Author
There was a problem hiding this comment.
DM에 보여지는 형태를 의미합니다.
Comment on lines
26
to
+38
| for strategy in strategies { | ||
| guard strategy.canHandle(host: host) else { continue } | ||
| return try await strategy.parse(url, networkProvider: networkProvider) | ||
| } | ||
|
|
||
| Task.detached(priority: .background) { | ||
| do { | ||
| let endpoint = QRCodeScannerEndpoint.notifyUnsupportedBrand(url: url, user: user) | ||
| try await networkProvider.requestVoid(endpoint: endpoint) | ||
| } catch { | ||
| Logger.network.error("미지원 브랜드 디스코드 웹훅 발송 실패: \(error)") | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
p3
전략에서 적합한 부분이 없으면 return이 실행이 안 돼서 throw로 에러보내던 로직 사이에
Task.detached 문이 추가된거군. 중요도 높은 기능이 아니라 .background로 한 거일테고!!
그럼 unsupportedBrand 에러 throw도 하면서 백그라운드 스레드에서는 디코 웹훅을 보내는 구조겠군요!
Member
Author
There was a problem hiding this comment.
맞습니다. 미지원 브랜드 얼럿 표시를 위해 UI 블로킹 하면 안되면서 디스코드 웹훅을 보내기 위해 별도의 비동기 작업을 진행합니다.
| struct Embed: Encodable { | ||
| let title: String | ||
| let description: String | ||
| let color: Int |
Member
There was a problem hiding this comment.
p3
color를 int로 받는 이유는 클아 때문인가요 아니면 디코 정책 때문인가요 둘 다인가요
Member
Author
There was a problem hiding this comment.
그냥 눈에 잘띄도록 디엠 색깔을 넣을 수 있는 옵션이 있어서 썼어요. 큰 의도는 없습니다. Constructor Injection 없이 기본값으로 빨간색 줘도 돼요.
| color: 16711680, // 빨강색 (Hex: FF0000) | ||
| fields: [ | ||
| Field(name: "🍎 Platform", value: "iOS", inline: true), | ||
| Field(name: "👤 User", value: "\(user.nickname) (ID: \(user.id))", inline: true), |
Member
There was a problem hiding this comment.
p3
유저가 왜 필요한가 했더니 유저 이름이랑 아이디 받고 있구나
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
2 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.
🌴 작업한 브랜치
✅ 작업한 내용
미지원 브랜드에 대한 QR 스캔 시도 시, 파싱 전략 수립을 위해 디스코드 웹훅을 이용해 알림을 띄우고 QR URL을 확보할 수 있도록 했습니다.
그리고 자잘하게 UI 컴포넌트 디자인적 변경점이 있는 것 같아서 이참에 하려고 했는데 막상 살펴보니 바꿀게 딱히 없어서 태스크에서 제외했습니다.
❗️PR Point
간단한 구현이므로 생략합니다!
디스코드 스레드에서 해당 기능을 프론트에서 할지, 백엔드에서 할지 의논하다가 프론트에서 하는 걸로 잠정 결론나서 구현했는데, 나중에 백엔드에서 하게될 수도 있다는 점만 인지해두시면 될 듯합니다.
📸 스크린샷
디스코드 웹훅 테스트로 대체합니다!
📟 관련 이슈
Summary by CodeRabbit