정보수정제안 v1 MR #172

kymjaehong · 2023-12-17T14:39:55Z

#147 구현 및 메인 화면 코드 반영에 따라 MR 후 추가 작업하겠습니다.

PlaceEntity와 InfoEntity의 경/위도 타입을 제가 nullable하게 해서 지훈님 코드 쪽으로 수정했습니다.

이후에 다이얼로그 및 placeId를 지도로 부터 가져오는 작업하겠습니다.

height · 2023-12-17T14:39:58Z

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

easyhooon · 2023-12-17T16:01:57Z

다음부터는 리뷰어로 저 선택해주세여 그래야 메일 알림이 와서 알수가 있어가지공

easyhooon · 2023-12-17T16:14:46Z

지금 보니까 domain 모듈 model 패키지의 entity 의 parameter 들이 어떤건 public 이 붙어있고, 어떤건 안붙어있군요 다 없애버려도 될 것 같은데 이건 제가 하도록 하겠습니다

=> 이게 어느 부분인지 잘 모르겠는데 예시 하나만 긁어주실 수 있으신가요?

easyhooon · 2023-12-17T16:15:23Z

domain/src/main/kotlin/us/wedemy/eggeum/android/domain/model/place/ProductEntity.kt

@@ -8,6 +8,6 @@
 package us.wedemy.eggeum.android.domain.model.place

 public data class ProductEntity(
-  val name: String,
-  val price: Int,
+  var name: String,


entity Paramter 를 var 로 변경하신 이유가 있으신가요

[ProposeCafeInfoViewModel]에서

placeBody.menu?.products.let { productEntities -> productEntities?.forEach { if (it.name == before?.name && it.price == before.price) { it.name = after?.name!! it.price = after.price } } } 여기서 일단 이렇게 구현을 해버려서 그랬습니다. 여기도 수정해야겠네요

객체를 복사해서 변경 하는 .copy() 같은 함수 사용하면 var로 선언하지 않아도 값 자체를 변경할 수 있습니다. uiState.update { it.copy() 이런식으로 } 내부 값을 변경하는 함수가 main 모듈의 UserInfoViewModel 같은 곳에 사용되고 있는데 참고해보시면 좋을 것 같아요

다음에 반영해볼게요

easyhooon · 2023-12-17T16:18:17Z

update-cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/ui/InputCafeInfoFragment.kt

@@ -163,31 +154,43 @@ class InputCafeInfoFragment : BaseFragment<FragmentInputCafeInfoBinding>() {
  private fun initObserver() {
    repeatOnStarted {
      launch {
-        viewModel.uiState.collect { uiState ->
+        viewModel.cafeInfo.collect { cafeInfo ->
          with(binding) {


kotlin 에서 with 은 이미 초기화된 객체에 대한 연산을 수행할때 사용합니다. 그래서 이때는 객체의 초기화나 설정을 할때 사용하는 apply가 더 적절합니다 binding.apply { } 로 변경해주세여

easyhooon · 2023-12-17T16:23:43Z

update-cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/ui/item/CafeInfoItem.kt

+
+import us.wedemy.eggeum.android.domain.model.place.InfoEntity
+
+data class CafeInfoItem(


사실 아예 바뀌는 변수가 존재하지 않는다면 딱히 mapper 를 만들어주지 않고 InfoEntity 그대로 뷰에서 사용해도 되긴힙니다. 그런데 이건 취향 차이라 그대로 둬도 괜찮을것 같아요

그렇군요. 바뀌는 변수라고 하면, 필드 명인가요? 아니면 데이터 타입? 이런 걸 수도 있나요?

음 뭐 entity data class 의 포함된 파라미터를 전부다 사용할 필요가 없다던지, 가공이 필요하다던지, 말씀하신대로 데이터 타입이 변경될일이 있을땐 타입 캐스팅을 해줘야할 수도 있고, model 자체를 네비게이션의 argument 로 전달해야해서 @parcelize 같은 어노테이션을 붙여야되는데 이건 순수 코틀린 모듈(domain 모듈)인 경우엔 붙히지 못해서요 그래서 어쩔수없이 매퍼를 통해 android 모듈쪽 클래스로 매핑해주는 작업이 필요하더라구요

easyhooon · 2023-12-17T16:24:52Z

뒤에 붙은 PR title 뒤에 MR 무슨 의미인가요?

-> 아.. 깃랩이랑 착각했네요

easyhooon · 2023-12-18T00:19:32Z

domain/src/main/kotlin/us/wedemy/eggeum/android/domain/model/place/InfoEntity.kt

+  val websiteUri: String?,
+) {
+  public companion object {
+    public fun of(


해당 companion object 내에 함수(of) 가 잘 이해가 안되는데 어떤 역할을 하는 함수인가요?
mapper 의 역할이라면 기존의 방식과 동일하게 바꾸면 좋을것 같아요 (mapper 패키지내에 mapper 를 만들어주는)

viewModel에서 데이터 관리하면, 없어져야 할 부분이라 생각하는데
이렇게 많은 변수를 다뤄본 적이 없어서 그냥 빌더패턴으로 객체 만들어서 넘겼습니다.

easyhooon · 2023-12-18T00:22:08Z

update-cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/ui/InputCafeInfoFragment.kt

@@ -26,37 +31,194 @@ class InputCafeInfoFragment : BaseFragment<FragmentInputCafeInfoBinding>() {

  private val viewModel by viewModels<InputCafeInfoViewModel>()

+  private var cafeArea = ""


이런식으로 뷰에서 변수를 선언하게 되면 구현함에 있어 간단하긴한데 configuration change 가 발생하게 되면 모든 데이터가 다 날라가게 됩니다. 화면을 가로로 돌린다던지, 다크모드로 바꾼다던지, 글씨 크기를 바꾼다던지... 와 같은 상황에서 configuration change 가 발생하게 되어서, 뷰모델로 변수의 위치를 옮기면 config change 에서도 그 데이터들이 유지되는 장점이 있습니다

전에 말씀해주셨던 부분인데, 이번에 뷰모델에서 관리하는 방법으로 바꿔보겠습니다.

easyhooon · 2023-12-18T00:27:27Z

update-cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/ui/InputCafeInfoFragment.kt

+  private var cafePhone = ""
+
+  private var cafeName: String = ""
+  private val guideMessage = "정보를 입력해주세요!"


하드코딩된 문자열 같은 경우엔 string resource 로 빼내서 관리해주세여

easyhooon · 2023-12-18T00:28:48Z

...cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/viewmodel/InputCafeInfoViewModel.kt

+      when {
+        result.isSuccess && result.getOrNull() != null -> {
+          val placeBody = result.getOrNull()!!
+          Timber.d("plcaeBody >>> $placeBody")


정상적으로 값이 들어온 것이 확인 되었으면 확인용 로그를 지워주세여

easyhooon · 2023-12-18T00:39:15Z

update-cafe/src/main/kotlin/us/wedemy/eggeum/android/updatecafe/ui/SelectCafeMenuFragment.kt

-  private val viewModel by viewModels<SelectCafeMenuViewModel>()
+  private val viewModel by activityViewModels<ProposeCafeInfoViewModel>()
+
+  private var callCount = 0


마찬가지로 config change 발생하면 의미 없어지는 방식이라 그냥 갱신을 위해 화면이 호출될때마다 api를 호출되게 하는게 맘 편할 수도 있고 꼭 한번만 호출시켜야 겠다 생각들면 viewModel 내에 init 블럭내에서 해당 api를 호출하면 됩니다.

easyhooon · 2023-12-18T00:46:08Z

각 텍스트필드의 제목에 (회의실, 다인석...) 괄호 치고 단위를 표시하는 항목이 누락되어있습니다 넣어주세요

easyhooon · 2023-12-18T00:52:09Z

메뉴 수정 또는 카페 정보 수정이 완료된 이후 다음 버튼을 눌렀을 때 다시 처음 화면 (어떤 것을 개선하고 싶으세요?) 화면으로 이동하는 로직이 누락되어 있는데, 이번 또는 다음 피처에서 구현 부탁드려요

=> 아 아예 기존 main activity인 지도로 돌아가는 줄 알았습니다. 수정할게요

easyhooon · 2023-12-18T00:53:36Z

메뉴 수정시, 가격의 자릿수가 늘어날 경우 3자리씩 , 로 끊어주는 로직이 필요합니다. 구현하는데 어려움이 있다면 제가 나중에 할게여 예전에 만들어놨던 함수가 있어가지고

=> 넵, 감사합니다. 지훈님 코드 반영되면 공부하겠습니다.

…android into feature/update-cafe-place

easyhooon · 2023-12-18T13:09:46Z

피드백 반영 당장 하실 수 있는 것들 먼저 하시구, 다음 피처때(merge 한 이후에) 남은 피드백 반영하시겠다고 알려주시면 approve 누르겠습니다

=> 피드백 일부 반영했습니다. 다음 피처에서 아래 내용 추가해서 코드 수정할게요.

뷰모델에서 데이터 관리
string resource에서 반복 사용되는 문자열 가져오기 (반영하려했으나, 에러발생해서 주석)

easyhooon · 2023-12-18T13:57:17Z

=> 이게 어느 부분인지 잘 모르겠는데 예시 하나만 긁어주실 수 있으신가요?

// domain 모듈 Entity -> FileEntity
public data class FileEntity( 
  public val uploadFileId: Int, // <- 그냥 val 이라고 해도 default 가 public 이라 상관없을것 같아서요
  val url: String,
  val url: String?,
)

easyhooon · 2023-12-18T14:06:29Z

=> 아 아예 기존 main activity인 지도로 돌아가는 줄 알았습니다. 수정할게요

같은 로직이 main 모듈의 유저 정보화면내에 문의하기 화면(reportFragment)에 구현되어있는데 거기에 있는 navigation 로직 참고해서 똑같은 패턴으로 구현하면 될거에여

==> 이 부분 반영했습니다

easyhooon · 2023-12-18T14:49:40Z

update-cafe/src/main/res/navigation/nav_update_cafe.xml

@@ -122,7 +122,11 @@
    android:id="@+id/fragment_update_menu_complete"
    android:name="us.wedemy.eggeum.android.updatecafe.ui.UpdateMenuCompleteFragment"
    android:label="UpdateMenuCompleteFragment"
-    tools:layout="@layout/fragment_update_menu_complete" />
+    tools:layout="@layout/fragment_update_menu_complete" >
+    <action


이렇게만 하면 그냥 위에 fragment(화면)이 하나 쌓이는 꼴이라 이전 화면들이 스택에 남아서 시스템 뒤로가기 버튼등을 누르면 다시 바로 뒤 화면으로 돌아가요 어떻게 처리하는지 문의화면쪽에 있는 코드 확인해주세여

=> 제가 반영 해놨습니다

넵 감사합니다 저녁에 확인하겠습니다

app:popUpTo="@id/fragment_select_info_categories"
app:popUpToInclusive="true" />

이 부분이 추가된거군요

시스템 백버튼을 눌렀을 때 다시 이전에 수정하는 화면으로 이동되지 않도록

easyhooon · 2023-12-18T22:21:45Z

수고하셨어요!

근데 카페 정보를 수정 완료할 경우엔 "정보 수정을 완료 했어요."가 타이틀인 화면으로 이동하는게 맞는것 같군요. 피그마에 건의는 넣어놨습니다. 아마 화면이 추가될 수 도 있을 것 같아요

merge 하시면 됩니다!

=> 감사합니다 !

easyhooon

LGTM

kymjaehong added 12 commits November 2, 2023 23:33

add: 정보 수정 제안 v1 (#147)

ddce006

정보 수정 제안, 메뉴 read (#147)

061f169

메뉴 수정에서 우측 버튼 클릭 시, 선택 창(#147)

ed97da7

edit: imageview -> spinner (#147)

ced382c

popupmenu 구현 (#147)

48d68ef

Add: 수정하기, 삭제하기 버튼 클릭 구현 (#147)

e05e4bf

Add : 페이지 이동 및 다이얼로그 기본 구현 (#147)

0760300

Edit: 메뉴 삭제 화면 재로딩, 다음 수정 반영 (#147)

36bfe00

Edit: 삭제 부분 피드백 반영 & 수정 시, fragment 데이터 전달(#147)

3857ff1

정보수정제안 api 구현 #147

0cf856a

메뉴 수정 시, 초기 데이터 초기화 (#147)

b17a9f0

edit: manifest && style check

4bc4441

easyhooon assigned kymjaehong Dec 17, 2023

easyhooon self-requested a review December 17, 2023 16:01

easyhooon added enhancement New feature or request design tasks related to design labels Dec 17, 2023

easyhooon reviewed Dec 17, 2023

View reviewed changes

easyhooon reviewed Dec 18, 2023

View reviewed changes

누락된 navigation animation 추가 및 개행

8bd073a

easyhooon marked this pull request as draft December 18, 2023 01:03

kymjaehong added 2 commits December 18, 2023 21:59

Edit: 피드백 일부 반영 #147

1abef05

Merge branch 'feature/update-cafe-place' of github.com:Wedemy/eggeum-…

3ac264e

…android into feature/update-cafe-place

easyhooon reviewed Dec 18, 2023

View reviewed changes

Edit: 메뉴 수정 완료 화면에서 정보 수정 제안 화면으로 돌아가는 경우 사이에 쌓여 있는 백스택 제거

1d4f0ec

시스템 백버튼을 눌렀을 때 다시 이전에 수정하는 화면으로 이동되지 않도록

easyhooon marked this pull request as ready for review December 18, 2023 22:21

easyhooon approved these changes Dec 18, 2023

View reviewed changes

kymjaehong merged commit 0f60eb4 into develop Dec 19, 2023
1 check passed

kymjaehong deleted the feature/update-cafe-place branch December 19, 2023 00:20


		import us.wedemy.eggeum.android.domain.model.place.InfoEntity

		data class CafeInfoItem(

		@@ -26,37 +31,194 @@ class InputCafeInfoFragment : BaseFragment<FragmentInputCafeInfoBinding>() {

		private val viewModel by viewModels<InputCafeInfoViewModel>()

		private var cafeArea = ""

정보수정제안 v1 MR #172

정보수정제안 v1 MR #172

Conversation

kymjaehong commented Dec 17, 2023 • edited by easyhooon Loading

height bot commented Dec 17, 2023

easyhooon commented Dec 17, 2023

easyhooon commented Dec 17, 2023 • edited by kymjaehong Loading

Choose a reason for hiding this comment

kymjaehong Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easyhooon commented Dec 17, 2023 • edited by kymjaehong Loading

easyhooon Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easyhooon Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easyhooon commented Dec 18, 2023 • edited Loading

easyhooon commented Dec 18, 2023 • edited by kymjaehong Loading

easyhooon commented Dec 18, 2023 • edited by kymjaehong Loading

easyhooon commented Dec 18, 2023 • edited by kymjaehong Loading

easyhooon commented Dec 18, 2023

easyhooon commented Dec 18, 2023 • edited by kymjaehong Loading

easyhooon Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easyhooon commented Dec 18, 2023 • edited by kymjaehong Loading

easyhooon left a comment

Choose a reason for hiding this comment

kymjaehong commented Dec 17, 2023 •

edited by easyhooon

Loading

easyhooon commented Dec 17, 2023 •

edited by kymjaehong

Loading

kymjaehong Dec 18, 2023 •

edited

Loading

easyhooon commented Dec 17, 2023 •

edited by kymjaehong

Loading

easyhooon Dec 18, 2023 •

edited

Loading

easyhooon Dec 18, 2023 •

edited

Loading

easyhooon commented Dec 18, 2023 •

edited

Loading

easyhooon commented Dec 18, 2023 •

edited by kymjaehong

Loading

easyhooon commented Dec 18, 2023 •

edited by kymjaehong

Loading

easyhooon commented Dec 18, 2023 •

edited by kymjaehong

Loading

easyhooon commented Dec 18, 2023 •

edited by kymjaehong

Loading

easyhooon Dec 18, 2023 •

edited

Loading

easyhooon commented Dec 18, 2023 •

edited by kymjaehong

Loading