Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@
android:value="" />
</activity>
<activity
android:name=".presentation.review.delete.MyReviewDialogActivity"
android:name=".presentation.common.MyReviewBottomSheetFragment"
android:exported="true"
android:theme="@style/Theme.MyDialog">
<meta-data
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.eatssu.android.presentation.common

import android.content.Intent
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.appcompat.app.AlertDialog
import androidx.fragment.app.activityViewModels
import androidx.lifecycle.lifecycleScope
import com.eatssu.android.App
import com.eatssu.android.R
import com.eatssu.android.databinding.FragmentBottomsheetMyReviewBinding
import com.eatssu.android.presentation.mypage.myreview.MyReviewViewModel
import com.eatssu.android.presentation.review.modify.ModifyReviewActivity
import com.eatssu.android.presentation.util.showToast
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.launch
import timber.log.Timber

@AndroidEntryPoint
class MyReviewBottomSheetFragment : BottomSheetDialogFragment() {
private var _binding: FragmentBottomsheetMyReviewBinding? = null
private val binding get() = _binding!!

interface OnReviewDeletedListener {
fun onReviewDeleted()
}

var onReviewDeletedListener: OnReviewDeletedListener? = null

private val viewModel: MyReviewViewModel by activityViewModels()

var reviewId = -1L
var menu = ""
var content = ""
var mainGrade = -1
var amountGrade = -1
var tasteGrade = -1
Comment on lines +36 to +41

This comment was marked as outdated.

Comment on lines +36 to +41
Copy link
Member

Choose a reason for hiding this comment

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

   var reviewId = -1L
    var menu = ""
    var content = ""
    var mainGrade = -1
    var amountGrade = -1
    var tasteGrade = -1

이런 정보는 이렇게 변수로 선언하는 것보다는 뷰모델에서 uiState로 가지고 있는게 좋지 않나요??
어차피 MyReviewViewModel을 activityViewModels 로 했응까.. 거기에 state저장해두고 가져오도록 하면 intent로 굳이 안넘겨도 되지 않나 해서용

Copy link
Member Author

@HI-JIN2 HI-JIN2 Mar 27, 2025

Choose a reason for hiding this comment

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

읆 그러면 activityViewModels인 MyReviewViewModel을 MyReviewBottomSheetFragment에서도 쓰고 ModifyReviewActivity에서도 쓰자는 말인거죵!?

ModifyReviewActivity에서는 ModifyViewModel이랑 MyReviewBottomSheetFragment 둘다 쓰고?

Copy link
Member

Choose a reason for hiding this comment

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

네네 맞아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

엄... 이건 다른 PR에서 작업해도 될까요? 해당 PR이 장기화 되는 것 같아서요..!

Copy link
Member

Choose a reason for hiding this comment

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

넵~! 어푸르브 하겠슴니당


override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View {
_binding = FragmentBottomsheetMyReviewBinding.inflate(inflater, container, false)
return binding.root
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)

arguments?.let {
reviewId = it.getLong("reviewId")
menu = it.getString("menu").toString()
content = it.getString("content").toString()
mainGrade = it.getInt("mainGrade")
amountGrade = it.getInt("amountGrade")
tasteGrade = it.getInt("tasteGrade")
}

Timber.d("넘겨받은 리뷰 정보: $reviewId $menu $content $reviewId")

binding.llModify.setOnClickListener {
val intent = Intent(requireContext(), ModifyReviewActivity::class.java)

intent.let {
it.putExtra("reviewId", reviewId)
it.putExtra("menu", menu)
it.putExtra("content", content)
it.putExtra("mainGrade", mainGrade)
it.putExtra("amountGrade", amountGrade)
it.putExtra("tasteGrade", tasteGrade)
}

startActivity(intent)
dismiss()
}

binding.llDelete.setOnClickListener {
AlertDialog.Builder(requireContext()).apply {
setTitle(R.string.delete)
setMessage(R.string.delete_description)
setNegativeButton("취소") { _, _ ->
activity?.showToast(App.appContext.getString(R.string.delete_undo))
}
setPositiveButton("삭제") { _, _ ->
viewModel.deleteReview(reviewId)
lifecycleScope.launch {
viewModel.uiState.collectLatest {
if (it.isDeleted) {
onReviewDeletedListener?.onReviewDeleted() // 콜백 호출
dismiss()
}
}
}
}
}.create().show()
}
Comment on lines +82 to +101
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

삭제 로직 개선 필요

다음 사항들을 개선하면 좋을 것 같습니다:

