Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.daedan.festabook.presentation.common.component

import androidx.compose.foundation.layout.wrapContentSize
import androidx.compose.material3.Switch
import androidx.compose.material3.SwitchDefaults
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import com.daedan.festabook.presentation.theme.FestabookColor

@Composable
fun FestabookSwitch(
enabled: Boolean,
checked: Boolean,
onCheckedChange: (Boolean) -> Unit,
modifier: Modifier = Modifier,
) {
Switch(
enabled = enabled,
modifier = modifier.wrapContentSize(),
checked = checked,
onCheckedChange = onCheckedChange,
colors =
SwitchDefaults.colors().copy(
checkedBorderColor = Color.Transparent,
uncheckedBorderColor = Color.Transparent,
disabledCheckedTrackColor = FestabookColor.black,
disabledUncheckedTrackColor = FestabookColor.gray200,
checkedTrackColor = FestabookColor.black,
uncheckedTrackColor = FestabookColor.gray200,
),
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.daedan.festabook.presentation.common.component

import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.compose.LocalLifecycleOwner
import androidx.lifecycle.repeatOnLifecycle
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.withContext

// MVI 리팩토링 PR에도 동일한 코드가 사용되어
// 머지 시 해당 부분 제거하여 충돌을 해결하겠습니다.
@Composable
fun <T> ObserveAsEvents(
flow: Flow<T>,
onEvent: suspend (T) -> Unit,
) {
val lifecycleOwner = LocalLifecycleOwner.current
LaunchedEffect(flow, lifecycleOwner.lifecycle) {
lifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
withContext(Dispatchers.Main.immediate) {
flow.collect(onEvent)
}
}
}
}
Comment on lines +14 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은데요?

Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,33 @@ package com.daedan.festabook.presentation.setting

import android.content.Intent
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.activity.result.ActivityResultLauncher
import androidx.activity.result.contract.ActivityResultContracts
import androidx.compose.runtime.getValue
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.ViewCompositionStrategy
import androidx.core.net.toUri
import androidx.fragment.app.Fragment
import androidx.fragment.app.viewModels
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.daedan.festabook.BuildConfig
import com.daedan.festabook.R
import com.daedan.festabook.databinding.FragmentSettingBinding
import com.daedan.festabook.di.fragment.FragmentKey
import com.daedan.festabook.presentation.NotificationPermissionManager
import com.daedan.festabook.presentation.NotificationPermissionRequester
import com.daedan.festabook.presentation.common.BaseFragment
import com.daedan.festabook.presentation.common.component.ObserveAsEvents
import com.daedan.festabook.presentation.common.showErrorSnackBar
import com.daedan.festabook.presentation.common.showNotificationDeniedSnackbar
import com.daedan.festabook.presentation.common.showSnackBar
import com.daedan.festabook.presentation.home.HomeViewModel
import com.daedan.festabook.presentation.setting.component.SettingScreen
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesIntoMap
import dev.zacsweers.metro.Inject
Expand Down Expand Up @@ -58,7 +67,7 @@ class SettingFragment(
onPermissionGranted()
} else {
Timber.d("Notification permission denied")
showNotificationDeniedSnackbar(binding.root, requireContext())
showNotificationDeniedSnackbar(requireView(), requireContext())
onPermissionDenied()
}
}
Expand All @@ -69,87 +78,60 @@ class SettingFragment(

override fun onPermissionDenied() = Unit

override fun onViewCreated(
view: View,
override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
) {
super.onViewCreated(view, savedInstanceState)
setupBindings()

setupNoticeAllowButtonClickListener()
setupServicePolicyClickListener()
setupContactUsButtonClickListener()
setupObservers()
}

private fun setupBindings() {
val versionName = BuildConfig.VERSION_NAME
binding.tvSettingAppVersionName.text = versionName
}

override fun shouldShowPermissionRationale(permission: String): Boolean = shouldShowRequestPermissionRationale(permission)

private fun setupObservers() {
settingViewModel.permissionCheckEvent.observe(viewLifecycleOwner) {
notificationPermissionManager.requestNotificationPermission(
requireContext(),
)
}
settingViewModel.isAllowed.observe(viewLifecycleOwner) {
binding.btnNoticeAllow.isChecked = it
}
settingViewModel.success.observe(viewLifecycleOwner) {
requireActivity().showSnackBar(getString(R.string.setting_notice_enabled))
}
settingViewModel.error.observe(viewLifecycleOwner) { throwable ->
showErrorSnackBar(throwable)
}
settingViewModel.isLoading.observe(viewLifecycleOwner) { loading ->
binding.btnNoticeAllow.isEnabled = !loading
}

// homeViewModel.festivalUiState.observe(viewLifecycleOwner) { state ->
// when (state) {
// is FestivalUiState.Error -> {
// showErrorSnackBar(state.throwable)
// Timber.w(
// state.throwable,
// "${this::class.simpleName}: ${state.throwable.message}",
// )
// }
//
// FestivalUiState.Loading -> {
// binding.tvSettingCurrentUniversityNotice.text = ""
// }
//
// is FestivalUiState.Success -> {
// binding.tvSettingCurrentUniversity.text = state.organization.universityName
// }
// }
// }
}

private fun setupServicePolicyClickListener() {
binding.tvSettingServicePolicy.setOnClickListener {
val intent = Intent(Intent.ACTION_VIEW, POLICY_URL.toUri())
startActivity(intent)
}
}

private fun setupContactUsButtonClickListener() {
binding.tvSettingContactUs.setOnClickListener {
val intent = Intent(Intent.ACTION_VIEW, CONTACT_US_URL.toUri())
startActivity(intent)
): View =
ComposeView(requireContext()).apply {
super.onCreateView(inflater, container, savedInstanceState)
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
Comment on lines +86 to +88
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.kt" | xargs grep -l "class BaseFragment" | head -5

Repository: festabook/android

Length of output: 137


🏁 Script executed:

cat -n app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt | sed -n '1,50p'

Repository: festabook/android

Length of output: 2673


🏁 Script executed:

cat -n app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt | sed -n '80,110p'

Repository: festabook/android

Length of output: 1721


🏁 Script executed:

cat -n ./app/src/main/java/com/daedan/festabook/presentation/common/BaseFragment.kt

Repository: festabook/android

Length of output: 2326


🏁 Script executed:

cat -n app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt | sed -n '80,132p'

Repository: festabook/android

Length of output: 2794


🏁 Script executed:

# Check if FragmentSettingBinding is used anywhere in SettingFragment
rg "FragmentSettingBinding" app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt -n

Repository: festabook/android

Length of output: 173


🏁 Script executed:

# Check if SettingFragment accesses binding anywhere
rg "binding\." app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt -n

Repository: festabook/android

Length of output: 126


🏁 Script executed:

# Check if binding is accessed in SettingFragment
rg "this\.binding" app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt -n

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Verify the complete SettingFragment file to ensure nothing uses binding
wc -l app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt

Repository: festabook/android

Length of output: 143


Remove unnecessary super.onCreateView() call and unused binding type parameter.

The super.onCreateView() call at line 87 inflates the XML layout (fragment_setting.xml) via DataBindingUtil.inflate() in BaseFragment, but the created binding and return value are immediately discarded since SettingFragment returns a ComposeView instead. This wastes resources during initialization. Additionally, the FragmentSettingBinding type parameter is never accessed in this fragment.

Remove the super.onCreateView() call and change the class declaration to BaseFragment<FragmentSettingBinding>() if the base class type parameter is required by the framework, or refactor to remove the type parameter entirely if possible.

🤖 Prompt for AI Agents
In
@app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt
around lines 86 - 88, Remove the redundant super.onCreateView(...) invocation
inside SettingFragment (it inflates fragment_setting.xml in BaseFragment and its
result is unused) and update the class generic: change the class declaration to
extend BaseFragment<FragmentSettingBinding>() if the base requires a type
parameter, or remove the generic from BaseFragment usage entirely if not needed;
ensure no other code in SettingFragment references the discarded binding and
keep the ComposeView creation and setViewCompositionStrategy call intact.

setContent {
val festival by homeViewModel.festivalUiState.collectAsStateWithLifecycle()
val isUniversitySubscribed by settingViewModel.isAllowed.collectAsStateWithLifecycle()
val isSubscribedLoading by settingViewModel.isLoading.collectAsStateWithLifecycle()
val context = LocalContext.current

ObserveAsEvents(flow = settingViewModel.permissionCheckEvent) {
notificationPermissionManager.requestNotificationPermission(context)
}

ObserveAsEvents(flow = settingViewModel.successFlow) {
requireActivity().showSnackBar(getString(R.string.setting_notice_enabled))
}

ObserveAsEvents(flow = settingViewModel.error) {
showErrorSnackBar(it)
}

SettingScreen(
festivalUiState = festival,
isUniversitySubscribed = isUniversitySubscribed,
appVersion = BuildConfig.VERSION_NAME,
isSubscribeEnabled = !isSubscribedLoading,
onSubscribeClick = {
settingViewModel.notificationAllowClick()
},
onPolicyClick = {
val intent = Intent(Intent.ACTION_VIEW, POLICY_URL.toUri())
startActivity(intent)
},
onContactUsClick = {
val intent = Intent(Intent.ACTION_VIEW, CONTACT_US_URL.toUri())
startActivity(intent)
},
onError = {
showErrorSnackBar(it.throwable)
Timber.w(
it.throwable,
"${this::class.simpleName}: ${it.throwable.message}",
)
},
)
}
}
}

private fun setupNoticeAllowButtonClickListener() {
binding.btnNoticeAllow.setOnClickListener {
// 기본적으로 클릭했을 때 checked되는 기능 무효화
binding.btnNoticeAllow.isChecked = !binding.btnNoticeAllow.isChecked
settingViewModel.notificationAllowClick()
}
}
override fun shouldShowPermissionRationale(permission: String): Boolean = shouldShowRequestPermissionRationale(permission)

companion object {
private const val POLICY_URL: String =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package com.daedan.festabook.presentation.setting

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.asLiveData
import androidx.lifecycle.viewModelScope
import com.daedan.festabook.di.viewmodel.ViewModelKey
import com.daedan.festabook.domain.repository.FestivalNotificationRepository
import com.daedan.festabook.presentation.common.SingleLiveData
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesIntoMap
import dev.zacsweers.metro.Inject
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
import timber.log.Timber

Expand All @@ -19,27 +24,33 @@ import timber.log.Timber
class SettingViewModel(
private val festivalNotificationRepository: FestivalNotificationRepository,
) : ViewModel() {
private val _permissionCheckEvent: SingleLiveData<Unit> = SingleLiveData()
val permissionCheckEvent: LiveData<Unit> get() = _permissionCheckEvent
private val _permissionCheckEvent: MutableSharedFlow<Unit> =
MutableSharedFlow()
val permissionCheckEvent: SharedFlow<Unit> = _permissionCheckEvent.asSharedFlow()

private val _isAllowed =
MutableLiveData(
MutableStateFlow(
festivalNotificationRepository.getFestivalNotificationIsAllow(),
)
val isAllowed: LiveData<Boolean> get() = _isAllowed
val isAllowed: StateFlow<Boolean> = _isAllowed.asStateFlow()

private val _error: SingleLiveData<Throwable> = SingleLiveData()
val error: LiveData<Throwable> get() = _error
private val _error: MutableSharedFlow<Throwable> =
MutableSharedFlow()
val error: SharedFlow<Throwable> = _error.asSharedFlow()

private val _isLoading: MutableLiveData<Boolean> = MutableLiveData(false)
val isLoading: LiveData<Boolean> get() = _isLoading
private val _isLoading: MutableStateFlow<Boolean> = MutableStateFlow(false)
val isLoading: StateFlow<Boolean> = _isLoading.asStateFlow()

private val _success: SingleLiveData<Unit> = SingleLiveData()
val success: LiveData<Unit> get() = _success
private val _success: MutableSharedFlow<Unit> =
MutableSharedFlow()
val success: LiveData<Unit> = _success.asLiveData()
val successFlow = _success.asSharedFlow()
Comment on lines +46 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요친구만 LiveData로 하신 이유를 알 수 있을까요?


fun notificationAllowClick() {
if (_isAllowed.value == false) {
_permissionCheckEvent.setValue(Unit)
if (!_isAllowed.value) {
viewModelScope.launch {
_permissionCheckEvent.emit(Unit)
}
} else {
deleteNotificationId()
}
Expand All @@ -54,21 +65,22 @@ class SettingViewModel(
}

fun saveNotificationId() {
if (_isLoading.value == true) return
if (_isLoading.value) return
_isLoading.value = true

// Optimistic UI 적용, 요청 실패 시 원복
saveNotificationIsAllowed(true)
updateNotificationIsAllowed(true)
_success.setValue(Unit)

viewModelScope.launch {
_success.emit(Unit)

val result =
festivalNotificationRepository.saveFestivalNotification()

result
.onFailure {
_error.setValue(it)
_error.emit(it)
saveNotificationIsAllowed(false)
updateNotificationIsAllowed(false)
Timber.e(it, "${this::class.java.simpleName} NotificationId 저장 실패")
Expand All @@ -79,7 +91,7 @@ class SettingViewModel(
}

private fun deleteNotificationId() {
if (_isLoading.value == true) return
if (_isLoading.value) return
_isLoading.value = true

// Optimistic UI 적용, 요청 실패 시 원복
Expand All @@ -92,7 +104,7 @@ class SettingViewModel(

result
.onFailure {
_error.setValue(it)
_error.emit(it)
saveNotificationIsAllowed(true)
updateNotificationIsAllowed(true)
Timber.e(it, "${this::class.java.simpleName} NotificationId 삭제 실패")
Expand Down
Loading