refactor: 토글 안되는거 해결 #445

hozzijeong · 2023-10-18T09:07:15Z

🔥 연관 이슈

close 마이페이지 토글 문제를 해결한다 #441

🚀 작업 내용

문제의 원인은 서버에서 반환하는 객체의 프로퍼티와 클라이언트에서 설정한 프로퍼티가 달랐기 때문입니다. JSP에서 is 접두어를 사용하면 자동으로 없애서 반환한다고 하더라구요 하하

마이페이지에서 Toggle 컴포넌트를 분리했습니다.
낙관적 업데이트를 좀 더 잘 적용해 봤습니다.
앱 푸시에 사용되는 토큰을 알림을 해제한다면 삭제하고, 구독한다면 새로 등록하는 방식으로 적용했습니다.

💬 리뷰 중점사항

웹 푸시에 사용되는 토큰을 전역 클래스로 해서 저장을 했습니다. 그 이유는 구독 on/off 할 때 토큰을 생성, 삭제하는데 생성하는 과정에서 매번 token을 요청하면 이게 시간이 좀 걸려서 바로 업데이트가 되지 않는 이슈가 있습니다 그래서 처음에 어플을 실행할 때 토큰을 받고, 삭제 요청이 성공했을 때 다시 토큰을 받는 식으로 처리했습니다.

WaiNaat

안녕하세요 클린!!!!!!!
낙관적 업데이트 재밌네요ㅋㅋ 뽑기하는맛이 있습니다

WaiNaat · 2023-10-19T00:15:45Z

frontend/src/hooks/queries/auth/useWebPush.ts