  • 다이얼로그 버튼의 문자열을 strings.xml로 이동
  • 삭제 실패 시 에러 처리 로직 추가
  • 코루틴 스코프 취소 처리 추가
-                setNegativeButton("취소") { _, _ ->
-                    activity?.showToast(App.appContext.getString(R.string.delete_undo))
-                }
-                setPositiveButton("삭제") { _, _ ->
+                setNegativeButton(R.string.cancel) { _, _ ->
+                    activity?.showToast(getString(R.string.delete_undo))
+                }
+                setPositiveButton(R.string.delete) { _, _ ->
                     viewModel.deleteReview(reviewId)
                     lifecycleScope.launch {
+                        try {
                             viewModel.uiState.collectLatest {
                                 if (it.isDeleted) {
                                     onReviewDeletedListener?.onReviewDeleted() // 콜백 호출
                                     dismiss()
+                                } else if (it.error != null) {
+                                    activity?.showToast(getString(R.string.delete_error))
                                 }
                             }
+                        } catch (e: Exception) {
+                            activity?.showToast(getString(R.string.unknown_error))
+                        }
                     }
                 }

Committable suggestion skipped: line range outside the PR's diff.


}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.eatssu.android.presentation.common

import android.content.Intent
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import com.eatssu.android.databinding.FragmentBottomsheetOthersBinding
import com.eatssu.android.presentation.review.report.ReportActivity
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import dagger.hilt.android.AndroidEntryPoint
import timber.log.Timber

@AndroidEntryPoint
class OthersBottomSheetFragment : BottomSheetDialogFragment() {
private var _binding: FragmentBottomsheetOthersBinding? = null
private val binding get() = _binding!!

var reviewId = -1L
var menu = ""

Comment on lines +16 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

메모리 누수 방지를 위한 바인딩 정리 필요

바인딩 객체가 프래그먼트 수명주기 이후에도 유지될 수 있어 메모리 누수가 발생할 수 있습니다. onDestroyView()에서 바인딩을 정리해주세요.

다음과 같이 수정해주세요:

+ override fun onDestroyView() {
+     super.onDestroyView()
+     _binding = null
+ }

또한 reviewId와 menu를 데이터 클래스로 캡슐화하는 것을 고려해보세요.

Committable suggestion skipped: line range outside the PR's diff.

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View {
_binding = FragmentBottomsheetOthersBinding.inflate(inflater, container, false)
return binding.root
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)

arguments?.let {
reviewId = it.getLong("reviewId")
menu = it.getString("menu").toString()
}
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

인자 검증 로직 개선 필요

arguments에서 reviewId와 menu를 가져올 때 다음과 같은 문제점들이 발견되었습니다:

  • reviewId가 -1L인 경우에 대한 검증이 누락되어 있습니다
  • menu는 null 체크 없이 toString()을 호출하고 있어 NPE 발생 가능성이 있습니다
  • 로그 메시지가 한글로 작성되어 있어 일관성이 떨어집니다

수정이 필요한 부분:

