Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Chore] Change delete/modify/report modal #248

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
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
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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

속성 유효성 검사 추가 필요

리뷰 데이터의 유효성 검사가 누락되어 있습니다:

  • 평점(mainGrade, amountGrade, tasteGrade)의 범위 검증 필요 (0-5)
  • reviewId의 유효성 검사 추가 필요
-    var mainGrade = -1
-    var amountGrade = -1
-    var tasteGrade = -1
+    var mainGrade = -1
+        set(value) {
+            require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
+            field = value
+        }
+    var amountGrade = -1
+        set(value) {
+            require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
+            field = value
+        }
+    var tasteGrade = -1
+        set(value) {
+            require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
+            field = value
+        }
📝 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.

Suggested change
var reviewId = -1L
var menu = ""
var content = ""
var mainGrade = -1
var amountGrade = -1
var tasteGrade = -1
var reviewId = -1L
var menu = ""
var content = ""
var mainGrade = -1
set(value) {
require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
field = value
}
var amountGrade = -1
set(value) {
require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
field = value
}
var tasteGrade = -1
set(value) {
require(value in -1..5) { "평점은 -1에서 5 사이여야 합니다" }
field = value
}


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