SCRUM-300 design: Add note report flow screen - #65
Conversation
Introduce NoteMenuBottomSheet to provide contextual actions for notes such as editing/deleting owned notes or reporting/blocking other users' notes. Add an onMenuClick listener to NoteComponent to trigger this bottom sheet.
Add NoteReport destination to FeelinNavHost and wire the report callback across screens. Display NoteMenuBottomSheet when the note menu button is clicked on HomeScreen, CommunityMainScreen, MyPageScreen, NoteDetailScreen, and NoteSearchResultScreen.
Apply specific icons (Modify, Delete, Report, Prohibit) to the menu actions in NoteMenuBottomSheet. Update FeelinModalBottomSheet to support rounded corners and fix icon spacing layouts.
Apply status bar and navigation bar padding to NoteReportScreen. Refactor radio buttons into ReportReasonItem using CheckBoxIconEnabled, and adjust spacing to match the design spec. Add UI tests in NoteReportScreenTest to verify default states, selection behavior, and button activation.
📝 WalkthroughWalkthroughAdds note menu actions across note surfaces, routes report actions to a new destination, implements the note report form and dialogs, updates bottom-sheet styling and icons, and adds Compose instrumentation tests for key report states. ChangesNote reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2e4751736
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/src/main/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreen.kt (2)
67-73: 🎯 Functional Correctness | 🔵 Trivial
noteIdis unused — report submission not yet wired up.The
@Suppress("UnusedParameter")annotation confirmsnoteIdis not consumed. The submit button (line 119) only shows a success dialog without calling any API. Would you like me to generate a ViewModel and API integration scaffold for the actual report submission?🤖 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/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreen.kt` around lines 67 - 73, Remove the unused noteId parameter and its `@Suppress`("UnusedParameter") annotation from NoteReportScreen until report submission is implemented. Update all call sites to match the simplified function signature, leaving the existing dialog-only submission behavior unchanged.
69-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove lambda parameters to the last position per coding guidelines.
Both
NoteReportScreenandReportReasonItemplace lambda parameters (onBackClick,onClick) before optional parameters, violating the guideline: "Keep lambda parameters as the last parameter in Composable functions." All call sites already use named arguments, so reordering is safe.♻️ Reorder parameters in both composables
`@Suppress`("UnusedParameter") `@Composable` fun NoteReportScreen( noteId: Long, - onBackClick: () -> Unit, modifier: Modifier = Modifier, initialDialogType: NoteReportDialogType? = null, + onBackClick: () -> Unit, ) {`@Composable` private fun ReportReasonItem( selected: Boolean, text: String, - onClick: () -> Unit, modifier: Modifier = Modifier + onClick: () -> Unit, ) {Also applies to: 223-229
🤖 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/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreen.kt` around lines 69 - 74, Reorder the parameters of the composable functions NoteReportScreen and ReportReasonItem so their lambda parameters (onBackClick and onClick) come after all optional parameters, preserving existing defaults and named-argument call-site behavior.Source: Coding guidelines
app/src/androidTest/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreenTest.kt (1)
44-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the "기타" conditional in
isSubmitEnabled.
selectingReasonAndAgreementEnablesSubmitonly tests a non-"기타" reason. TheisSubmitEnabledlogic at lines 81-82 has a conditional branch for "기타" requiring non-blankotherReasonText— this branch is not exercised. Consider adding a test that selects "기타", enters text, agrees, and verifies submit is enabled, plus a case where "기타" is selected with no text and submit remains disabled.✅ Suggested additional test cases
`@Test` fun selectingOtherWithTextAndAgreementEnablesSubmit() { setContent() composeTestRule.onNodeWithText("기타").performClick() composeTestRule.onNodeWithText("신고사유를 작성해주세요.").performClick() composeTestRule.onNodeWithText("개인정보 수집에 동의합니다.").performClick() composeTestRule.onNode(hasText("신고하기") and hasClickAction()) .assertIsEnabled() } `@Test` fun selectingOtherWithoutTextDisablesSubmit() { setContent() composeTestRule.onNodeWithText("기타").performClick() composeTestRule.onNodeWithText("개인정보 수집에 동의합니다.").performClick() composeTestRule.onNode(hasText("신고하기") and hasClickAction()) .assertIsNotEnabled() }🤖 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/androidTest/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreenTest.kt` around lines 44 - 55, Add test coverage in the NoteReportScreenTest cases for the “기타” branch of isSubmitEnabled: verify selecting “기타”, entering non-blank text, and agreeing enables submission, and verify selecting “기타” without text keeps the submit action disabled. Anchor the new tests alongside selectingReasonAndAgreementEnablesSubmit and preserve the existing interaction and assertion style.app/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteMenuBottomSheet.kt (1)
16-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the
selectedNoteForMenu+NoteMenuBottomSheetpattern.The same state-and-sheet wiring block is duplicated across
CommunityMainScreen,HomeScreen,MyPageScreen,NoteDetailScreen, andNoteSearchResultScreen:var selectedNoteForMenu by remember { mutableStateOf<NoteComponentData?>(null) } // … onMenuClick = { selectedNoteForMenu = it } // … selectedNoteForMenu?.let { selectedNote -> NoteMenuBottomSheet( noteData = selectedNote, currentUserId = currentUserId, onReportClick = { noteId -> selectedNoteForMenu = null onNoteReportClick(noteId) }, onDismissRequest = { selectedNoteForMenu = null }, ) }A small helper composable or a
rememberNoteMenuStateholder would centralize this logic and reduce maintenance across five call sites.🤖 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/java/com/lyrics/feelin/presentation/view/component/note/NoteMenuBottomSheet.kt` around lines 16 - 60, Extract the duplicated selectedNoteForMenu state and NoteMenuBottomSheet wiring into a reusable helper composable or rememberNoteMenuState holder. Update CommunityMainScreen, HomeScreen, MyPageScreen, NoteDetailScreen, and NoteSearchResultScreen to use it, preserving menu selection, dismissal, report handling, and the existing edit/delete/block callbacks.
🤖 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
`@app/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteMenuBottomSheet.kt`:
- Around line 23-26: Update NoteMenuBottomSheet’s onEditClick, onDeleteClick,
and onBlockClick parameters to be nullable instead of defaulting to empty no-op
lambdas, and conditionally render each corresponding action only when its
callback is provided. Preserve the existing visibility logic for ownership and
report actions, and safely invoke callbacks only when non-null.
---
Nitpick comments:
In
`@app/src/androidTest/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreenTest.kt`:
- Around line 44-55: Add test coverage in the NoteReportScreenTest cases for the
“기타” branch of isSubmitEnabled: verify selecting “기타”, entering non-blank text,
and agreeing enables submission, and verify selecting “기타” without text keeps
the submit action disabled. Anchor the new tests alongside
selectingReasonAndAgreementEnablesSubmit and preserve the existing interaction
and assertion style.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteMenuBottomSheet.kt`:
- Around line 16-60: Extract the duplicated selectedNoteForMenu state and
NoteMenuBottomSheet wiring into a reusable helper composable or
rememberNoteMenuState holder. Update CommunityMainScreen, HomeScreen,
MyPageScreen, NoteDetailScreen, and NoteSearchResultScreen to use it, preserving
menu selection, dismissal, report handling, and the existing edit/delete/block
callbacks.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreen.kt`:
- Around line 67-73: Remove the unused noteId parameter and its
`@Suppress`("UnusedParameter") annotation from NoteReportScreen until report
submission is implemented. Update all call sites to match the simplified
function signature, leaving the existing dialog-only submission behavior
unchanged.
- Around line 69-74: Reorder the parameters of the composable functions
NoteReportScreen and ReportReasonItem so their lambda parameters (onBackClick
and onClick) come after all optional parameters, preserving existing defaults
and named-argument call-site behavior.
🪄 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 Plus
Run ID: 93fd2cc0-2ede-441e-aede-63c961348fb8
📒 Files selected for processing (15)
app/src/androidTest/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreenTest.ktapp/src/main/java/com/lyrics/feelin/core/designsystem/component/FeelinModalBottomSheet.ktapp/src/main/java/com/lyrics/feelin/core/designsystem/icon/Icons.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinDestination.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/navigation/HomeNavigation.ktapp/src/main/java/com/lyrics/feelin/presentation/view/community/CommunityMainScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteComponent.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteMenuBottomSheet.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/HomeScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/FeedSection.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/detail/NoteDetailScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/report/NoteReportScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/result/NoteSearchResultScreen.kt
Expose the current user ID as a StateFlow in MyPageViewModel by reading from AuthRepository.
|
@codex re-review this. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.kt (1)
333-342: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftWire every action exposed by the bottom sheet.
NoteMenuBottomSheetprovides no-op defaults foronEditClick,onDeleteClick, andonBlockClick, but this call supplies onlyonReportClick. Consequently, edit/delete for owned notes and block for other users’ notes appear tappable but do nothing. Add and propagate the corresponding callbacks, or hide unsupported actions.🤖 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/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.kt` around lines 333 - 342, Update the NoteMenuBottomSheet invocation in MyPageScreen to wire onEditClick, onDeleteClick, and onBlockClick instead of relying on their no-op defaults. Propagate each action through the screen’s existing callback flow, clear selectedNoteForMenu consistently before invoking the handlers, and preserve the current onReportClick behavior.
🤖 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.
Outside diff comments:
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.kt`:
- Around line 333-342: Update the NoteMenuBottomSheet invocation in MyPageScreen
to wire onEditClick, onDeleteClick, and onBlockClick instead of relying on their
no-op defaults. Propagate each action through the screen’s existing callback
flow, clear selectedNoteForMenu consistently before invoking the handlers, and
preserve the current onReportClick behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 379b2ffe-2eae-4feb-9469-d89ad78ced6d
📒 Files selected for processing (2)
app/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageViewModel.kt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a88252c419
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onBlockClick: (Long) -> Unit = {}, | ||
| ) { | ||
| val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) | ||
| val isMyNote = currentUserId != null && currentUserId == noteData.publisher.id |
There was a problem hiding this comment.
isMyNote가 currentUserId == publisher.id에만 의존하지만 MyPage를 제외한 프로덕션 진입점(HomeRoute, CommunityMainScreen, NoteDetailScreen, NoteSearchResultScreen)은 모두 기본값 null을 사용합니다. 그 결과 홈/검색/상세/커뮤니티에서 본인 노트의 더보기 메뉴를 열어도 수정·삭제 대신 신고·차단 메뉴가 표시되어 자기 노트를 신고/차단하는 잘못된 흐름이 됩니다.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
마이페이지 화면을 제외한 화면의 사용자 ID 전달은 실제 기능 구현시 진행하겠습니다.
outside diff range comments 는 #65 (comment) 와 동일한 이유로 실제 기능 구현시 진행하겠습니다. |
|
병합하겠습니다. |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
노트 메뉴에서 신고 화면으로 진입할 수 없으며, 신고 사유를 선택하고 제출하는 화면이 구현되어 있지 않았습니다.
What is the new behavior (if this is a feature change)?
노트 메뉴 바텀시트와 신고 화면을 추가하고 주요 노트 화면에 연결했습니다.
NoteReportScreen을 구현했습니다.NoteMenuBottomSheet를 추가했습니다.Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No breaking changes.
ScreenShots (If needed)
Light mode
Dark mode
Other information:
AI Agent
User
이번 화면 레이아웃 구현중 확인한 부분인데, FeelinCheckbox의 disabled icon이 다크모드에 대응하지 않는 것을 확인했습니다. 이를 별도의 이슈로 등록한 후 추후 수정하겠습니다.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Tests