  • arguments?.let 블록 내에서 reviewId와 menu 값 검증 추가
  • menu는 it.getString("menu")?.toString() ?: "" 형태로 안전한 널 처리 필요
  • 영문 로그 메시지로 통일
🔗 Analysis chain

인자 유효성 검사 추가 필요

reviewId가 유효하지 않은 경우(-1)에 대한 처리가 필요합니다. 또한 menu가 null이나 빈 문자열일 경우의 처리도 필요합니다.


Let me check how these reviewId values are handled in the related files.


Let me check how these fragments handle their lifecycle and any error cases.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any null checks for these arguments in other files
rg "reviewId.*-1|menu.*isEmpty" app/src/main/java/

Length of output: 786


Script:

#!/bin/bash
# Check the implementation of these bottom sheet fragments and activities
ast-grep --pattern 'class $_ : BottomSheetDialogFragment {
  $$$
  reviewId $$$
  $$$
}'

# Also check for menu parameter handling
rg "menu.*toString|getString.*menu" app/src/main/java/

Length of output: 1924


Script:

#!/bin/bash
# Check implementation of these bottom sheet fragments
cat app/src/main/java/com/eatssu/android/presentation/common/OthersBottomSheetFragment.kt
cat app/src/main/java/com/eatssu/android/presentation/common/MyReviewBottomSheetFragment.kt

# Check if there's any error handling in parent classes
rg "class.*BottomSheetDialogFragment" app/src/main/java/

Length of output: 5925


Timber.d("넘겨받은 리뷰 정보: $reviewId $menu")

binding.llReport.setOnClickListener {

val intent = Intent(requireContext(), ReportActivity::class.java)
intent.putExtra("reviewId", reviewId)
Timber.d("reviewId $reviewId")
startActivity(intent)
dismiss()
}
}
}
Original file line number Diff line number Diff line change
@@ -1,69 +1,61 @@
package com.eatssu.android.presentation.mypage.myreview

import android.content.Intent
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.ImageView
import androidx.core.content.ContextCompat
import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter
import com.bumptech.glide.Glide
import com.eatssu.android.data.MySharedPreferences
import com.eatssu.android.databinding.ItemReviewBinding
import com.eatssu.android.domain.model.Review
import com.eatssu.android.presentation.review.delete.MyReviewDialogActivity
import timber.log.Timber