-    onSuccess: () => {
-      addToast({ type: 'success', message: '알림을 해제했습니다', time: 3000 });
+    onSuccess: async () => {
+      console.log('설마 성공?');


WaiNaat · 2023-10-19T00:17:55Z

frontend/src/utils/firebase.ts

+  apiKey: 'AIzaSyAOEUhyDZ1FQ2Ly77t-TNEqzb-686teUKU',
+  authDomain: 'pium-7ddfe.firebaseapp.com',
+  projectId: 'pium-7ddfe',
+  storageBucket: 'pium-7ddfe.appspot.com',
+  messagingSenderId: '66938335591',
+  appId: '1:66938335591:web:88ebf4f7f9dba08031ffc2',
+  measurementId: 'G-8SL2D547VW',


C

이건 언제 바뀌는 건가요??

firebase를 새로 세팅해서 바꿨습니다!

WaiNaat · 2023-10-19T00:18:24Z

frontend/src/utils/firebase.ts


-export { analytics, messaging, getCurrentToken };
+onMessage(messaging, (payload) => {
+  console.log(payload, 'this is payload');


foreground인 경우에도 알림을 받기위해서 작성해 둔 것입니다 새롭게 반영했어요11

WaiNaat · 2023-10-19T00:23:02Z

frontend/public/firebase-messaging-sw.js

+const firebaseConfig = {
+  apiKey: 'AIzaSyAOEUhyDZ1FQ2Ly77t-TNEqzb-686teUKU',
+  authDomain: 'pium-7ddfe.firebaseapp.com',
+  projectId: 'pium-7ddfe',
+  storageBucket: 'pium-7ddfe.appspot.com',
+  messagingSenderId: '66938335591',
+  appId: '1:66938335591:web:88ebf4f7f9dba08031ffc2',
+  measurementId: 'G-8SL2D547VW',
+};


C

이 값을 여기랑 utils/firebase에서 같이 쓸 수 있는 방법이 있을까요?
밖으로 빼자니 얘는 번들이 아니라 안될 것 같고 이 친구가 이걸 export하자니 안 어울린다는 생각이 드네요 😢

저도 중복되는 값이라 같이 쓰고 싶긴 한데 파일 구조상 쉽지 않네요 ㅠ 애초에 serviceWorker라는 객체? 기능이 src 내부에 있는 파일과는 완전히 무관하다고 생각해서 config 정보의 경우에는 서로 따로 두는게 낫다고 생각합니다. firebase 메세지 수신관련 문서에서도 serviceWorker 파일에 같은 형태로 Config 값을 설정하도록 되어있습니다!

WaiNaat · 2023-10-19T00:24:15Z

frontend/src/hooks/queries/auth/useWebPush.ts

@@ -1,13 +1,16 @@
 import { useMutation, useQueryClient, useSuspenseQuery } from '@tanstack/react-query';
 import useAddToast from 'hooks/@common/useAddToast';
+import PushStatus from 'models/PushStatus';


C

export는 camelCase인데 import는 PascalCase로 하신 이유가 있으신가요??

결국에 static으로 사용하는 클래스의 형태라고 생각해서 import 대문자가 맞다고 생각했습니다. 근데 export할 때는 인스턴스를 반환하도록 해서 소문자가 되버리는 어색한 형태가 되었네요... 한 가지로 통일하겠습니다.

WaiNaat · 2023-10-19T00:36:25Z

frontend/src/hooks/queries/auth/useWebPush.ts

 }

+// 지금 tanskack query에서 error를 잡지 않고 무조건 success를 돌려버림... 그 이유ㄴ느 뭔지 모르겟다 정말;;


R

tanstack query에는 useQueries에서 에러 타입 추론이 안 되는 문제 말고는 다른 에러 관련 문제는 없네요

찾아보니 원인은 바로 throwOnInvalidStatus에서 400 코드를 처리하지 않았기 때문입니다!!!!

아! 이 문제는 해결했습니다... (하하) 세심한 리뷰 감사합니다 👍

저 문제가 발생한게 저도 언제 적은건지 모르겠지만 thrownOnInvalidStatus 메서드가 async로 되어있어서 에러를 throw하면 tanstack-query의 catch가 아니라 한 단계 더 생기는 것 같더라구요

근데 async-await를 사용했을 때 가장 가까운 catch 문에서 에러를 잡는다고 생각을 했는데 그게 아니어서 좀 신기한긴 했습니다. 자세한 이유는 나중에 한 번 알아보면 좋을 것 같아요!

WaiNaat · 2023-10-19T00:40:57Z

frontend/src/mocks/handlers.ts

      const { token } = await req.json();

-      if (Math.random() < failRate) {
-        return res(ctx.delay(delay), ctx.status(407));
+      if (Math.random() < 0.5) {


A

이런 방법은 사실 테스트로서는 적합하지 않아서 나중에 tanstack query devtools를 도입해보면 좋겠네요 👍

그게 맞긴합니다... 일단은 확률을 좀 낮추는 방식으로 수정할게요..!

WaiNaat · 2023-10-19T00:43:06Z

frontend/src/hooks/@common/usePushAlert.ts

+    pushSupport: PushStatus.getIsSupport(), // 푸시 서비스를 지원하는지 여부
+    notificationDenied: PushStatus.getPermission(), // 알림 구독 여부


C

이 친구들은 이 훅에서 제공하는 상태도, 계산해주는 값도 아닌데 여기서 반환하는 이유가 있나요???

굳이 여기서 할 필요는 없네요! 수정하겠습니다

WaiNaat · 2023-10-19T00:51:33Z

frontend/src/models/PushStatus.ts

+  }
+
+  getPermission() {
+    if (!this.pushStatus.notificationPermission) throw new Error('알림 상태를 알 수 없습니다');


R

사이트 접속 시 처음부터 브라우저 차원에서 알림을 막았다면 맛이 갑니다.. (brave 시크릿모드에서 이 현상을 봤어요)

이건 저희쪽에서 try catch로 해결 가능한 문제인진 모르겠네요

이 부분은 어딘가에서 try catch를 쓰거나 메서드 자체가 boolean을 반환하는 것이니 false를 반환해도 좋을듯해요

이거도 PushAlert 컴포넌트가 disable된 것 같은 ui로 바꾸는 게 좋아보여요. 제가 당해보니까 무슨 에러인지도 모르겠고 저희 마이페이지에 어떤 기능이 있는지 모르는 사용자들은 마이페이지 전체가 망가졌다는 생각을 할 것 같습니다

이 부분은 서비스 워커가 등록될 때 token을 저장하는데, 알림을 허용하지 않은 상태에서 토큰을 받다보니 발생하는 에러같습니다! 한번 수정해볼게요!

bassyu

확인했습니다~!

구독 요청 실패는 바로 토글이 돌아오는데, 구독취소 요청 실패는 토글이 나중에 돌아오네요.
혹시 의도된 동작이 아니라면 통일하는 건 어떤가요?

bassyu · 2023-10-19T01:53:28Z

frontend/src/models/PushStatus.ts

+
+const pushStatus = new PushStatus();
+
+export default pushStatus;


C

확실히 파일 자체는 클래스인데, export default가 인스턴스라서 헷갈릴 수 있겠어요
일반 export로 하는 건 어떤가요?

Suggested change

export default pushStatus;

export pushStatus;

import { pushStateus } from 'models/PushStatus';

class 인스턴스 하나만 반환하기 때문에 대문자로 반환하도록 하겠습니다...!

WaiNaat

멋있어요

WaiNaat · 2023-10-19T06:46:13Z

frontend/src/hooks/@common/useInstallApp.ts

@@ -23,13 +23,12 @@ const useInstallApp = () => {
  const [deferredPrompt, setDeferredPrompt] = useState<BeforeInstallPromptEvent | null>(null);

  const installApp = async () => {
-    if (deferredPrompt) {
-      await deferredPrompt.prompt();
+    if (!deferredPrompt) return;


bassyu

멋있어요

* refactor: webpush 토글 분리 * refactor: firebase config 수정 * refactor: deleteToken 추가 * test: 15%확률로 구독 해제,등록 실패 * refactor: subscript 낙관적 업데이트 적용 * refactor: pushmanager 조건 추가 및 currentToken 전역 객체 사용 * refactor: pushStatus Class 로 반환 * test: click에 force 추가 * test: type force:true로 설정 * test: 등록에 force 추가 * test: 에러 throw확률 5%로 변경 * refactor: permission이 허용되지 않았을 때 토큰 받지 못하도록 설정 * refactor: 의존성 없는 변수 분리 * chore: 불필요한 콘솔 제거 * feat: foreground인 경우에도 알림 받도록 설정

hozzijeong added 7 commits October 18, 2023 13:06

refactor: webpush 토글 분리

6aed915

refactor: firebase config 수정

3781864

refactor: deleteToken 추가

3f54354

test: 15%확률로 구독 해제,등록 실패

dcc86c3

refactor: subscript 낙관적 업데이트 적용

8f36c04

refactor: pushmanager 조건 추가 및 currentToken 전역 객체 사용

44f099e

refactor: pushStatus Class 로 반환

7e8d8df

hozzijeong added 🛠️ 리팩터링 리팩터링을 위한 이슈입니다 🍇 프론트엔드 프론트엔드 관련 이슈입니다 labels Oct 18, 2023

hozzijeong added this to the 6차 스프린트 milestone Oct 18, 2023

hozzijeong requested review from bassyu and WaiNaat October 18, 2023 09:07

hozzijeong self-assigned this Oct 18, 2023

hozzijeong linked an issue Oct 18, 2023 that may be closed by this pull request

마이페이지 토글 문제를 해결한다 #441

Closed

WaiNaat changed the title ~~토글 안되는거 해결~~ refactor: 토글 안되는거 해결 Oct 18, 2023

hozzijeong added 3 commits October 18, 2023 21:17

test: click에 force 추가

8bc29e2

test: type force:true로 설정

016683e

test: 등록에 force 추가

2550434

WaiNaat requested changes Oct 19, 2023

View reviewed changes

test: 에러 throw확률 5%로 변경

1ed4bfe

bassyu reviewed Oct 19, 2023

View reviewed changes

hozzijeong added 4 commits October 19, 2023 11:55

refactor: permission이 허용되지 않았을 때 토큰 받지 못하도록 설정

0f7246b

refactor: 의존성 없는 변수 분리

0b910f6

chore: 불필요한 콘솔 제거

7705a4f

feat: foreground인 경우에도 알림 받도록 설정

10ef92e

hozzijeong requested review from WaiNaat and bassyu October 19, 2023 04:58

WaiNaat approved these changes Oct 19, 2023

View reviewed changes

bassyu approved these changes Oct 19, 2023

View reviewed changes

bassyu merged commit ba08bb9 into develop Oct 19, 2023
2 checks passed

bassyu deleted the refactor/441-mypage_toggle branch October 19, 2023 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: 토글 안되는거 해결 #445

refactor: 토글 안되는거 해결 #445

hozzijeong commented Oct 18, 2023

WaiNaat left a comment

WaiNaat Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023 •

edited

Loading

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

WaiNaat Oct 19, 2023

hozzijeong Oct 19, 2023

bassyu left a comment

bassyu Oct 19, 2023 •

edited

Loading

hozzijeong Oct 19, 2023

WaiNaat left a comment

WaiNaat Oct 19, 2023

bassyu left a comment

		}

		// 지금 tanskack query에서 error를 잡지 않고 무조건 success를 돌려버림... 그 이유ㄴ느 뭔지 모르겟다 정말;;

		pushSupport: PushStatus.getIsSupport(), // 푸시 서비스를 지원하는지 여부
		notificationDenied: PushStatus.getPermission(), // 알림 구독 여부


		const pushStatus = new PushStatus();

		export default pushStatus;

refactor: 토글 안되는거 해결 #445

refactor: 토글 안되는거 해결 #445

Conversation

hozzijeong commented Oct 18, 2023

🔥 연관 이슈

🚀 작업 내용

💬 리뷰 중점사항

WaiNaat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R

Choose a reason for hiding this comment

WaiNaat Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

A

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R

Choose a reason for hiding this comment

bassyu left a comment

Choose a reason for hiding this comment

bassyu Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

WaiNaat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bassyu left a comment

Choose a reason for hiding this comment

WaiNaat Oct 19, 2023 •

edited

Loading

bassyu Oct 19, 2023 •

edited

Loading