chore: update cash refresh logic - #69
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aa9c306a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const loginUserInfoQuery = $api.useQuery('get', '/api/session', { | ||
| credentials: 'include', | ||
| onError: () => { | ||
| toast.error('ユーザ名の取得に失敗しました。', { id: 'root-username' }); | ||
| }, |
There was a problem hiding this comment.
Pass onError via query options
Here onError is being passed in the init argument to $api.useQuery, but openapi-react-query expects init (fetch options) and React Query options as separate arguments. As a result, the callback is never registered and session failures on /api/session will be silent (no toast), even though the UI still depends on that request. This should be useQuery('get', '/api/session', { credentials: 'include' }, { onError: ... }) so error handling actually runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR refactors parts of the frontend to rely more on React Query’s query cache (instead of imperative “refetch via mutation”) and adjusts several UI labels/components related to sharing and profile pages.
Changes:
- Refactor
GroupDetailto fetch group info/debt history via queries and refresh debt history via cache invalidation after register/delete/cancel. - Simplify
InviteButtonprops by removingsetInviteUrlusage and update call sites accordingly. - Update profile/account UI wording and introduce a
SubTitlecomponent for section headings.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| products/frontend/src/share/InviteButton/index.tsx | Removes setInviteUrl from the component API and keeps copy logic focused on clipboard + status. |
| products/frontend/src/routes/Share/components/Header/index.tsx | Simplifies identicon rendering condition in header navigation. |
| products/frontend/src/routes/Root/index.tsx | Replaces session “mutation + effect” with a session query and uses query data for the welcome message. |
| products/frontend/src/routes/Root/components/Menu/index.tsx | Updates menu link labels. |
| products/frontend/src/routes/Profile/index.tsx | Updates page title text and adds a subtitle section heading. |
| products/frontend/src/routes/Profile/components/index.ts | Exposes the new SubTitle component from the Profile components barrel. |
| products/frontend/src/routes/Profile/components/SubTitle/index.tsx | Adds a new subtitle heading component for Profile sections. |
| products/frontend/src/routes/Profile/components/SubTitle/index.module.css | Adds styling for the new Profile subtitle. |
| products/frontend/src/routes/Profile/components/SocialLink/index.tsx | Switches section heading rendering to use SubTitle. |
| products/frontend/src/routes/Profile/components/SocialLink/index.module.css | Removes now-unused .title styling after heading change. |
| products/frontend/src/routes/GroupDetail/index.tsx | Moves group/debt fetching to queries and refreshes debt history via cache invalidation. |
| products/frontend/src/routes/GenerateGroup/index.tsx | Updates InviteButton usage to match the simplified props. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
|
|
||
| const SubTitle: FC<Props> = (props: Props) => { | ||
| return <h1 className={styles.subTitle}>{props.subTitle}</h1>; |
There was a problem hiding this comment.
This SubTitle component renders an <h1>, but pages already render a top-level <h1> via the shared Title component. Using multiple <h1> headings on the same page hurts document semantics and screen-reader navigation. Consider rendering this as an <h2>/<h3> (or supporting an as prop) and updating usages accordingly.
| return <h1 className={styles.subTitle}>{props.subTitle}</h1>; | |
| return <h2 className={styles.subTitle}>{props.subTitle}</h2>; |
| import { type FC } from 'react'; | ||
| // css | ||
| import styles from './index.module.css'; | ||
|
|
||
| type Props = { | ||
| subTitle: string; | ||
| }; | ||
|
|
||
| const SubTitle: FC<Props> = (props: Props) => { | ||
| return <h1 className={styles.subTitle}>{props.subTitle}</h1>; | ||
| }; | ||
|
|
There was a problem hiding this comment.
There is already a shared SubTitle component at products/frontend/src/share/SubTitle. Adding another SubTitle under Profile duplicates API and styling and will make imports ambiguous over time. Prefer reusing the shared component (and extending it via props/className if you need spacing differences) rather than introducing a route-local duplicate.
| import { type FC } from 'react'; | |
| // css | |
| import styles from './index.module.css'; | |
| type Props = { | |
| subTitle: string; | |
| }; | |
| const SubTitle: FC<Props> = (props: Props) => { | |
| return <h1 className={styles.subTitle}>{props.subTitle}</h1>; | |
| }; | |
| import SubTitle from '../../../../share/SubTitle'; |
| debtHistoryMutationIsPending={debtHistoryQuery.isPending} | ||
| debtHistoryMutationIsError={debtHistoryQuery.isError} | ||
| debtHistoryMutationIsSuccess={debtHistoryQuery.isSuccess} | ||
| debtHistoryResult={(debtHistoryQuery.data ?? null) as GetGroupDebtHistoryResponseSchemaType | null} |
There was a problem hiding this comment.
Avoid the as GetGroupDebtHistoryResponseSchemaType | null type assertion here. It bypasses type-checking and can mask API/typing mismatches. Prefer passing debtHistoryQuery.data ?? null directly and aligning History's prop type with the actual $api.useQuery response type (or derive the type from the query result).
| debtHistoryResult={(debtHistoryQuery.data ?? null) as GetGroupDebtHistoryResponseSchemaType | null} | |
| debtHistoryResult={debtHistoryQuery.data ?? null} |
| import { FaDiscord, FaGoogle } from 'react-icons/fa6'; | ||
| // css | ||
| import styles from './index.module.css'; | ||
| import { SubTitle } from '..'; |
There was a problem hiding this comment.
import { SubTitle } from '..' creates a circular dependency because components/index.ts also re-exports SocialLink from this module. This can lead to brittle module init order / runtime undefined issues depending on the bundler. Import SubTitle directly from its module (e.g., ../SubTitle) instead of via the barrel.
| import { SubTitle } from '..'; | |
| import { SubTitle } from '../SubTitle'; |
No description provided.