class MyReviewAdapter :
ListAdapter<Review, MyReviewAdapter.ViewHolder>(ReviewDiffCallback()) {

class MyReviewAdapter(private val dataList: List<Review>) :
RecyclerView.Adapter<MyReviewAdapter.ViewHolder>() {
interface OnItemClickListener {
fun onMyReviewClicked(view: View, reviewData: Review)
}

inner class ViewHolder(private val binding: ItemReviewBinding) :
RecyclerView.ViewHolder(binding.root) {
// 객체 저장 변수
private lateinit var mOnItemClickListener: OnItemClickListener

fun bind(position: Int) {
binding.tvReviewItemComment.text = dataList[position].content
binding.tvReviewItemDate.text = dataList[position].writeDate
binding.tvMenuName.text = dataList[position].menu
// 객체 전달 메서드
fun setOnItemClickListener(onItemClickListener: OnItemClickListener) {
mOnItemClickListener = onItemClickListener
}
Comment on lines +23 to +28
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

클릭 리스너 설정 메서드의 안전성 강화 필요

setOnItemClickListener 메서드에서 mOnItemClickListener를 설정하고 있는데, 이 경우 클릭 이벤트 전에 반드시 이 메서드가 호출되어야 합니다. 이를 보장하기 위해 초기값을 설정하거나 예외 처리가 필요합니다.

예를 들어, mOnItemClickListener를 nullable로 선언하고 클릭 시 null 체크를 하거나, 초기값을 설정하는 방법을 고려해 볼 수 있습니다.


binding.rbRate.rating = dataList[position].mainGrade.toFloat()
inner class ViewHolder(private val binding: ItemReviewBinding) :
androidx.recyclerview.widget.RecyclerView.ViewHolder(binding.root) {

fun bind(data: Review) {
binding.tvWriterNickname.text = MySharedPreferences.getUserName(binding.root.context)
binding.tvReviewItemComment.text = data.content
binding.tvReviewItemDate.text = data.writeDate
binding.tvMenuName.text = data.menu
binding.rbRate.rating = data.mainGrade.toFloat()

val imageView: ImageView = binding.ivReviewPhoto

if (dataList[position].imgUrl?.isEmpty() == true) {
if (data.imgUrl?.isEmpty() == true || data.imgUrl?.get(0).isNullOrEmpty()) {
imageView.visibility = View.GONE
} else {
Timber.d("사진 있다")
Glide.with(itemView)
.load(dataList[position].imgUrl?.get(0))
.load(data.imgUrl?.get(0))
.into(imageView)
imageView.visibility = View.VISIBLE

if (dataList[position].imgUrl?.get(0) == "" || dataList[position].imgUrl?.get(0) == null) {
binding.ivReviewPhoto.visibility = View.GONE
}
}

binding.btnDetail.setOnClickListener {
val intent = Intent(binding.btnDetail.context, MyReviewDialogActivity::class.java)
intent.putExtra("reviewId", dataList[position].reviewId)
intent.putExtra("menu", dataList[position].menu)

intent.putExtra("content", dataList[position].content)

intent.putExtra("mainGrade", dataList[position].mainGrade)
intent.putExtra("amountGrade", dataList[position].amountGrade)
intent.putExtra("tasteGrade", dataList[position].tasteGrade)
binding.btnDetail.setOnClickListener { v: View ->
mOnItemClickListener.onMyReviewClicked(v, data)

Timber.d(
"리뷰 상세 정보 - ID: %d, 메뉴: %s, 내용: %s",
dataList[position].reviewId,
dataList[position].menu,
dataList[position].content
data.reviewId,
data.menu,
data.content
)

ContextCompat.startActivity(binding.btnDetail.context, intent, null)
}
}
}
Expand All @@ -75,8 +67,17 @@ class MyReviewAdapter(private val dataList: List<Review>) :
}

override fun onBindViewHolder(holder: ViewHolder, position: Int) {
holder.bind(position)
val item = getItem(position)
holder.bind(item)
}
}

override fun getItemCount(): Int = dataList.size
}
class ReviewDiffCallback : DiffUtil.ItemCallback<Review>() {
override fun areItemsTheSame(oldItem: Review, newItem: Review): Boolean {
return oldItem.reviewId == newItem.reviewId
}

override fun areContentsTheSame(oldItem: Review, newItem: Review): Boolean {
return oldItem == newItem
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@ package com.eatssu.android.presentation.mypage.myreview
import android.os.Bundle
import android.view.View
import androidx.activity.viewModels
import androidx.fragment.app.DialogFragment
import androidx.lifecycle.lifecycleScope
import androidx.recyclerview.widget.LinearLayoutManager
import com.eatssu.android.R
import com.eatssu.android.databinding.ActivityMyReviewListBinding
import com.eatssu.android.domain.model.Review
import com.eatssu.android.presentation.base.BaseActivity
import com.eatssu.android.presentation.common.MyReviewBottomSheetFragment
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.launch

@AndroidEntryPoint
class MyReviewListActivity :
BaseActivity<ActivityMyReviewListBinding>(ActivityMyReviewListBinding::inflate) {
BaseActivity<ActivityMyReviewListBinding>(ActivityMyReviewListBinding::inflate),
MyReviewBottomSheetFragment.OnReviewDeletedListener {

private val myReviewViewModel: MyReviewViewModel by viewModels()

Expand All @@ -28,10 +32,21 @@ class MyReviewListActivity :
}

private fun setAdapter(reviewList: List<Review>) {
val listAdapter = MyReviewAdapter(reviewList)

val adapter = MyReviewAdapter()
adapter.submitList(reviewList)

val linearLayoutManager = LinearLayoutManager(this)

binding.rvReview.adapter = listAdapter
adapter.setOnItemClickListener(object :
MyReviewAdapter.OnItemClickListener {

override fun onMyReviewClicked(view: View, reviewData: Review) {
onMyReviewClicked(review = reviewData)
}
})

binding.rvReview.adapter = adapter
binding.rvReview.layoutManager = linearLayoutManager
binding.rvReview.setHasFixedSize(true)
}
Expand Down Expand Up @@ -62,4 +77,29 @@ class MyReviewListActivity :
super.onResume()
lodeReview()
}

fun onMyReviewClicked(review: Review) {

val modalBottomSheet = MyReviewBottomSheetFragment().apply {
arguments = Bundle().apply {
putLong("reviewId", review.reviewId)
putString("menu", review.menu)
putString("content", review.content)
putInt("mainGrade", review.mainGrade)
putInt("amountGrade", review.amountGrade)
putInt("tasteGrade", review.tasteGrade)
}
onReviewDeletedListener = this@MyReviewListActivity
}
modalBottomSheet.setStyle(
DialogFragment.STYLE_NORMAL,
R.style.RoundCornerBottomSheetDialogTheme
)
modalBottomSheet.show(supportFragmentManager, "Open Bottom Sheet")
}

override fun onReviewDeleted() {
lodeReview()
}

}
Loading
Loading