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.

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

通知が1回のみ表示されるようにした #107

Merged
merged 4 commits into from
Oct 31, 2024
Merged

通知が1回のみ表示されるようにした #107

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf74382
Remove login & logout success notification
kimurash Oct 29, 2024
c05bc76
Show notification only once
kimurash Oct 29, 2024
5c6ed03
Remove FIXME comment
kimurash Oct 31, 2024
892b134
Revalidate after show notification
kimurash Oct 31, 2024
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 frontend/app/components/header/HeaderLoginComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const HeaderLoginComponent = () => {
setUser(undefined);
// モーダルを閉じる
close();
// home.tsx の action を呼び出す
// ログアウトのactionを呼び出す
submit({}, { method: 'DELETE', action: '/home' });
};

Expand Down
2 changes: 1 addition & 1 deletion frontend/app/routes/_index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ export const meta: MetaFunction = () => {
};

export async function loader() {
return redirect('home');
return redirect('/home');
}
16 changes: 9 additions & 7 deletions frontend/app/routes/auth.login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {

// 未ログインの場合
// エラーメッセージを取得
const data = { error: session.get('loginError') };
const data = { error: session.get('error') };
Copy link
Contributor

Choose a reason for hiding this comment

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

sessionは一律errorにしたんか

Copy link
Member Author

Choose a reason for hiding this comment

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

異なるrouteが同じflashを読み出さないように、flashの種類を分けてたんだけど、エラーメッセージの増加に伴って管理コストが増大するので、1つに絞ることにした


return json<LoaderData>(data, {
headers: {
Expand All @@ -49,9 +49,11 @@ export const action = async ({ request }: ActionFunctionArgs) => {

// ログインに成功した場合
if (response.status === 200) {
session.flash('loginSuccess', 'ログインに成功しました');
session.set('userId', response.data.id.toString());
session.set('sessionToken', response.data.sessionToken!);
// FIXME: homeのloaderで読み出してもCookieが削除されない
Copy link
Contributor

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.

そのコメントは消しておかねば

session.flash('success', 'ログインに成功しました');

return redirect('/home/mypage', {
headers: {
'Set-Cookie': await commitSession(session),
Expand All @@ -63,20 +65,20 @@ export const action = async ({ request }: ActionFunctionArgs) => {
switch (response.status) {
case 400:
// prettier-ignore
session.flash('loginError', 'メールアドレスまたはパスワードが間違っています');
session.flash('error', 'メールアドレスまたはパスワードが間違っています');
break;
case 401:
// prettier-ignore
session.flash('loginError', 'メールアドレスまたはパスワードが間違っています');
session.flash('error', 'メールアドレスまたはパスワードが間違っています');
break;
case 404:
session.flash('loginError', 'ユーザーが見つかりません');
session.flash('error', 'ユーザーが見つかりません');
break;
case 500:
session.flash('loginError', 'サーバーエラーが発生しました');
session.flash('error', 'サーバーエラーが発生しました');
break;
default:
session.flash('loginError', 'エラーが発生しました');
session.flash('error', 'エラーが発生しました');
}

return redirect('/auth/login', {
Expand Down
53 changes: 11 additions & 42 deletions frontend/app/routes/home._index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import { GetBooksParams } from 'client/client.schemas';
import { useAtom } from 'jotai';
import { useEffect } from 'react';
import BookListComponent from '~/components/books/BookListComponent';
import { commitSession, getSession } from '~/services/session.server';
import { selectedBooksAtom } from '~/stores/cartAtom';
import { errorNotification, successNotification } from '~/utils/notification';

interface LoaderData {
booksResponse: getBooksResponse;
Expand All @@ -22,10 +20,6 @@ interface LoaderData {
page?: string;
limit?: string;
};
flash: {
success?: string;
error?: string;
};
}

export const loader = async ({ request }: LoaderFunctionArgs) => {
Expand All @@ -47,40 +41,22 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
limit: limit,
});

const session = await getSession(request.headers.get('Cookie'));

// flashメッセージを取得する
const success = session.get('deleteBookSuccess');
const error = session.get('deleteBookError');

return json<LoaderData>(
{
booksResponse: response,
condition: {
title: title,
author: auther,
publisher: publisher,
isbn: isbn,
page: page,
limit: limit,
},
flash: {
success: success,
error: success ? undefined : error,
},
},
{
headers: {
'Set-Cookie': await commitSession(session),
},
return json<LoaderData>({
booksResponse: response,
condition: {
title: title,
author: auther,
publisher: publisher,
isbn: isbn,
page: page,
limit: limit,
},
);
});
};

const BooKListPage = () => {
const { booksResponse, condition, flash } = useLoaderData<typeof loader>();
const { booksResponse, condition } = useLoaderData<typeof loader>();
const { title, author, publisher, isbn, page, limit } = condition;
const { success, error } = flash;

const [opened, { open, close }] = useDisclosure();
const navigate = useNavigate();
Expand All @@ -96,13 +72,6 @@ const BooKListPage = () => {
});

useEffect(() => {
// flashメッセージを表示する
if (success) {
successNotification(success);
} else if (error) {
errorNotification(error);
}

// 選択中の書籍をリセットする
setSelectedBook([]);
}, []);
Expand Down
10 changes: 5 additions & 5 deletions frontend/app/routes/home.books.$bookId.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const action = async ({ request }: ActionFunctionArgs) => {

// 未ログインの場合
if (!session.has('userId')) {
session.flash('loginError', 'ログインしてください');
session.flash('error', 'ログインしてください');
return redirect('/auth/login', {
headers: {
'Set-Cookie': await commitSession(session),
Expand All @@ -80,28 +80,28 @@ export const action = async ({ request }: ActionFunctionArgs) => {
});
switch (response.status) {
case 204:
session.flash('deleteBookSuccess', '削除しました');
session.flash('success', '削除しました');
return redirect('/home', {
headers: {
'Set-Cookie': await commitSession(session),
},
});
case 401:
session.flash('loginError', 'ログインしてください');
session.flash('error', 'ログインしてください');
return redirect('/auth/login', {
headers: {
'Set-Cookie': await commitSession(session),
},
});
case 404:
session.flash('deleteBookError', '書籍が見つかりませんでした');
session.flash('error', '書籍が見つかりませんでした');
return redirect('/home', {
headers: {
'Set-Cookie': await commitSession(session),
},
});
case 500:
session.flash('deleteBookError', 'サーバーエラーが発生しました');
session.flash('error', 'サーバーエラーが発生しました');
return json<ActionResponse>(
{ method: 'DELETE', status: 500 },
{
Expand Down
27 changes: 3 additions & 24 deletions frontend/app/routes/home.mypage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import { json, LoaderFunctionArgs, redirect } from '@remix-run/cloudflare';
import { useLoaderData } from '@remix-run/react';
import { useEffect } from 'react';
import { commitSession, getSession } from '~/services/session.server';
import { successNotification } from '~/utils/notification';

interface LoaderData {
success?: string;
}
import { LoaderFunctionArgs, redirect } from '@remix-run/cloudflare';
import { getSession } from '~/services/session.server';

export const loader = async ({ request }: LoaderFunctionArgs) => {
const session = await getSession(request.headers.get('Cookie'));
Expand All @@ -17,24 +10,10 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
return redirect('/auth/login');
}

const data = { success: session.get('loginSuccess') };

return json<LoaderData>(data, {
headers: {
'Set-Cookie': await commitSession(session),
},
});
return null;
};

const MyPage = () => {
const { success } = useLoaderData<typeof loader>();

useEffect(() => {
if (success) {
successNotification(success);
}
}, []);

return <div>MyPage</div>;
};

Expand Down
31 changes: 20 additions & 11 deletions frontend/app/routes/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
const session = await getSession(request.headers.get('Cookie'));

const userId = session.get('userId');
const success = session.get('logoutSuccess');
const error = session.get('logoutError');
const success = session.get('success');
const error = session.get('error');

return json<LoaderData>(
{
userId: userId,
success: success,
error: success ? undefined : error,
error: error,
},
{
headers: {
Expand All @@ -54,14 +54,16 @@ export const action = async ({ request }: ActionFunctionArgs) => {
if (response.status === 204) {
session.unset('userId');
session.unset('sessionToken');
session.flash('logoutSuccess', 'ログアウトに成功しました');
session.flash('success', 'ログアウトに成功しました');

return redirect('/home', {
headers: {
'Set-Cookie': await commitSession(session),
},
});
} else {
session.flash('logoutError', 'ログアウトに失敗しました');
session.flash('error', 'ログアウトに失敗しました');

return redirect('/home', {
headers: {
'Set-Cookie': await commitSession(session),
Expand Down Expand Up @@ -92,13 +94,20 @@ const Home = () => {
} else {
setUser(undefined);
}
if (success) {
successNotification(success);
} else if (error) {
errorNotification(error);
}
}
}, [navigation.state]);
}, [userId]);

useEffect(() => {
if (success) {
successNotification(success);
}
}, [success]);

useEffect(() => {
if (error) {
errorNotification(error);
}
}, [error]);
Copy link
Contributor

Choose a reason for hiding this comment

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

結局このやり方にしたんか
このやり方やと、同じエラーが連発した時に通知が一回しか出ない問題があったけど解決したん？
flashで入れてるから一回undefinedに変換されて良い感じにuseEffectが呼ばれるようになるとか

Copy link
Member Author

Choose a reason for hiding this comment

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

ログイン画面ではerrorではなくnavigation.stateを依存関係にしているので、エラーが発生した回数だけ通知が表示される

Copy link
Contributor

@Kosei805 Kosei805 Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

他の画面ではおなじerrorは連続で呼ばれないだろってイメージ？

Copy link
Member Author

Choose a reason for hiding this comment

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

なるほど、ログイン画面以外で同じエラーが連続して発生したときにちゃんと通知がでるんか？ってことだね

通知がでてから一度loaderが走れば、errorがundefinedになるので、同じエラーでも通知が出てくれると思う

手元の環境で試してみる。エラーは再現が難しいので、本の削除を2回繰り返したときに、成功の通知が2回とも出るか？を確かめてみる。

Copy link
Member Author

@kimurash kimurash Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

2回は出なかった

Copy link
Member Author

Choose a reason for hiding this comment

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

useRevalidator を使ってsuccessとerrorの値を再検証することで解決した

https://remix.run/docs/en/main/hooks/use-revalidator

Copy link
Contributor

Choose a reason for hiding this comment

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

ならマージして良いよ！


return (
<AppShell header={{ height: 70 }} padding={{ default: 'md', sm: 'sm' }}>
Expand Down
11 changes: 2 additions & 9 deletions frontend/app/services/session.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,8 @@ interface SessionData {
}

interface SessionFlashData {
// ログイン
loginSuccess: string;
loginError: string;
// ログアウト
logoutSuccess: string;
logoutError: string;
// 書籍の削除
deleteBookSuccess: string;
deleteBookError: string;
success?: string;
error?: string;
}

const { getSession, commitSession, destroySession } =
Expand Down
Loading