Skip to content

[Fix] 알림 권한 미허용 시 재요청 로직 추가 #33

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.

Sign up for GitHub

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

Merged
kongwoojin merged 1 commit into from
Feb 20, 2026
Merged

[Fix] 알림 권한 미허용 시 재요청 로직 추가 #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 25 additions & 8 deletions app/_layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Stack, useRouter } from 'expo-router';
import { useState, useEffect } from 'react';
import { AppState } from 'react-native';
import { getForceUpdate, appVersion, versionToNumber } from '../services/forceupdate';
import * as Notifications from 'expo-notifications';
import { registerForPushNotificationsAsync } from '../services/notifications';
import { registerForPushNotificationsAsync, shouldRecheckPermission } from '../services/notifications';
import CookieManager from '@react-native-cookies/cookies';

Notifications.setNotificationHandler({
Expand Down Expand Up @@ -32,17 +33,33 @@ export default function RootLayout() {
}, []);

useEffect(() => {
if (!isReady) return;
const handleToken = (token?: string) => {
if (token) {
addTokenToCookie(token);
console.log('Expo Push Token:', token);
}
};

registerForPushNotificationsAsync()
.then((token) => {
if (token) {
addTokenToCookie(token);
console.log('Expo Push Token:', token);
}
})
.then(handleToken)
.catch((error: any) => console.error(error));

const subscription = AppState.addEventListener('change', (nextAppState) => {
if (nextAppState === 'active' && shouldRecheckPermission()) {
registerForPushNotificationsAsync()
.then(handleToken)
.catch((error: any) => console.error(error));
Comment on lines +48 to +51
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

The function shouldRecheckPermission() is stateful and consumes the flag when called. However, it's called inline in the AppState event handler condition. If this condition is evaluated multiple times during the same render cycle or if multiple listeners exist, this could lead to the flag being consumed unexpectedly. Consider checking the flag and storing the result in a variable before using it in the condition, or refactoring to make the state management more explicit.

Suggested change
if (nextAppState === 'active' && shouldRecheckPermission()) {
registerForPushNotificationsAsync()
.then(handleToken)
.catch((error: any) => console.error(error));
if (nextAppState === 'active') {
const shouldRecheck = shouldRecheckPermission();
if (shouldRecheck) {
registerForPushNotificationsAsync()
.then(handleToken)
.catch((error: any) => console.error(error));
}

Copilot uses AI. Check for mistakes.
}
});
Comment on lines +47 to +53
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

The AppState listener is set up immediately when the component mounts, but the initial permission check also runs immediately. This creates a potential race condition where both the initial registration and the AppState 'active' event could fire simultaneously, especially if the app regains focus during initialization. This could lead to duplicate permission requests or alert dialogs.

Copilot uses AI. Check for mistakes.

return () => {
subscription.remove();
};
}, []);

useEffect(() => {
if (!isReady) return;

const response = Notifications.getLastNotificationResponse();
if (response?.notification) {
const data = response.notification.request.content.data.path;
Expand Down
31 changes: 29 additions & 2 deletions services/notifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
import { Platform } from 'react-native';
import { Alert, Linking, Platform } from 'react-native';
import * as Device from 'expo-device';
import * as Notifications from 'expo-notifications';
import Constants from 'expo-constants';
import { exitApp } from '@logicwind/react-native-exit-app';

let wentToSettings = false;
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

The module-level state variable wentToSettings creates a potential race condition. If notificationPermissionAlert is triggered multiple times before the user acts, or if the user navigates to settings but the app state changes multiple times, the flag could be consumed by an unintended AppState change event. This can lead to unexpected behavior where permission checks are skipped or performed at incorrect times.

Copilot uses AI. Check for mistakes.

const notificationPermissionAlert = () =>
Alert.alert('알림 권한이 필요해요', '설정으로 이동해서 알림 권한을 허용해주세요.', [
{
text: '앱 종료',
onPress: () => exitApp(),
style: 'cancel',
},
{
text: '확인',
onPress: () => {
wentToSettings = true;
Linking.openSettings();
},
},
]);
Comment on lines +18 to +23
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

When the user clicks "확인" (Confirm) to go to settings, the flag wentToSettings is set to true before opening settings. However, if the user doesn't actually grant permissions in settings and returns to the app, the alert will not be shown again because the flag was already consumed by shouldRecheckPermission(). This means the user is stuck without notifications and no way to re-enable them except by reinstalling the app. Consider checking the actual permission status after returning from settings rather than just relying on the flag.

Copilot uses AI. Check for mistakes.

function handleRegistrationError(errorMessage: string) {
console.error(errorMessage);
Expand All @@ -24,7 +43,7 @@ export async function registerForPushNotificationsAsync() {
finalStatus = status;
}
if (finalStatus !== 'granted') {
handleRegistrationError('Permission not granted to get push token for push notification!');
notificationPermissionAlert();
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

The alert could potentially be shown multiple times if registerForPushNotificationsAsync is called concurrently (e.g., initial call and AppState change happening simultaneously). This creates a poor user experience with duplicate alert dialogs. Consider adding a guard to prevent showing the alert when one is already active.

Copilot uses AI. Check for mistakes.
return;
}
const projectId =
Expand All @@ -47,3 +66,11 @@ export async function registerForPushNotificationsAsync() {
handleRegistrationError('Must use physical device for push notifications');
}
}

export function shouldRecheckPermission() {
if (wentToSettings) {
wentToSettings = false;
return true;
}
return false;
}