StorageScrapFragment Compose 마이그레이션 및 ViewModel 테스트 추가#392
Conversation
XML/RecyclerView+DataBinding 기반 StorageScrapFragment를 Jetpack Compose로 포팅. 동작은 그대로 유지하고 구현 방식만 교체. StorageViewModel unit test 10개(MockK+Turbine+coroutines-test)도 함께 추가해 회귀 안전망을 마련.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
기존엔 assembleDebug만 돌고 테스트 검증 없이 빌드 체크만 했음. testDebugUnitTest 스텝을 추가해 모든 PR에서 테스트 결과가 GitHub Actions 체크로 객관적으로 남도록 하고, 트리거도 develop 베이스가 아닌 PR에도 적용되도록 확대.
gradle/gradle-daemon-jvm.properties는 JetBrains JDK 21을 요구하는데 CI는 Temurin 17로 고정돼 있어 불일치. 이 차이가 원인으로 보이는 kapt+DataBinding NullPointerException(ProcessDataBinding.getSupportedOptions, processingEnv null)이 CI에서만 재현됐음. JDK 버전을 21로 맞춰 검증.
| ): View { | ||
| return ComposeView(requireContext()).apply { | ||
| setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) | ||
| setContent { |
There was a problem hiding this comment.
ComposeView + setContent — Fragment 안에 Compose 트리 심기
onCreateView에서 XML을 inflate하는 대신 ComposeView를 만들고, 그 안에 setContent { }로 Composable 트리를 올립니다. Fragment 자체는 그대로 두고(Hilt, 생명주기 등은 기존 View 시스템 그대로), 화면 내용만 Compose로 바꾸는 점진 마이그레이션의 핵심 진입점입니다.
return ComposeView(requireContext()).apply {
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
setContent {
RunnectTheme { /* Composable 트리 시작 */ }
}
}핵심 개념:
- XML 대응:
inflater.inflate(R.layout.xxx, container, false)자리를ComposeView가 대신함 setViewCompositionStrategy는 Compose 트리가 언제 폐기(dispose)될지 결정 — Fragment의 View 생명주기와 맞춰주지 않으면 메모리 누수/중복 구독 위험- 이 한 줄이 있어야 View 기반 화면 안에 Compose 화면을 부분적으로 섞어 쓸 수 있음 (interop)
| setContent { | ||
| RunnectTheme { | ||
| val getState by viewModel.myScrapCourseGetState.observeAsState() | ||
| val scrapState by viewModel.courseScrapState.observeAsState() |
There was a problem hiding this comment.
observeAsState() — LiveData를 Compose의 State로 변환
StorageViewModel은 LiveData를 그대로 쓰고 있어서(이번 PR은 ViewModel 로직을 안 건드림), Compose 쪽에서 구독하려면 LiveData를 Compose가 이해하는 State<T>로 바꿔주는 다리가 필요합니다. observeAsState()가 그 역할입니다.
val getState by viewModel.myScrapCourseGetState.observeAsState()핵심 개념:
- XML/Fragment 대응:
liveData.observe(viewLifecycleOwner) { value -> ... }콜백 등록과 동일한 효과 by위임을 쓰면getState가State<T>가 아니라 그 안의 값(T) 자체처럼 동작 — 매번.value를 안 써도 됨- LiveData 값이 바뀌면 이 값을 읽고 있는 Composable이 자동으로 recomposition(재구성)됨 — 옵저버 콜백 안에서 직접 View를 갱신하던 방식과 다르게, "값이 바뀌면 화면이 따라간다"는 선언적 흐름
androidx.compose.runtime:runtime-livedata의존성이 있어야 함(이번 PR에서 신규 추가)
| val scrapState by viewModel.courseScrapState.observeAsState() | ||
|
|
||
| var courses by remember { mutableStateOf(emptyList<MyScrapCourse>()) } | ||
| var errorMessage by remember { mutableStateOf<String?>(null) } |
There was a problem hiding this comment.
remember { mutableStateOf(...) } — Composable이 자기 상태를 기억하는 법
Composable 함수는 recomposition(재구성)될 때마다 처음부터 다시 실행됩니다. 그냥 var courses = emptyList<MyScrapCourse>()라고 쓰면 재구성마다 빈 리스트로 초기화돼버려서, remember로 감싸 "다음 재구성에도 이 값을 유지하라"고 알려줘야 합니다.
var courses by remember { mutableStateOf(emptyList<MyScrapCourse>()) }핵심 개념:
- XML/View 대응: 없음에 가까움 — View는 객체이고 한 번 만들어지면 필드가 유지되지만, Composable은 "함수"라서 재호출 시 지역변수가 초기화됨.
remember가 그 차이를 메워줌 mutableStateOf로 만든 값이 바뀌면 그 값을 읽는 Composable만 자동으로 재구성됨 (전체 화면을 다시 그리지 않음)by위임 문법(getValue/setValueimport 필요)을 쓰면State<T>를 매번.value로 안 풀고 일반 변수처럼 읽고 쓸 수 있음- 이 화면에서는 ViewModel의 LiveData 값을 그대로 화면에 못 박지 않고, "스크랩 해제로 로컬에서 한 번 더 필터링한 리스트"를 따로 들고 있어야 해서 이 로컬 상태가 필요했음
| var courses by remember { mutableStateOf(emptyList<MyScrapCourse>()) } | ||
| var errorMessage by remember { mutableStateOf<String?>(null) } | ||
|
|
||
| LaunchedEffect(getState) { |
There was a problem hiding this comment.
LaunchedEffect — Composable 안에서 안전하게 "한 번만" 실행하기
Composable 함수 본문에 직접 Analytics.logEvent(...)나 상태 변경 코드를 적으면, 재구성될 때마다(심지어 화면이 안 바뀌어도) 계속 실행될 위험이 있습니다. LaunchedEffect(key)는 key 값이 처음 나타나거나 바뀔 때만 코루틴을 실행하고, 그 외엔 건너뜁니다.
LaunchedEffect(getState) {
when (val current = getState) {
is UiStateV2.Success -> { courses = current.data; Analytics.logEvent(...) }
is UiStateV2.Failure -> errorMessage = current.msg
else -> Unit
}
}핵심 개념:
- XML/Fragment 대응: LiveData observer 콜백 내부 로직과 비슷하지만, "콜백이 호출되는 시점"이 아니라 "key가 바뀌는 시점"에 반응한다는 차이가 있음
- key(
getState)가 동일한 값으로 다시 들어오면 재실행되지 않음 — 그래서Analytics.logEvent가 화면 재구성마다 중복 호출되는 걸 막아줌 - 코루틴이라서
delay,suspend fun호출 등 비동기 작업도 안전하게 가능 - 이 화면엔 같은 패턴이
scrapState에도 한 번 더 쓰였음(하트탭 결과 처리) — 원리는 동일
| onScrapItemClick: (MyScrapCourse) -> Unit, | ||
| onHeartClick: (MyScrapCourse) -> Unit, | ||
| onGoToScrapClick: () -> Unit, | ||
| ) { |
There was a problem hiding this comment.
@composable + State Hoisting — "상태 없는" 화면 함수 패턴
StorageScrapScreen은 자기 상태(ViewModel, LiveData)를 직접 들고 있지 않습니다. 대신 현재 상태(state)를 파라미터로 받고, 사용자 액션은 콜백 람다(onRefresh, onHeartClick 등)로 위로 흘려보냅니다. 이렇게 "상태는 위에서 내려주고, 이벤트는 위로 올려보내는" 패턴을 State Hoisting이라 부릅니다.
@Composable
fun StorageScrapScreen(
state: StorageScrapUiState,
onRefresh: () -> Unit,
onScrapItemClick: (MyScrapCourse) -> Unit,
onHeartClick: (MyScrapCourse) -> Unit,
onGoToScrapClick: () -> Unit,
)핵심 개념:
- XML/Fragment 대응: Fragment가 ViewModel을 직접 들고 옵저빙하던 책임을 여기선 호출하는 쪽(
StorageScrapFragment)이 가짐 — 이 함수는 순수하게 "상태 → UI"만 담당 - 이렇게 분리하면 이 Composable은 ViewModel/LiveData 없이도 미리보기(
@Preview)나 테스트가 가능해짐 @Composable어노테이션은 "이 함수는 Compose 컴파일러가 특별 취급해서, 내부에서 다른 Composable을 호출하고 recomposition 추적 대상이 된다"는 표시- 파라미터로 상태를 받기 때문에 같은 화면을 여러 군데서 재사용하거나, 가짜 상태로 테스트하기 쉬움
| } | ||
|
|
||
| Scaffold( | ||
| snackbarHost = { SnackbarHost(snackbarHostState) } |
There was a problem hiding this comment.
Scaffold — Material3의 "화면 뼈대" 컴포넌트
Scaffold는 Snackbar, TopBar, BottomBar, FAB 같은 화면의 표준 영역들을 위한 슬롯을 미리 정의해둔 레이아웃입니다. 여기서는 스낵바를 띄울 자리(snackbarHost)만 지정하고, 본문 콘텐츠는 람다의 paddingValues를 받아서 그 아래에 그립니다.
Scaffold(
snackbarHost = { SnackbarHost(snackbarHostState) }
) { paddingValues ->
// 본문 콘텐츠, paddingValues로 시스템 영역(스낵바 등) 침범 방지
}핵심 개념:
- XML 대응: 정확히 대응되는 단일 위젯은 없지만,
CoordinatorLayout+ 별도 Snackbar 호출 코드를 합친 것과 비슷한 역할 - 본문 람다가 받는
paddingValues를 반드시 적용해야 함 — 안 그러면 콘텐츠가 시스템 바/스낵바 영역에 가려질 수 있음(이 코드에서는PullToRefreshBox의modifier.padding(paddingValues)로 적용) snackbarHostState.showSnackbar(...)를 호출하는 코드(LaunchedEffect)와 짝을 이뤄서 동작함
| onRefresh = onRefresh, | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .padding(paddingValues) |
There was a problem hiding this comment.
PullToRefreshBox — SwipeRefreshLayout의 Compose 버전
당겨서 새로고침(pull-to-refresh) UI를 감싸주는 Material3 컴포넌트입니다. isRefreshing이 true면 로딩 인디케이터를 보여주고, 사용자가 당기면 onRefresh 콜백을 호출합니다.
PullToRefreshBox(
isRefreshing = state.isLoading,
onRefresh = onRefresh,
modifier = Modifier.fillMaxSize().padding(paddingValues)
) {
// 그리드/빈 화면 콘텐츠
}핵심 개념:
- XML 대응:
androidx.swiperefreshlayout.widget.SwipeRefreshLayout—setOnRefreshListener { }+isRefreshing = true/false직접 토글하던 것과 동일한 역할 isRefreshing은 ViewModel의 로딩 상태(State)를 그대로 전달하기만 하면 됨 — View 때처럼 수동으로true/false를 따로 맞춰줄 필요가 줄어듦- 안에 들어가는 content 람다가 실제 스크롤 가능한 콘텐츠(그리드 등)
| onClick = { onScrapItemClick(course) }, | ||
| onHeartClick = { onHeartClick(course) } | ||
| ) | ||
| } |
There was a problem hiding this comment.
LazyVerticalGrid + items(key=) — RecyclerView/GridLayoutManager의 Compose 버전
RecyclerView + GridLayoutManager + Adapter를 합친 역할을 하는 컴포넌트입니다. "Lazy"라는 이름처럼 화면에 보이는 항목만 그리고(RecyclerView의 뷰 재활용과 비슷한 효과), 스크롤하면 필요한 만큼만 추가로 그립니다.
LazyVerticalGrid(
columns = GridCells.Fixed(2),
horizontalArrangement = Arrangement.spacedBy(6.dp),
verticalArrangement = Arrangement.spacedBy(20.dp)
) {
items(state.courses, key = { it.id }) { course ->
ScrapCourseItem(course = course, ...)
}
}핵심 개념:
- XML 대응:
RecyclerView(adapter 클래스 작성 불필요) +GridLayoutManager(context, 2)+GridSpacingItemDecoration을 이 컴포넌트 하나가 대체함 key = { it.id }가 예전의DiffUtil.ItemCallback이 하던 일과 비슷함 — 리스트가 바뀔 때 Compose가 "어떤 항목이 추가/삭제/이동됐는지"를 안정적으로 추적해서, 불필요한 재구성을 줄여줌. key를 안 주면 위치(index)로만 식별해서 애니메이션/상태가 꼬일 수 있음items(list, key, itemContent)의 itemContent 람다가 곧onBindViewHolder역할
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
app/src/test/java/com/runnect/runnect/presentation/storage/StorageViewModelTest.kt (1)
179-195: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the
scrapTF = falsepath used by this screen.The migrated fragment only invokes
postCourseScrap(..., false)on heart taps, but the suite currently verifies only thetruerequest mapping. A regression in the unsave branch would slip through here.🤖 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 `@app/src/test/java/com/runnect/runnect/presentation/storage/StorageViewModelTest.kt` around lines 179 - 195, Add test coverage for the `scrapTF = false` branch in `StorageViewModelTest.postCourseScrap` so the screen’s unsave flow is verified too. Mirror the existing `postCourseScrap(id, scrapTF)` success test but call it with `false`, and stub `courseRepository.postCourseScrap(RequestPostCourseScrap(publicCourseId = 10, scrapTF = "false"))` to return a successful flow. Assert the `courseScrapState` emits `UiStateV2.Loading` followed by `UiStateV2.Success(postScrap)` for this branch as well.
🤖 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 @.github/workflows/CI.yml:
- Line 23: The workflow uses a floating tag for actions/setup-java, which should
be pinned to an immutable commit SHA. Update the setup-java step in the CI
workflow to use the exact approved SHA instead of actions/setup-java@v4. Keep
the change scoped to the actions/setup-java reference so the workflow remains
functionally the same while using a fixed revision.
In
`@app/src/main/java/com/runnect/runnect/presentation/storage/StorageScrapFragment.kt`:
- Around line 55-56: The snackbar error flow is stateful instead of one-shot, so
repeated identical failures do not retrigger
`LaunchedEffect(state.errorMessage)` in `StorageScrapScreen`. Update the error
handling around `errorMessage` in `StorageScrapFragment` so the message is
cleared immediately after it is consumed, or refactor the error path to use a
one-time event model. Make sure the state update logic in the relevant error
callbacks and the `LaunchedEffect` consumer both work with a transient error
signal rather than a persistent string.
- Around line 57-65: The storage scrap screen-view event is being logged twice
because `VIEW_STORAGE_SCRAP` is emitted both in `onViewCreated()` and again
inside the `LaunchedEffect(getState)` success branch in `StorageScrapFragment`.
Remove the duplicate analytics call from the state observer and keep the
screen-view logging in a single place, while leaving the course data update in
`UiStateV2.Success` intact.
In
`@app/src/main/java/com/runnect/runnect/presentation/storage/StorageScrapScreen.kt`:
- Around line 155-163: The scrap toggle in StorageScrapScreen is too small and
unlabeled, so make the clickable control meet the 48dp touch target and expose
an accessible spoken label. Update the Box/Image area driven by onHeartClick and
painterResource so the tappable container is larger while preserving the icon
size, and add a meaningful contentDescription or semantics label that TalkBack
can announce for the scrap action.
- Around line 83-85: The empty-state branch in StorageScrapScreen should not
rely only on state.courses.isEmpty(), because it shows EmptyScrapView before
getMyScrapCourses() finishes. Update the conditional in StorageScrapScreen so
EmptyScrapView is rendered only when loading has completed, using
state.isLoading (or an equivalent uninitialized/loaded state) together with the
existing courses check. Keep the change localized to the conditional around
EmptyScrapView and preserve the current onGoToScrapClick behavior.
In `@app/src/main/res/values/strings.xml`:
- Line 70: The empty-state copy in storage_scrap_empty_guide has an extra
leading space after the newline, causing the second line to render indented.
Update the string resource in strings.xml to remove the space after the line
break so the centered empty state displays both lines consistently.
---
Nitpick comments:
In
`@app/src/test/java/com/runnect/runnect/presentation/storage/StorageViewModelTest.kt`:
- Around line 179-195: Add test coverage for the `scrapTF = false` branch in
`StorageViewModelTest.postCourseScrap` so the screen’s unsave flow is verified
too. Mirror the existing `postCourseScrap(id, scrapTF)` success test but call it
with `false`, and stub
`courseRepository.postCourseScrap(RequestPostCourseScrap(publicCourseId = 10,
scrapTF = "false"))` to return a successful flow. Assert the `courseScrapState`
emits `UiStateV2.Loading` followed by `UiStateV2.Success(postScrap)` for this
branch as well.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddce0cb8-6f63-4220-8465-1d21c5b8b2d0
📒 Files selected for processing (12)
.github/workflows/CI.ymlapp/build.gradleapp/src/main/java/com/runnect/runnect/presentation/storage/StorageScrapFragment.ktapp/src/main/java/com/runnect/runnect/presentation/storage/StorageScrapScreen.ktapp/src/main/java/com/runnect/runnect/presentation/storage/adapter/StorageScrapAdapter.ktapp/src/main/java/com/runnect/runnect/util/callback/listener/OnHeartButtonClick.ktapp/src/main/java/com/runnect/runnect/util/callback/listener/OnScrapItemClick.ktapp/src/main/res/layout/fragment_storage_scrap.xmlapp/src/main/res/layout/item_storage_scrap.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/com/runnect/runnect/presentation/storage/StorageViewModelTest.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (5)
- app/src/main/java/com/runnect/runnect/util/callback/listener/OnScrapItemClick.kt
- app/src/main/java/com/runnect/runnect/presentation/storage/adapter/StorageScrapAdapter.kt
- app/src/main/res/layout/fragment_storage_scrap.xml
- app/src/main/res/layout/item_storage_scrap.xml
- app/src/main/java/com/runnect/runnect/util/callback/listener/OnHeartButtonClick.kt
|
|
||
| - name: set up JDK 17 | ||
| - name: set up JDK 21 | ||
| uses: actions/setup-java@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
git ls-files .github/workflows/CI.yml app/build.gradleRepository: Runnect/Runnect-Android
Length of output: 203
🏁 Script executed:
sed -n '1,140p' .github/workflows/CI.ymlRepository: Runnect/Runnect-Android
Length of output: 4364
🏁 Script executed:
sed -n '1,220p' app/build.gradleRepository: Runnect/Runnect-Android
Length of output: 6577
Pin actions/setup-java to a commit SHA
actions/setup-java@v4 is still a mutable tag. Pin it to the exact commit SHA your workflow allows instead of using a floating version.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/CI.yml at line 23, The workflow uses a floating tag for
actions/setup-java, which should be pinned to an immutable commit SHA. Update
the setup-java step in the CI workflow to use the exact approved SHA instead of
actions/setup-java@v4. Keep the change scoped to the actions/setup-java
reference so the workflow remains functionally the same while using a fixed
revision.
Source: Linters/SAST tools
There was a problem hiding this comment.
좋은 제안입니다. CI 워크플로 전체에 영향 주는 변경이라 이번 PR 범위 밖으로 보고 별도 이슈로 분리하겠습니다.
There was a problem hiding this comment.
@unam98 이해했습니다. 이번 PR 범위에서는 분리하는 판단이 합리적입니다.
원하시면 제가 후속 GitHub 이슈로 정리해 드릴게요. actions/setup-java를 고정 SHA로 pinning 하는 작업 범위와 수용 기준까지 포함해서 작성할 수 있습니다.
(•ᵕ•)🐇
There was a problem hiding this comment.
네, 부탁드립니다. 이슈로 열어주세요.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="Runnect/Runnect-Android"
requester="unam98"
pr_url="https://github.com/Runnect/Runnect-Android/pull/392"
comment_url="https://github.com/Runnect/Runnect-Android/pull/392#discussion_r3469158477"
title="Pin actions/setup-java to an immutable commit SHA in CI workflow"
body_file="$(mktemp)"
cat > "$body_file" <<EOF
## Summary
Update the CI workflow to pin \`actions/setup-java\` to an immutable commit SHA instead of using the floating tag \`actions/setup-java@v4\`.
## Why
The current workflow uses a mutable action reference in \`.github/workflows/CI.yml\`. Pinning the action to an exact commit SHA improves supply-chain security and aligns the workflow with stricter CI/SAST policies that require immutable action references.
## Affected area
- \`.github/workflows/CI.yml\`
## Required change
Replace the current \`uses: actions/setup-java@v4\` reference with the exact approved commit SHA for the same action revision, keeping the step behavior otherwise unchanged.
## Acceptance criteria
- \`.github/workflows/CI.yml\` no longer uses a floating tag for \`actions/setup-java\`
- The workflow references \`actions/setup-java\` using a full commit SHA
- The JDK configuration remains functionally the same (Java 21, Temurin, Gradle cache)
- CI continues to run successfully after the change
## References
- Follow-up from PR `#392`: $pr_url
- Review discussion: $comment_url
## Requested by
- @$requester
EOF
issue_url="$(
gh issue create \
--repo "$repo" \
--title "$title" \
--body-file "$body_file" \
--assignee "$requester"
)"
echo "$issue_url"Length of output: 1806
@unam98 후속 이슈 생성해두었습니다: #393
- 링크: Pin actions/setup-java to an immutable commit SHA in CI workflow #393
- 범위:
.github/workflows/CI.yml의actions/setup-java@v4를 immutable commit SHA로 pinning - 포함 사항: 변경 배경, 영향 파일, 수용 기준, PR
#392/ 해당 리뷰 코멘트 백링크, assignee 지정
필요하시면 이어서 이 PR의 해당 리뷰 코멘트도 제가 정리해서 마무리 방향 제안드릴게요.
| <string name="storage_my_draw_make_course">코스 그리기</string> | ||
| <string name="storage_scrap_make_scrap">스크랩하기</string> | ||
| <string name="storage_total_course_count">총 코스 %1$d개</string> | ||
| <string name="storage_scrap_empty_guide">아직 스크랩한 코스가 없어요\n 코스를 스크랩 해주세요</string> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the leading space in the empty-state copy.
storage_scrap_empty_guide contains \n , so the second line renders indented in the centered empty state.
Proposed fix
- <string name="storage_scrap_empty_guide">아직 스크랩한 코스가 없어요\n 코스를 스크랩 해주세요</string>
+ <string name="storage_scrap_empty_guide">아직 스크랩한 코스가 없어요\n코스를 스크랩 해주세요</string>📝 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.
| <string name="storage_scrap_empty_guide">아직 스크랩한 코스가 없어요\n 코스를 스크랩 해주세요</string> | |
| <string name="storage_scrap_empty_guide">아직 스크랩한 코스가 없어요\n코스를 스크랩 해주세요</string> |
🤖 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 `@app/src/main/res/values/strings.xml` at line 70, The empty-state copy in
storage_scrap_empty_guide has an extra leading space after the newline, causing
the second line to render indented. Update the string resource in strings.xml to
remove the space after the line break so the centered empty state displays both
lines consistently.
There was a problem hiding this comment.
확인했습니다 — 원본 하드코딩 문자열을 그대로 복사한 거라 기존부터 있던 오타입니다. 1:1 포팅 범위를 지키기 위해 이번 PR에서는 그대로 두고, 별도 이슈로 분리하겠습니다.
There was a problem hiding this comment.
@unam98 이해했습니다. 1:1 포팅 범위를 지키기 위해 이번 PR에서는 그대로 두는 방향 괜찮습니다.
원하시면 제가 후속 이슈로 분리할 수 있어요. app/src/main/res/values/strings.xml의 storage_scrap_empty_guide에서 줄바꿈 뒤 선행 공백을 제거하는 정리 작업으로 남겨두면 될 것 같습니다.
(•᷄ᴗ•᷅)
There was a problem hiding this comment.
네, 부탁드립니다. 이슈로 열어주세요.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="Runnect/Runnect-Android"
assignee="unam98"
pr_url="https://github.com/Runnect/Runnect-Android/pull/392"
comment_url="https://github.com/Runnect/Runnect-Android/pull/392#discussion_r3469158524"
tmp_body="$(mktemp)"
cat > "$tmp_body" <<'EOF'
## Summary
Clean up the `storage_scrap_empty_guide` string resource by removing the leading space after the newline.
Current value:
`아직 스크랩한 코스가 없어요\n 코스를 스크랩 해주세요`
Expected value:
`아직 스크랩한 코스가 없어요\n코스를 스크랩 해주세요`
## Why
During the `StorageScrapFragment` Compose migration in PR `#392`, this string was intentionally kept unchanged to preserve the scope of the 1:1 port. However, the existing leading space after `\n` causes the second line to render slightly indented in the centered empty state.
## Affected area
- `app/src/main/res/values/strings.xml`
- `storage_scrap_empty_guide`
## Acceptance criteria
- Remove the leading space after the newline in `storage_scrap_empty_guide`.
- Confirm the empty-state text renders centered consistently across both lines.
- Keep the change isolated from the Compose migration behavior.
## Backlinks
- PR: https://github.com/Runnect/Runnect-Android/pull/392
- Review comment: https://github.com/Runnect/Runnect-Android/pull/392#discussion_r3469158524
Requested by: `@unam98`
EOF
gh issue create \
--repo "$repo" \
--title "Follow up: remove leading space in storage_scrap_empty_guide string" \
--body-file "$tmp_body" \
--assignee "$assignee"Length of output: 1666
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="Runnect/Runnect-Android"
assignee="unam98"
pr_url="https://github.com/Runnect/Runnect-Android/pull/392"
comment_url="https://github.com/Runnect/Runnect-Android/pull/392#discussion_r3469158524"
tmp_body="$(mktemp)"
cat > "$tmp_body" <<'EOF'
## Summary
Clean up the `storage_scrap_empty_guide` string resource by removing the leading space after the newline.
Current value:
`아직 스크랩한 코스가 없어요\n 코스를 스크랩 해주세요`
Expected value:
`아직 스크랩한 코스가 없어요\n코스를 스크랩 해주세요`
## Why
During the `StorageScrapFragment` Compose migration in PR `#392`, this string was intentionally kept unchanged to preserve the scope of the 1:1 port. However, the existing leading space after `\n` causes the second line to render slightly indented in the centered empty state.
## Affected area
- `app/src/main/res/values/strings.xml`
- `storage_scrap_empty_guide`
## Acceptance criteria
- Remove the leading space after the newline in `storage_scrap_empty_guide`.
- Confirm the empty-state text renders centered consistently across both lines.
- Keep the change isolated from the Compose migration behavior.
## Backlinks
- PR: https://github.com/Runnect/Runnect-Android/pull/392
- Review comment: https://github.com/Runnect/Runnect-Android/pull/392#discussion_r3469158524
Requested by: `@unam98`
EOF
gh issue create \
--repo "$repo" \
--title "Follow up: remove leading space in storage_scrap_empty_guide string" \
--body-file "$tmp_body" \
--assignee "$assignee"Length of output: 1666
@unam98 후속 이슈 생성되었습니다.
#395— Follow up: remove leading space in storage_scrap_empty_guide string #395#396— Follow up: remove leading space in storage_scrap_empty_guide string #396
다만 실행이 중복되면서 동일한 내용의 이슈가 2개 열렸습니다. 원하시면 제가 하나를 기준 이슈로 두고, 나머지 중복 이슈는 정리하는 방향으로 도와드릴게요.
| .aspectRatio(162f / 114f) | ||
| .clip(RoundedCornerShape(5.dp)) | ||
| .background(G4) | ||
| ) |
There was a problem hiding this comment.
AsyncImage + Modifier 체이닝 — Glide의 Compose 대응, 그리고 "순서가 의미를 만든다"
AsyncImage(coil-compose)는 Glide.with(view).load(url).into(imageView)를 한 Composable로 합친 것입니다. 그리고 Modifier는 .fillMaxWidth().aspectRatio(...).clip(...).background(...)처럼 체이닝하는데, 이 순서가 실제로 결과를 바꿉니다.
AsyncImage(
model = course.image,
contentScale = ContentScale.Crop,
modifier = Modifier
.fillMaxWidth()
.aspectRatio(162f / 114f)
.clip(RoundedCornerShape(5.dp))
.background(G4)
)핵심 개념:
- XML 대응:
Glide.with(ivItem.context).load(url).centerCrop().into(ivItem)—contentScale = ContentScale.Crop이centerCrop()과 동일 - Modifier는 위에서 아래로 "각 단계마다 제약을 좁혀가는" 방식으로 적용됨 — 여기선
fillMaxWidth로 너비를 정하고 →aspectRatio로 그 너비 기준 높이를 계산하고 → 그 영역을clip으로 둥글게 자르고 → 빈 배경색을 깔아줌. 만약clip을aspectRatio보다 앞에 두면 잘리는 영역의 크기 기준이 달라져서 의도한 모양이 안 나올 수 있음 model에 URL을 넘기면 코일이 네트워크 로딩/캐싱/플레이스홀더까지 처리 — 별도의ImageView참조나 콜백이 필요 없음
| containerColor = M1, | ||
| contentColor = White | ||
| ), | ||
| shape = RoundedCornerShape(10.dp) |
There was a problem hiding this comment.
Button (Material3) — 스타일을 XML drawable이 아니라 파라미터로
XML에서는 android:background="@drawable/radius_10_m1_button"처럼 별도 drawable 리소스로 버튼 모양/색을 정의했는데, Compose에서는 colors/shape 파라미터로 직접 넘깁니다.
Button(
onClick = onGoToScrapClick,
colors = ButtonDefaults.buttonColors(
containerColor = M1,
contentColor = White
),
shape = RoundedCornerShape(10.dp)
) {
Text(text = stringResource(R.string.storage_scrap_make_scrap), ...)
}핵심 개념:
- XML 대응:
<Button android:background="@drawable/radius_10_m1_button" android:onClick=... />+ 별도 drawable XML 파일이 필요했던 것을,shape(모양)과colors(색)를 코드에서 바로 지정해서 끝냄 — drawable XML 파일 자체가 불필요해짐 Button은 내부에content: @Composable RowScope.() -> Unit람다를 받음 — 그 안에 원하는 아이콘+텍스트 등을 자유롭게 배치 가능 (XML 버튼은 텍스트 하나만 가능했던 것과 차이)ButtonDefaults.buttonColors(...)는 일부 색만 지정하면 나머지는 테마 기본값을 따라감
- 빈 화면이 초기 로딩 중에도 잠깐 보이던 문제: 원본 XML은 로딩 중 그리드/빈화면 둘 다 숨겼는데 Compose 버전은 courses 기본값이 빈 리스트라 로딩 중에도 EmptyScrapView가 떴음. isLoading 조건 추가로 수정. - 동일한 에러가 연속으로 발생하면 두 번째 스낵바가 안 뜨던 문제: LiveData는 같은 값이어도 항상 옵저버를 호출하지만 Compose State는 동일 키면 LaunchedEffect가 재실행되지 않아서 생긴 차이. 에러를 보여준 뒤 즉시 errorMessage를 null로 리셋해서 1회성 이벤트로 만듦.
검증 매트릭스의⚠️ 미검증 항목(카드탭 이동, 빈 화면 진입) 두 곳을 ComposeTestRule로 채움. 빈 화면 테스트는 오늘 고친 로딩 중 깜빡임 회귀를 그대로 락인.
connectedDebugAndroidTest는 기기/에뮬레이터가 필요해 기존 unit test 스텝으로 커버되지 않았음. reactivecircus/android-emulator-runner로 PR마다 자동 실행되도록 별도 잡(android-test)을 추가하고, 결과 XML을 아티팩트로 업로드. REMOTE_KEY_FORCE_UPDATE secret 누락 이슈가 풀려야 함께 정상 동작함.
원본은 courseScrapState Loading 상태에서 프로그레스바를 띄웠지만 Compose 포팅 시 PullToRefreshBox의 isRefreshing이 myScrapCourseGetState에만 연결돼 하트탭 네트워크 대기 중 아무 피드백 없이 멈춰있다가 아이템이 사라지는 회귀가 있었음. before/after 영상 재검토로 발견.
StorageScrapUiState.from()으로 getState/scrapState의 Loading 결합 판단을 Fragment 밖으로 빼서 에뮬레이터 없이 JVM 유닛 테스트로 고정. StorageScrapScreen에는 isLoading=true일 때 로딩 인디케이터가 실제로 보이는지 검증하는 Compose UI test 추가 (네트워크 타이밍에 의존하지 않음).
x86_64 시스템 이미지는 aarch64 호스트의 QEMU2에서 못 돌아서 에뮬레이터가 즉시 죽고 10분간 부팅 대기만 하다 타임아웃났음. CI 동작 자체가 아니라 잡 설정 문제였음.
macOS 러너는 HVF가 HV_UNSUPPORTED를 내면서 하드웨어 가속 자체가 막혀 에뮬레이터가 못 떴음. android-emulator-runner 공식 문서가 권장하는 ubuntu-latest + KVM 활성화 방식으로 변경.
com.example.runnect로 박혀있던 안드로이드 스튜디오 생성 샘플 테스트. connectedDebugAndroidTest가 CI에서 처음 실제로 도니까 드러난 기존 버그.
testLogging으로 유닛 테스트가 콘솔에 테스트명 단위 PASSED/FAILED를 찍게 하고, dorny/test-reporter로 유닛/Compose UI 테스트 결과를 각각 별도 체크(Unit Test Results / Compose UI Test Results)로 게시해서 어떤 테스트가 깨졌는지 PR에서 클릭 한 번으로 특정 가능하게 함.
dorny/test-reporter가 테스트 결과 요약은 만들었지만 체크런 생성 API 호출 직전에 조용히 멈췄음. actions:read 권한 누락이 원인.
v1은 Job Summary로만 동작해서 PR 체크 목록에 안 보였음. 공식 문서가 권장하는 v3로 올려서 실제 체크런으로 생성되게 함.
dorny/test-reporter(v1/v3 둘 다)는 job summary는 만들지만 체크런 생성은 조용히 안 됐음(에러 없이 스킵, 원인 특정 못함). 더 기본적이고 통제 가능한 방식으로 전환: JUnit XML을 직접 파싱(.github/scripts/post_test_report.py) 해서 테스트별 ✅/❌ 마크다운을 PR 코멘트로 직접 게시(마커 기반으로 같은 코멘트를 갱신, 스팸 방지).
connectedAndroidTest는 디바이스 1대당 XML 1개에 모든 클래스의 테스트를 합쳐서 쓰는데, testsuite 레벨 name은 첫 번째 클래스명만 가져서 나머지 클래스 테스트가 잘못 묶였음.
PR 본문 검증 매트릭스에서 특정 테스트의 최신 통과 여부로 바로 링크 걸 수 있게, classname+테스트명 해시로 만든 안정적인 anchor id를 각 줄에 부여.
- 실패한 테스트만 출력하도록 post_test_report.py 단순화 - 전부 통과하면 기존 실패 코멘트를 삭제, 코멘트 없으면 무시 - 통과/실패 여부는 CI 체크 뱃지로 확인 가능하므로 성공 코멘트 불필요
* feat: StorageScrapFragment Compose 마이그레이션 및 ViewModel 테스트 추가 XML/RecyclerView+DataBinding 기반 StorageScrapFragment를 Jetpack Compose로 포팅. 동작은 그대로 유지하고 구현 방식만 교체. StorageViewModel unit test 10개(MockK+Turbine+coroutines-test)도 함께 추가해 회귀 안전망을 마련. * ci: PR CI에 unit test 실행 단계 추가 및 트리거 확대 기존엔 assembleDebug만 돌고 테스트 검증 없이 빌드 체크만 했음. testDebugUnitTest 스텝을 추가해 모든 PR에서 테스트 결과가 GitHub Actions 체크로 객관적으로 남도록 하고, 트리거도 develop 베이스가 아닌 PR에도 적용되도록 확대. * fix: CI JDK 버전을 21로 맞춤 (gradle-daemon-jvm.properties와 일치) gradle/gradle-daemon-jvm.properties는 JetBrains JDK 21을 요구하는데 CI는 Temurin 17로 고정돼 있어 불일치. 이 차이가 원인으로 보이는 kapt+DataBinding NullPointerException(ProcessDataBinding.getSupportedOptions, processingEnv null)이 CI에서만 재현됐음. JDK 버전을 21로 맞춰 검증. * fix: CodeRabbit이 발견한 Compose 포팅 회귀 2건 수정 - 빈 화면이 초기 로딩 중에도 잠깐 보이던 문제: 원본 XML은 로딩 중 그리드/빈화면 둘 다 숨겼는데 Compose 버전은 courses 기본값이 빈 리스트라 로딩 중에도 EmptyScrapView가 떴음. isLoading 조건 추가로 수정. - 동일한 에러가 연속으로 발생하면 두 번째 스낵바가 안 뜨던 문제: LiveData는 같은 값이어도 항상 옵저버를 호출하지만 Compose State는 동일 키면 LaunchedEffect가 재실행되지 않아서 생긴 차이. 에러를 보여준 뒤 즉시 errorMessage를 null로 리셋해서 1회성 이벤트로 만듦. * test: StorageScrapScreen Compose UI test 3건 추가 검증 매트릭스의⚠️ 미검증 항목(카드탭 이동, 빈 화면 진입) 두 곳을 ComposeTestRule로 채움. 빈 화면 테스트는 오늘 고친 로딩 중 깜빡임 회귀를 그대로 락인. * ci: Compose UI test(androidTest)를 에뮬레이터로 자동 실행하는 잡 추가 connectedDebugAndroidTest는 기기/에뮬레이터가 필요해 기존 unit test 스텝으로 커버되지 않았음. reactivecircus/android-emulator-runner로 PR마다 자동 실행되도록 별도 잡(android-test)을 추가하고, 결과 XML을 아티팩트로 업로드. REMOTE_KEY_FORCE_UPDATE secret 누락 이슈가 풀려야 함께 정상 동작함. * fix: 스크랩 해제(하트탭) 시 누락된 로딩 피드백 복구 원본은 courseScrapState Loading 상태에서 프로그레스바를 띄웠지만 Compose 포팅 시 PullToRefreshBox의 isRefreshing이 myScrapCourseGetState에만 연결돼 하트탭 네트워크 대기 중 아무 피드백 없이 멈춰있다가 아이템이 사라지는 회귀가 있었음. before/after 영상 재검토로 발견. * refactor: 로딩 상태 결합 로직을 순수 함수로 분리 + 회귀 방지 테스트 추가 StorageScrapUiState.from()으로 getState/scrapState의 Loading 결합 판단을 Fragment 밖으로 빼서 에뮬레이터 없이 JVM 유닛 테스트로 고정. StorageScrapScreen에는 isLoading=true일 때 로딩 인디케이터가 실제로 보이는지 검증하는 Compose UI test 추가 (네트워크 타이밍에 의존하지 않음). * fix: CI 에뮬레이터 잡을 macos-latest(Apple Silicon)에 맞춰 arm64-v8a로 수정 x86_64 시스템 이미지는 aarch64 호스트의 QEMU2에서 못 돌아서 에뮬레이터가 즉시 죽고 10분간 부팅 대기만 하다 타임아웃났음. CI 동작 자체가 아니라 잡 설정 문제였음. * fix: 안드로이드 에뮬레이터 CI 잡을 ubuntu-latest+KVM으로 전환 macOS 러너는 HVF가 HV_UNSUPPORTED를 내면서 하드웨어 가속 자체가 막혀 에뮬레이터가 못 떴음. android-emulator-runner 공식 문서가 권장하는 ubuntu-latest + KVM 활성화 방식으로 변경. * fix: ExampleInstrumentedTest의 하드코딩된 패키지명 오타 수정 com.example.runnect로 박혀있던 안드로이드 스튜디오 생성 샘플 테스트. connectedDebugAndroidTest가 CI에서 처음 실제로 도니까 드러난 기존 버그. * ci: 테스트별 성공/실패를 PR 체크에서 바로 확인하도록 test-reporter 추가 testLogging으로 유닛 테스트가 콘솔에 테스트명 단위 PASSED/FAILED를 찍게 하고, dorny/test-reporter로 유닛/Compose UI 테스트 결과를 각각 별도 체크(Unit Test Results / Compose UI Test Results)로 게시해서 어떤 테스트가 깨졌는지 PR에서 클릭 한 번으로 특정 가능하게 함. * fix: test-reporter 체크런 생성에 필요한 actions:read 권한 추가 dorny/test-reporter가 테스트 결과 요약은 만들었지만 체크런 생성 API 호출 직전에 조용히 멈췄음. actions:read 권한 누락이 원인. * fix: dorny/test-reporter를 v3로 올려 체크런 대신 잡 서머리에만 찍히던 문제 해결 v1은 Job Summary로만 동작해서 PR 체크 목록에 안 보였음. 공식 문서가 권장하는 v3로 올려서 실제 체크런으로 생성되게 함. * ci: dorny/test-reporter 대신 자체 스크립트로 테스트 결과를 PR 코멘트에 게시 dorny/test-reporter(v1/v3 둘 다)는 job summary는 만들지만 체크런 생성은 조용히 안 됐음(에러 없이 스킵, 원인 특정 못함). 더 기본적이고 통제 가능한 방식으로 전환: JUnit XML을 직접 파싱(.github/scripts/post_test_report.py) 해서 테스트별 ✅/❌ 마크다운을 PR 코멘트로 직접 게시(마커 기반으로 같은 코멘트를 갱신, 스팸 방지). * fix: 테스트 결과 코멘트에서 testcase의 classname으로 그루핑하도록 수정 connectedAndroidTest는 디바이스 1대당 XML 1개에 모든 클래스의 테스트를 합쳐서 쓰는데, testsuite 레벨 name은 첫 번째 클래스명만 가져서 나머지 클래스 테스트가 잘못 묶였음. * feat: 테스트 결과 코멘트의 각 테스트에 고정 앵커 추가 PR 본문 검증 매트릭스에서 특정 테스트의 최신 통과 여부로 바로 링크 걸 수 있게, classname+테스트명 해시로 만든 안정적인 anchor id를 각 줄에 부여. * ci: 테스트 실패 시에만 PR 코멘트 게시, 통과 시 코멘트 삭제 - 실패한 테스트만 출력하도록 post_test_report.py 단순화 - 전부 통과하면 기존 실패 코멘트를 삭제, 코멘트 없으면 무시 - 통과/실패 여부는 CI 체크 뱃지로 확인 가능하므로 성공 코멘트 불필요 --------- Co-authored-by: unam98 <unam98@users.noreply.github.com>
작업 배경
변경 사항
핵심: 동작을 바꾸는 작업이 아니라 구현 방식만 View → Compose로 교체. UI 인터랙션(그리드 렌더링/하트탭 스크랩 해제/pull-to-refresh)은 기존과 동일하게 동작.
StorageViewModel• LiveData 동기 테스트용
androidx-core-testing(InstantTaskExecutorRule) 의존성 추가StorageScrapFragment• 리스너 인터페이스(
OnHeartButtonClick,OnScrapItemClick) → 람다 콜백으로 변경• 동일 에러 연속 발생 시 스낵바 안 뜨던 문제 수정(
onErrorShown으로 1회성 이벤트화)• 하트탭(스크랩 해제) 네트워크 대기 중 로딩 피드백이 빠졌던 문제 수정 —
getState/scrapState로딩 결합 판단을StorageScrapUiState.from()순수 함수로 분리StorageScrapScreen(신규)• 빈 화면, pull-to-refresh(
PullToRefreshBox) 포함• 초기 로딩 중 빈 화면이 먼저 보이던 문제 수정(
!isLoading && courses.isEmpty())• Compose UI test 4건 추가(
androidTest) — 카드탭 콜백, 빈 화면 표시 조건, 로딩 인디케이터 표시StorageScrapAdapter,OnHeartButtonClick,OnScrapItemClick,fragment_storage_scrap.xml,item_storage_scrap.xml제거storage_total_course_count,storage_scrap_empty_guide문자열 추가 (기존 하드코딩 대체).github/workflows/CI.yml에testDebugUnitTest/connectedDebugAndroidTest자동 실행 잡 추가• 테스트 실패 시 어떤 테스트가 실패했는지 PR 코멘트로 게시
영향 범위
StorageScrapFragment(보관함 > 스크랩 코스 탭)에 한정 — 구체적 동작별 검증은 아래 매트릭스 참고검증 매트릭스
getMyScrapCourses 성공 시 스크랩 목록과 itemSize가 갱신된다•
getMyScrapCourses 실패 시 Failure 상태로 갱신된다postCourseScrap 성공 시 Success 상태로 갱신된다•
스크랩_토글만_Loading이어도_isLoading은_true다•
isLoading이_true이면_로딩_인디케이터가_보인다getMyScrapCourses 성공 시 스크랩 목록과 itemSize가 갱신된다(재사용)카드탭_시_onScrapItemClick이_해당_코스로_호출된다(Compose UI test, 콜백 호출까지만 검증·실제 Activity 전환은 범위 밖)로딩_중에는_스크랩이_없어도_빈_화면이_보이지_않는다•
로딩이_끝나고_스크랩이_없으면_빈_화면이_보인다getMyDrawList 성공 시 코스 목록과 상태가 갱신된다•
getMyDrawList 실패 시 에러 메시지와 Failure 상태로 갱신된다•
deleteMyDrawCourse 성공 시 선택한 코스만 목록에서 제거된다•
deleteMyDrawCourse 실패 시 Failure 상태로 갱신된다•
modifyItemsToDelete는 같은 id를 다시 호출하면 선택을 해제한다•
clearItemsToDelete는 선택 목록을 비운다•
saveClickedCourseId는 clickedCourseId를 갱신한다Before / After 영상
같은 인터랙션(그리드 진입 → 하트탭 스크랩 해제 → pull-to-refresh)을 기존 버전과 Compose 버전에서 각각 녹화해 비교.
before2.mp4
after2.mp4
사전 조건: 테스트 계정의 스크랩 코스가 3개("JTBC마라톤 10KM" 포함)인 상태에서 녹화 시작.
영상 확인 체크리스트 (리뷰어가 직접 보면서 체크)