[Feat/#152] 태그 및 자연어처리 버튼 초기화, 스케쥴 하루종일 로직 추가 #157
Conversation
into feat/#152-natural-language-refactor
|
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedNaturalLanguage.kt (1)
247-250: Fix key lookup bug for Korean relative date keywordsYou normalize to
e, but lookup uses the untrimmedexpr. This can miss matches with stray spaces.Apply within this range:
- mapOf("그제" to -2, "어제" to -1, "오늘" to 0, "내일" to 1, "모레" to 2) - .takeIf { it.containsKey(expr) } - ?.let { return now.plusDays(it[expr]!!.toLong()).truncatedToDays() } + mapOf("그제" to -2, "어제" to -1, "오늘" to 0, "내일" to 1, "모레" to 2) + .takeIf { it.containsKey(e) } + ?.let { return now.plusDays(it[e]!!.toLong()).truncatedToDays() }app/src/main/java/org/memento/presentation/plusbottomsheet/AddToDoViewModel.kt (1)
304-320: Cancel any in-flight parse on reset to avoid post-reset state mutationsWithout canceling, a pending job may repopulate fields after reset if it completes later.
Apply near the start of resetData():
fun resetData() { + parseJob?.cancel() + parseJob = null _selectedDateText.value = "Today" _addToDoText.value = "" _addTagId.value = 0 _addTagColor.value = "#F0F0F3" _deadLineText.value = "Add DeadLine" _addPriorityType.value = PriorityTagType.None _isSwitchOn.value = false
🧹 Nitpick comments (6)
app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedDateResult.kt (1)
5-10: Consider Java interop: add @jvmoverloads to avoid breaking Java call sitesIf any Java code constructs ParsedDateResult, the added param requires a source change. @jvmoverloads preserves Java overloads with defaults.
Apply within this range:
-data class ParsedDateResult( +data class ParsedDateResult @JvmOverloads constructor( val title: String, val startDate: LocalDateTime? = null, val endDate: LocalDateTime? = null, val isAllDay: Boolean = false, )app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedNaturalLanguage.kt (2)
173-177: Strip all occurrences of all-day markers, not just the first matchEnsures no leftover “종일/all-day” fragments remain in titles.
Apply within this range:
-// 3-0) 먼저 all-day 의미 표현 감지 및 제거 -ALL_DAY_REGEX.find(text)?.let { m -> - isAllDay = true - text = text.replace(m.value, "") -} +// 3-0) 먼저 all-day 의미 표현 감지 및 제거 +if (ALL_DAY_REGEX.containsMatchIn(text)) { + isAllDay = true + text = ALL_DAY_REGEX.replace(text, "") +}
236-237: Use locale-stable case conversionstoLowerCase()/toUpperCase() are locale-dependent and deprecated. Use lowercase/uppercase with Locale.ROOT to avoid edge cases (e.g., Turkish I).
Apply within these ranges:
- val e = expr.trim().toLowerCase() + val e = expr.trim().lowercase(java.util.Locale.ROOT)- val m = months[mon.toLowerCase()] ?: now.monthValue + val m = months[mon.lowercase(java.util.Locale.ROOT)] ?: now.monthValue- val dow = DayOfWeek.valueOf(dayEn.toUpperCase()) + val dow = DayOfWeek.valueOf(dayEn.uppercase(java.util.Locale.ROOT))Also add the import:
import java.util.LocaleAlso applies to: 293-295, 316-318
app/src/main/java/org/memento/presentation/plusbottomsheet/AddToDoViewModel.kt (1)
96-104: Guard against empty tag list to prevent IndexOutOfBoundsExceptionIf the repository returns an empty list,
tags[0]will crash. UsefirstOrNull().Apply within this range:
- if (_addTagId.value == 0) { - _addTagId.value = tags[0].id - } + if (_addTagId.value == 0) { + tags.firstOrNull()?.let { _addTagId.value = it.id } + }Do we guarantee at least one tag exists for all users/tenants? If yes, you can ignore this.
app/src/main/java/org/memento/presentation/plusbottomsheet/AddScheduleViewModel.kt (2)
261-266: Re-check switch state after debounce to avoid parsing when turned offIf the user flips the NLP switch off during the 500ms delay, parsing still proceeds. Add a guard after the debounce/equality check.
val source = input delay(500L) // 입력이 변경되었으면 중단 if (_eventText.value != source) return@launch + // 스위치 상태 재확인 (디바운스 중 Off로 바뀐 경우 중단) + if (!_isSwitchOn.value) return@launch
274-276: Guard against stale updates and empty titles before applying parse resultsDouble-check staleness just before applying, and avoid overwriting title with blank.
- val parsed = parseNaturalLanguage(_eventText.value, isParseTime = true) - _eventText.value = parsed.title + val parsed = parseNaturalLanguage(source, isParseTime = true) + // 적용 직전, 입력이 바뀌었으면 중단 + if (_eventText.value != source) return@launch + if (parsed.title.isNotBlank()) { + _eventText.value = parsed.title + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/org/memento/presentation/plusbottomsheet/AddScheduleViewModel.kt(3 hunks)app/src/main/java/org/memento/presentation/plusbottomsheet/AddToDoViewModel.kt(2 hunks)app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedDateResult.kt(1 hunks)app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedNaturalLanguage.kt(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI
🔇 Additional comments (8)
app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedDateResult.kt (1)
9-10: Adding isAllDay to the result model looks goodDefaulted flag preserves source compatibility for Kotlin callers.
app/src/main/java/org/memento/presentation/plusbottomsheet/ParsedNaturalLanguage.kt (2)
11-11: All-day regex addition is appropriateGood coverage for “하루 종일/종일/all-day”.
220-225: Propagating isAllDay in the result is consistent with the new parsing behaviorNo further action needed here.
app/src/main/java/org/memento/presentation/plusbottomsheet/AddToDoViewModel.kt (2)
271-283: Debounce + stale-input guard + blank-input reset look solidUsing a source snapshot with a 500ms delay prevents stale parses, and resetting dates when the input is cleared matches the issue requirements.
284-286: Parsing with time disabled is appropriate for ToDoKeeps date-only behavior and avoids unintended time parsing.
app/src/main/java/org/memento/presentation/plusbottomsheet/AddScheduleViewModel.kt (3)
267-272: Early reset on blank input looks goodNice UX: restoring default times and clearing all-day when input is blank and NLP is on.
287-292: ```shell
#!/bin/bashLocate and inspect the relevant logic in AddScheduleViewModel.kt
FILE="app/src/main/java/org/memento/presentation/plusbottomsheet/AddScheduleViewModel.kt"
rg -n -C2 'parsed.isAllDay|validateTimeOrder|updateAllDayCheck' "$FILE"Find the definition of calculateEndTime in the same package
rg -n 'fun calculateEndTime' -n app/src/main/java/org/memento/presentation/plusbottomsheet/*.kt
--- `353-361`: **Remove invalid `parseJob` suggestion** The ViewModel doesn’t declare or use any `parseJob` property, so cancelling it in `resetData()` isn’t applicable. Discard the diff and focus on real cancellable coroutines or jobs if needed. > Likely an incorrect or invalid review comment. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ALL_DAY_REGEX.find(text)?.let { m -> | ||
| isAllDay = true |
| val source = input | ||
| delay(500L) | ||
|
|
||
| val parsed = parseNaturalLanguage(input, isParseTime = true) | ||
| // 입력이 변경되었으면 중단 | ||
| if (_eventText.value != source) return@launch |
✅ 𝗖𝗵𝗲𝗰𝗸-𝗟𝗶𝘀𝘁
📌 𝗜𝘀𝘀𝘂𝗲𝘀
📎𝗪𝗼𝗿𝗸 𝗗𝗲𝘀𝗰𝗿𝗶𝗽𝘁𝗶𝗼𝗻
📷 𝗦𝗰𝗿𝗲𝗲𝗻𝘀𝗵𝗼𝘁
Screen_recording_20250907_222446.mp4
💬 𝗧𝗼 𝗥𝗲𝘃𝗶𝗲𝘄𝗲𝗿𝘀
Summary by CodeRabbit
New Features
Bug Fixes