ユーザー画面の再提出機能の実装#2042
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
- isEdit の !isSubmission を isResubmission に修正(ロジックが逆だった) - onEdit も再提出待ち時に渡すよう修正 - AccordionMenu の isEdit に再提出待ち条件を追加 - isEditing の初期値を false に戻す(誤って true で初期化していた) - isFormListMode を isEmployeesData && !isEditing にシンプル化
- APIは { group, submissions: [...] } を返すが単一オブジェクトとして扱っていた
- Rails enumはJSON上で文字列("waiting_resubmission")で返るため数値変換マップを追加
- submissions配列からemployeeのエントリを探して返すよう修正
- APIエンドポイントのURLを修正し、末尾のスラッシュを削除 - healthCenterSubmissionStatusの型定義を明確化し、可読性を向上
- injectパッケージを削除 - coffee-scriptパッケージの削除に伴う関連エントリをpackage-lock.jsonから削除
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR implements a resubmission workflow for the user dashboard: it adds a health-center submission status API and APPLICATION_STATUS constants, introduces a ChangesResubmission Status Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
user/src/components/Applications/FireEquipment/components/hooks.ts (1)
139-188:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the resubmission status transition to the “火気不使用” submit path too.
The new
updateStatus()flow is only used fromonSubmitFireEquipment. If the user resubmits by changing this application to “火気不使用”,onSubmitUnregisteredsucceeds without clearingwaiting_resubmission, so the application can stay stuck in the resubmission state.Suggested fix
await mutateFireEquipmentOrder(); + if (status === 'waiting_resubmission') { + await updateStatus('unapproved'); + } toast.success('火気申請を行わない登録が完了しました'); handleEditCancel?.();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/components/Applications/FireEquipment/components/hooks.ts` around lines 139 - 188, The resubmission status transition is only applied in onSubmitFireEquipment, so when a user submits via the “火気不使用” path handled by onSubmitUnregistered the health-center status can remain waiting_resubmission; update onSubmitUnregistered to mirror onSubmitFireEquipment’s behavior by calling updateStatus('unapproved') after the successful submit (after post/patch and mutateFireEquipmentOrder/mutate equivalent and toast), guarded by if (status === 'waiting_resubmission'); use the existing updateStatus function to perform the patch and subsequent mutateHealthCenterSubmissionStatus.user/src/components/Applications/FoodProduct/hooks.ts (1)
153-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t treat a saved product update as a failed submit just because the status patch broke.
By the time
updateStatus()runs here, the deletes/upsert andmutateFoodProducts()have already completed. If the status request fails, the outercatchstill shows the generic update error for an operation that was already persisted, which makes duplicate retries much more likely.Suggested fix
await upsertFoodProducts({ body: { food_products: apiProducts }, }); // データを強制的に再取得(optimistic updateではなく確実な更新) await mutateFoodProducts(); @@ - // 再提出完了時 - if (status === 'waiting_resubmission') { - // status更新処理 - await updateStatus('unapproved'); - } - // 成功時のみビューモードに戻す setIsEditing(false); toast.success(t('applications.foodProduct.messages.updateSuccess'), { position: 'top-right', autoClose: 3000, }); + + if (status === 'waiting_resubmission') { + try { + await updateStatus('unapproved'); + } catch (statusError) { + console.error('販売品ステータス更新エラー:', statusError); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/components/Applications/FoodProduct/hooks.ts` around lines 153 - 178, The current flow runs upsertFoodProducts and mutateFoodProducts then calls updateStatus and if updateStatus fails the outer catch treats the whole operation as failed; change this so updateStatus('unapproved') is awaited inside its own try/catch (only when status === 'waiting_resubmission') and handle its error separately—e.g., log or show a non-blocking warning toast instead of throwing—so success path (setIsEditing(false), success toast) runs when the upsert/mutate completed; locate the block around upsertFoodProducts, mutateFoodProducts, updateStatus, and status === 'waiting_resubmission' to implement the isolated try/catch.user/src/pages/home/index.tsx (2)
367-388:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent
groupIdfallback pattern.Lines 367, 378, and 388 use
groupId || 0, which convertsundefinedback to0. This seems inconsistent with the change at line 351 wheregroupIdis allowed to beundefined. Ifundefinedis a valid state, the API hooks should handle it; if0is the correct fallback, then line 351 should use?? 0.🔧 Suggested fix
Either revert line 351:
- const groupId = groupUserIdAndGroupCategoryId?.id ?? undefined; + const groupId = groupUserIdAndGroupCategoryId?.id ?? 0;Or remove the fallbacks and ensure child components handle
undefined:- groupId={groupId || 0} + groupId={groupId}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/pages/home/index.tsx` around lines 367 - 388, There is an inconsistent fallback for groupId: some places pass groupId || 0 while earlier code allows groupId to be undefined; pick one approach and make it consistent — either (A) enforce a 0 default at the source (set the parent variable to use nullish coalescing, e.g. groupId = incomingGroupId ?? 0) so all children (ViceRepresentative, GroupCategoryContent and any other consumers using the groupId prop) can safely rely on a numeric id, or (B) remove the inline fallbacks (drop the "|| 0" in the props for ViceRepresentative and GroupCategoryContent and anywhere else) and update those child components (their prop types/handlers) to accept undefined and handle that state; update the symbol(s) ViceRepresentative, GroupCategoryContent and any mutate* callers accordingly so the behavior is uniform.
162-339:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus prop not being threaded to resubmission-enabled components in other categories.
VenueMap, FoodProduct, and FireEquipment components accept a
status?: stringprop and implement resubmission logic (derivable from their hooks), but only theFOOD_SALEScategory extracts submission status data and threads it to these components. TheGOODS_SALES,RESEARCH_LAB,EXHIBITION, andCOMMITTEEcategories use these same components without thestatusprop, leaving their resubmission feature inactive.Clarify whether resubmission is intentionally limited to food sales only, or if other categories should also derive and pass submission status to enable resubmission functionality for their respective components.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/pages/home/index.tsx` around lines 162 - 339, The VenueMap, FoodProduct, and FireEquipment components accept a status?: string prop used to enable resubmission, but in the GOODS_SALES, RESEARCH_LAB, EXHIBITION, and COMMITTEE branches you are not passing status (only FOOD_SALES does), disabling resubmission; fix this by deriving the submission status in those branches the same way you do in the FOOD_SALES branch and pass it into the components as status={...} (update the VenueMap, FoodProduct, and FireEquipment JSX in the GOODS_SALES, RESEARCH_LAB, EXHIBITION, and COMMITTEE branches to include the status prop, using the same status source logic used for FOOD_SALES so resubmission logic activates where appropriate).
🧹 Nitpick comments (4)
user/src/api/healthCenterSubmissionStatusApi.ts (1)
43-47: ⚡ Quick winAvoid raw
stringfor submission status.The
statusfield is currentlystring, but downstream logic depends on specific literals (e.g.waiting_resubmission). Defining a status union here will tighten type safety across the flow.💡 Suggested refactor
+export type HealthCenterSubmissionStatus = + | 'unapproved' + | 'waiting_resubmission' + | 'approved' + | 'unsubmitted'; + export type HealthCenterSubmissionStatusResponse = { id: number; group_id: number; applicationType: string; - status: string; + status: HealthCenterSubmissionStatus; createdAt: string; updatedAt: string; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/api/healthCenterSubmissionStatusApi.ts` around lines 43 - 47, The status field in HealthCenterSubmissionStatusResponse should not be a raw string; define a typed union (e.g., export a SubmissionStatus type with the allowed literals such as "waiting_resubmission" and the other known status values) and replace status: string with status: SubmissionStatus; update any related usages to import/use SubmissionStatus so downstream code has strict literal typing.user/src/components/Applications/VenueMap/hooks.ts (1)
35-35: ⚡ Quick winReplace magic string with a constant or enum.
The string literal
'unapproved'is used as a type constraint and passed directly. Define this as a constant or use an enum to prevent typos and improve maintainability.♻️ Use constants
+const APPLICATION_STATUS = { + UNAPPROVED: 'unapproved', + APPROVED: 'approved', + WAITING_RESUBMISSION: 'waiting_resubmission', +} as const; + -const updateStatus = async (status: 'unapproved') => { +const updateStatus = async (status: typeof APPLICATION_STATUS.UNAPPROVED) => { const venueMapSubmission = healthCenterSubmissionStatus.find( (submission) => submission.applicationType === 'venue_map' ); // ... await patchHealthCenterSubmissionStatus({ id: venueMapSubmission.id, body: { status }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/components/Applications/VenueMap/hooks.ts` at line 35, The updateStatus function currently uses the literal 'unapproved' as both a type and value; define a shared constant or enum (e.g., VenueStatus with UNAPPROVED) and replace the literal in the updateStatus signature and all callers where 'unapproved' is passed; update any related type annotations or imports to reference the new VenueStatus constant/enum so callers use VenueStatus.UNAPPROVED instead of the magic string.user/src/pages/home/index.tsx (1)
82-99: ⚡ Quick winOptimize multiple
find()calls with a lookup map.Lines 82-99 perform six separate
.find()calls onhealthCenterSubmissionStatus. This is inefficient and could be replaced with a single pass to build a lookup object.♻️ Optimize with a lookup map
if (groupCategoryId === GROUP_CATEGORY.FOOD_SALES) { - const employeeSubmission = healthCenterSubmissionStatus?.find( - (s) => s.applicationType === 'employee' - ); - const foodProductsSubmission = healthCenterSubmissionStatus?.find( - (s) => s.applicationType === 'food_product' - ); - // ... etc + const submissionByType = healthCenterSubmissionStatus?.reduce( + (acc, submission) => { + acc[submission.applicationType] = submission; + return acc; + }, + {} as Record<string, HealthCenterSubmissionStatusResponse> + ) ?? {}; + + const employeeSubmission = submissionByType['employee']; + const foodProductsSubmission = submissionByType['food_product']; + const purchaseListsSubmission = submissionByType['purchase_list']; + const venueMapSubmission = submissionByType['venue_map']; + const cookingProcessOrderSubmission = submissionByType['cooking_process_order']; + const fireEquipmentSubmission = submissionByType['equipment'];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/pages/home/index.tsx` around lines 82 - 99, Multiple .find() calls over healthCenterSubmissionStatus (used to create employeeSubmission, foodProductsSubmission, purchaseListsSubmission, venueMapSubmission, cookingProcessOrderSubmission, fireEquipmentSubmission) cause repeated iteration; replace them with a single pass that builds a lookup (e.g., reduce into an object or a Map keyed by applicationType) and then reference lookup['employee'], lookup['food_product'], etc.; update the variables to pull from that lookup (employeeSubmission = lookup['employee'], etc.) so you iterate the array only once.user/src/components/Applications/Employees/hooks.ts (1)
401-401: ⚡ Quick winReplace magic string with a constant.
Same as in
VenueMap/hooks.ts— use a constant or enum for'unapproved'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@user/src/components/Applications/Employees/hooks.ts` at line 401, The updateStatus function currently uses the magic string 'unapproved'; replace this literal with a shared constant or enum (e.g., STATUS_UNAPPROVED or an EmployeeStatus enum) to match the pattern used in VenueMap/hooks.ts. Update the function signature and any call sites that pass or compare 'unapproved' to import and use the new constant/enum, and ensure TypeScript types are adjusted (union or enum type) so updateStatus accepts the constant/enum value instead of the raw string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@user/src/api/healthCenterSubmissionStatusApi.ts`:
- Around line 66-68: The endpoint construction uses a truthy check on groupId
which incorrectly treats 0 as absent; update the conditional in the endpoint
assignment to use a nullish check (e.g., groupId != null) so that values like 0
are allowed, keeping the reference to
API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS and the endpoint constant
(endpoint) and groupId identifier intact.
In `@user/src/components/Applications/CookingProcessOrder/hooks.ts`:
- Around line 151-166: The status patch is performed before persisting data, so
if upsertCookingProcessOrders fails the submission stays in
waiting_resubmission; change the order so patchHealthCenterSubmissionStatus (and
mutateHealthCenterSubmissionStatus) is called only after await
upsertCookingProcessOrders completes successfully. Locate updateStatus and the
code path that calls upsertCookingProcessOrders (and the similar block around
lines 183-191), move the await patchHealthCenterSubmissionStatus(...) and await
mutateHealthCenterSubmissionStatus() to after the upsertCookingProcessOrders
call (or perform the patch inside the success branch), and ensure any thrown
errors from upsert prevent the status update.
In `@user/src/components/Applications/Employees/Employees.tsx`:
- Line 16: The status prop on the Employees component is currently typed as a
plain string; change it to the stricter application status type used across the
app (e.g. APPLICATION_STATUS or ApplicationStatus) by importing that enum/type
and updating the prop/interface (e.g., the Props or EmployeesProps where
`status?: string` is declared) to `status?: APPLICATION_STATUS` (or the exact
exported name), and then adjust any usages inside Employees (rendering/filters)
to accept that enum type.
- Line 38: The inline comment for the isEdit prop is inaccurate: it says
"期限内または再提出待ちの場合に編集可能" but the code sets isEdit={!isDeadline} and does not
account for the resubmission state handled inside the Content component; either
update the comment to reflect that isEdit currently only checks isDeadline, or
change the expression passed to the isEdit prop to include the resubmission flag
used by Content (e.g., combine !isDeadline || isWaitingForResubmission or the
existing resubmission state name) so the prop and comment stay consistent (refer
to the isEdit prop, isDeadline variable, and the Content component's
resubmission logic).
In `@user/src/components/Applications/Employees/hooks.ts`:
- Line 378: The parameter currently typed as status?: string in
user/src/components/Applications/Employees/hooks.ts is too broad; change it to a
constrained type (e.g., an existing EmployeeStatus enum/union or create one like
'type EmployeeStatus = "active" | "inactive" | "terminated"') and replace
status?: string with status?: EmployeeStatus in the hook signature(s) that
accept status (search for functions/hooks in this file that declare a status
parameter), and update any callers or tests to use the specific values or cast
accordingly.
- Around line 518-524: The updateStatus('unapproved') call can reject and
currently has no error handling; wrap this call in a try/catch around the block
that checks if (status === 'waiting_resubmission') and on failure log the error
(use the existing logger or console.error) and surface user feedback (e.g., show
a toast or return an error response) so the rejection is handled gracefully
instead of causing an unhandled rejection; ensure you reference updateStatus and
the status check when implementing the try/catch and preserve existing flow on
success.
In `@user/src/components/Applications/FireEquipment/FireEquipment.tsx`:
- Line 11: The status prop on the FireEquipment component is currently typed too
broadly as status?: string; update the Props/interface for FireEquipment to use
the same stricter union type used elsewhere (for example replace string with the
established StatusType union like 'active'|'inactive'|'unknown' or the project's
shared Status enum/type), and update any imports to pull in the shared Status
type if it exists; ensure the prop declaration (status?: ...) and any places
that read or pass status in the FireEquipment component match the new type.
- Around line 35-46: Add a clarifying inline comment where the
FireEquipmentFormView is rendered with disableValidate (the branch guarded by
isDeadline && isResubmission) that explains that disableValidate intentionally
skips change-detection validation (not schema/Zod validation) to allow
resubmission even when no field values changed; reference the
FireEquipmentFormView prop disableValidate and the conditions isDeadline and
isResubmission, and add a matching comment in the similar branch that appears
later (the other condition around line 56) so future maintainers understand this
is intentional behavior.
In `@user/src/components/Applications/PurchaseLists/hooks.ts`:
- Around line 410-416: The current flow calls onSuccess() before awaiting
updateStatus('unapproved'), so if updateStatus fails the UI already left edit
mode and user sees a failure for a partially successful submit; change the flow
to perform the status patch first when status === 'waiting_resubmission' (await
updateStatus('unapproved')) and only call onSuccess() after that succeeds (or
alternatively call onSuccess() conditionally only when updateStatus resolves),
and keep reset(formData) after onSuccess(); also ensure the catch only treats
errors from the full sequence so failures in updateStatus are handled before UI
state changes.
In `@user/src/components/Applications/VenueMap/hooks.ts`:
- Around line 35-50: The updateStatus function can throw when no venue_map
submission exists and handleFormSubmitted calls it without catching errors; wrap
the call to updateStatus inside handleFormSubmitted with a try-catch (or
alternatively make updateStatus return a success/failure value instead of
throwing) and on failure log the error and surface a user-friendly message or
safe fallback; ensure you reference healthCenterSubmissionStatus,
patchHealthCenterSubmissionStatus and mutateHealthCenterSubmissionStatus so any
cleanup/mutateHealthCenterSubmissionStatus is still called on success or handled
appropriately on failure.
- Line 11: The status parameter on useVenueMapHooks is currently typed as
string; define a constrained union type (e.g., ApplicationStatus =
'waiting_resubmission' | 'approved' | 'unapproved') in a shared types file or
nearby and change the signature to useVenueMapHooks(groupId: number, status?:
ApplicationStatus) so only valid statuses are allowed; update any imports/usages
of useVenueMapHooks accordingly and run TypeScript checks to fix resulting type
errors.
- Line 33: The hook is being invoked twice—change the double-call pattern
useUpdateHealthCenterSubmissionStatus()() to a single hook call and destructure
its trigger; e.g. call const { trigger: patchHealthCenterSubmissionStatus } =
useUpdateHealthCenterSubmissionStatus();; locate
useUpdateHealthCenterSubmissionStatus (which wraps
useAuthenticatedPatchWithId(endpoint)) and replace any instances of the trailing
() invocation (also fix the same pattern for the analogous hooks in FoodProduct,
Employees, PurchaseLists, CookingProcessOrder, and FireEquipment) so the hook is
only called once and its trigger is used.
In `@user/src/components/Applications/VenueMap/VenueMap.tsx`:
- Line 11: The status prop on VenueMap (currently declared as status?: string)
should be narrowed to a specific union or enum to match hooks.ts; define a
VenueStatus union (e.g. 'open' | 'closed' | 'pending' etc.) or an exported enum
and change the prop signature in the VenueMapProps/interface from status?:
string to status?: VenueStatus, then update any usages and imports of VenueMap
and related hooks to use the same VenueStatus type so the types remain
consistent across VenueMap and hooks.ts.
In `@user/src/pages/home/index.tsx`:
- Line 351: The change made groupId to be possibly undefined (const groupId =
groupUserIdAndGroupCategoryId?.id ?? undefined), which breaks type expectations
for components that require groupId: number (VenueApplications, RentItems,
Power, PublicRelations, Employees, VenueMap, FoodProduct, PurchaseLists,
CookingProcessOrder, FireEquipment). Fix by either passing a numeric fallback
when rendering those components (use groupId || 0 in the JSX props where each
component is invoked) or change each component's prop type to accept number |
undefined and add internal guards in their props-handling logic; prefer the
former to match existing fallback usage (see renders around lines where groupId
is passed) to restore type compatibility without changing component internals.
---
Outside diff comments:
In `@user/src/components/Applications/FireEquipment/components/hooks.ts`:
- Around line 139-188: The resubmission status transition is only applied in
onSubmitFireEquipment, so when a user submits via the “火気不使用” path handled by
onSubmitUnregistered the health-center status can remain waiting_resubmission;
update onSubmitUnregistered to mirror onSubmitFireEquipment’s behavior by
calling updateStatus('unapproved') after the successful submit (after post/patch
and mutateFireEquipmentOrder/mutate equivalent and toast), guarded by if (status
=== 'waiting_resubmission'); use the existing updateStatus function to perform
the patch and subsequent mutateHealthCenterSubmissionStatus.
In `@user/src/components/Applications/FoodProduct/hooks.ts`:
- Around line 153-178: The current flow runs upsertFoodProducts and
mutateFoodProducts then calls updateStatus and if updateStatus fails the outer
catch treats the whole operation as failed; change this so
updateStatus('unapproved') is awaited inside its own try/catch (only when status
=== 'waiting_resubmission') and handle its error separately—e.g., log or show a
non-blocking warning toast instead of throwing—so success path
(setIsEditing(false), success toast) runs when the upsert/mutate completed;
locate the block around upsertFoodProducts, mutateFoodProducts, updateStatus,
and status === 'waiting_resubmission' to implement the isolated try/catch.
In `@user/src/pages/home/index.tsx`:
- Around line 367-388: There is an inconsistent fallback for groupId: some
places pass groupId || 0 while earlier code allows groupId to be undefined; pick
one approach and make it consistent — either (A) enforce a 0 default at the
source (set the parent variable to use nullish coalescing, e.g. groupId =
incomingGroupId ?? 0) so all children (ViceRepresentative, GroupCategoryContent
and any other consumers using the groupId prop) can safely rely on a numeric id,
or (B) remove the inline fallbacks (drop the "|| 0" in the props for
ViceRepresentative and GroupCategoryContent and anywhere else) and update those
child components (their prop types/handlers) to accept undefined and handle that
state; update the symbol(s) ViceRepresentative, GroupCategoryContent and any
mutate* callers accordingly so the behavior is uniform.
- Around line 162-339: The VenueMap, FoodProduct, and FireEquipment components
accept a status?: string prop used to enable resubmission, but in the
GOODS_SALES, RESEARCH_LAB, EXHIBITION, and COMMITTEE branches you are not
passing status (only FOOD_SALES does), disabling resubmission; fix this by
deriving the submission status in those branches the same way you do in the
FOOD_SALES branch and pass it into the components as status={...} (update the
VenueMap, FoodProduct, and FireEquipment JSX in the GOODS_SALES, RESEARCH_LAB,
EXHIBITION, and COMMITTEE branches to include the status prop, using the same
status source logic used for FOOD_SALES so resubmission logic activates where
appropriate).
---
Nitpick comments:
In `@user/src/api/healthCenterSubmissionStatusApi.ts`:
- Around line 43-47: The status field in HealthCenterSubmissionStatusResponse
should not be a raw string; define a typed union (e.g., export a
SubmissionStatus type with the allowed literals such as "waiting_resubmission"
and the other known status values) and replace status: string with status:
SubmissionStatus; update any related usages to import/use SubmissionStatus so
downstream code has strict literal typing.
In `@user/src/components/Applications/Employees/hooks.ts`:
- Line 401: The updateStatus function currently uses the magic string
'unapproved'; replace this literal with a shared constant or enum (e.g.,
STATUS_UNAPPROVED or an EmployeeStatus enum) to match the pattern used in
VenueMap/hooks.ts. Update the function signature and any call sites that pass or
compare 'unapproved' to import and use the new constant/enum, and ensure
TypeScript types are adjusted (union or enum type) so updateStatus accepts the
constant/enum value instead of the raw string.
In `@user/src/components/Applications/VenueMap/hooks.ts`:
- Line 35: The updateStatus function currently uses the literal 'unapproved' as
both a type and value; define a shared constant or enum (e.g., VenueStatus with
UNAPPROVED) and replace the literal in the updateStatus signature and all
callers where 'unapproved' is passed; update any related type annotations or
imports to reference the new VenueStatus constant/enum so callers use
VenueStatus.UNAPPROVED instead of the magic string.
In `@user/src/pages/home/index.tsx`:
- Around line 82-99: Multiple .find() calls over healthCenterSubmissionStatus
(used to create employeeSubmission, foodProductsSubmission,
purchaseListsSubmission, venueMapSubmission, cookingProcessOrderSubmission,
fireEquipmentSubmission) cause repeated iteration; replace them with a single
pass that builds a lookup (e.g., reduce into an object or a Map keyed by
applicationType) and then reference lookup['employee'], lookup['food_product'],
etc.; update the variables to pull from that lookup (employeeSubmission =
lookup['employee'], etc.) so you iterate the array only once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9cb70e0f-849d-4a15-93eb-ec8e7d552389
⛔ Files ignored due to path filters (1)
admin_view/nuxt-project/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
admin_view/nuxt-project/package.jsonuser/public/locales/en/common.jsonuser/public/locales/ja/common.jsonuser/src/api/checkAllRegisteredApi.tsuser/src/api/healthCenterSubmissionStatusApi.tsuser/src/components/AccordionMenu/AccordionMenu.tsxuser/src/components/Applications/CookingProcessOrder/CookingProcessOrder.tsxuser/src/components/Applications/CookingProcessOrder/hooks.tsuser/src/components/Applications/Employees/Employees.tsxuser/src/components/Applications/Employees/hooks.tsuser/src/components/Applications/FireEquipment/FireEquipment.tsxuser/src/components/Applications/FireEquipment/components/FireEquipmentFormView.tsxuser/src/components/Applications/FireEquipment/components/hooks.tsuser/src/components/Applications/FireEquipment/hooks.tsuser/src/components/Applications/FoodProduct/FoodProduct.tsxuser/src/components/Applications/FoodProduct/hooks.tsuser/src/components/Applications/PurchaseLists/PurchaseLists.tsxuser/src/components/Applications/PurchaseLists/hooks.tsuser/src/components/Applications/VenueMap/VenueMap.tsxuser/src/components/Applications/VenueMap/hooks.tsuser/src/components/Status/Status.tsxuser/src/components/Status/hooks.tsuser/src/pages/home/index.tsxuser/src/styles/globals.cssuser/src/utils/constants.tsuser/tailwind.config.ts
💤 Files with no reviewable changes (1)
- admin_view/nuxt-project/package.json
| const endpoint = groupId | ||
| ? `${API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS}/${groupId}` | ||
| : null; |
There was a problem hiding this comment.
Use a nullish check for groupId when building the endpoint.
At Line 66, the truthy check (groupId ? ...) skips fetch when groupId is 0. Use groupId != null for consistent behavior.
💡 Suggested fix
- const endpoint = groupId
+ const endpoint = groupId != null
? `${API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS}/${groupId}`
: null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const endpoint = groupId | |
| ? `${API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS}/${groupId}` | |
| : null; | |
| const endpoint = groupId != null | |
| ? `${API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS}/${groupId}` | |
| : null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/api/healthCenterSubmissionStatusApi.ts` around lines 66 - 68, The
endpoint construction uses a truthy check on groupId which incorrectly treats 0
as absent; update the conditional in the endpoint assignment to use a nullish
check (e.g., groupId != null) so that values like 0 are allowed, keeping the
reference to API_ENDPOINTS.HEALTH_CENTER_SUBMISSION_STATUS and the endpoint
constant (endpoint) and groupId identifier intact.
| const updateStatus = async (status: 'unapproved') => { | ||
| const cookingProcessOrderSubmission = healthCenterSubmissionStatus.find( | ||
| (submission) => submission.applicationType === 'cooking_process_order' | ||
| ); | ||
|
|
||
| if (!cookingProcessOrderSubmission?.id) { | ||
| throw new Error('Cooking process order submission status id not found'); | ||
| } | ||
|
|
||
| await patchHealthCenterSubmissionStatus({ | ||
| id: cookingProcessOrderSubmission.id, | ||
| body: { status }, | ||
| }); | ||
|
|
||
| await mutateHealthCenterSubmissionStatus(); | ||
| }; |
There was a problem hiding this comment.
Update the submission status only after the save succeeds.
Right now the resubmission status is patched before upsertCookingProcessOrders. If the upsert fails afterward, this application leaves waiting_resubmission even though the user’s changes were never persisted.
Suggested fix
- // 再提出完了時
- if (status === 'waiting_resubmission') {
- // status更新処理
- await updateStatus('unapproved');
- }
-
await upsertCookingProcessOrders({
body: { cooking_process_orders: payload },
});
+
+ if (status === 'waiting_resubmission') {
+ await updateStatus('unapproved');
+ }Also applies to: 183-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/CookingProcessOrder/hooks.ts` around lines
151 - 166, The status patch is performed before persisting data, so if
upsertCookingProcessOrders fails the submission stays in waiting_resubmission;
change the order so patchHealthCenterSubmissionStatus (and
mutateHealthCenterSubmissionStatus) is called only after await
upsertCookingProcessOrders completes successfully. Locate updateStatus and the
code path that calls upsertCookingProcessOrders (and the similar block around
lines 183-191), move the await patchHealthCenterSubmissionStatus(...) and await
mutateHealthCenterSubmissionStatus() to after the upsertCookingProcessOrders
call (or perform the patch inside the success branch), and ensure any thrown
errors from upsert prevent the status update.
| isRegistered?: boolean; // 既に登録済みかどうか | ||
| groupId: number; // 対象のグループID | ||
| mutateCheckAllRegisteredGroups: () => void; | ||
| status?: string; // 申請のステータス(APPLICATION_STATUSの値) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a more specific type for the status prop.
Consistent with other components, use a stricter type for status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/Employees/Employees.tsx` at line 16, The
status prop on the Employees component is currently typed as a plain string;
change it to the stricter application status type used across the app (e.g.
APPLICATION_STATUS or ApplicationStatus) by importing that enum/type and
updating the prop/interface (e.g., the Props or EmployeesProps where `status?:
string` is declared) to `status?: APPLICATION_STATUS` (or the exact exported
name), and then adjust any usages inside Employees (rendering/filters) to accept
that enum type.
| <AccordionMenu | ||
| title={employeesApplicationHook.texts.title} | ||
| isEdit={!isDeadline} // 期限内(isDeadline=false)の場合のみ編集可能 | ||
| isEdit={!isDeadline} // 期限内または再提出待ちの場合に編集可能 |
There was a problem hiding this comment.
Fix inconsistent comment.
The comment states "期限内または再提出待ちの場合に編集可能" (editable during deadline or resubmission), but isEdit={!isDeadline} only checks the deadline, not the resubmission status. The actual resubmission logic is handled inside Content at lines 109-113 and 126-131.
📝 Update the comment
<AccordionMenu
title={employeesApplicationHook.texts.title}
- isEdit={!isDeadline} // 期限内または再提出待ちの場合に編集可能
+ isEdit={!isDeadline} // 期限内の場合に編集アイコン表示
isExist={isRegistered} // 登録済みの場合に表示
required={true} // 必須項目として表示
status={status} // 申請のステータスを渡す
>Or update the logic to match the comment:
- isEdit={!isDeadline} // 期限内または再提出待ちの場合に編集可能
+ isEdit={!isDeadline || status === 'waiting_resubmission'} // 期限内または再提出待ちの場合に編集可能📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| isEdit={!isDeadline} // 期限内または再提出待ちの場合に編集可能 | |
| isEdit={!isDeadline} // 期限内の場合に編集アイコン表示 |
| isEdit={!isDeadline} // 期限内または再提出待ちの場合に編集可能 | |
| isEdit={!isDeadline || status === 'waiting_resubmission'} // 期限内または再提出待ちの場合に編集可能 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/Employees/Employees.tsx` at line 38, The
inline comment for the isEdit prop is inaccurate: it says "期限内または再提出待ちの場合に編集可能"
but the code sets isEdit={!isDeadline} and does not account for the resubmission
state handled inside the Content component; either update the comment to reflect
that isEdit currently only checks isDeadline, or change the expression passed to
the isEdit prop to include the resubmission flag used by Content (e.g., combine
!isDeadline || isWaitingForResubmission or the existing resubmission state name)
so the prop and comment stay consistent (refer to the isEdit prop, isDeadline
variable, and the Content component's resubmission logic).
| isDeadline?: boolean, | ||
| mutateCheckAllRegisteredGroups?: () => void | ||
| mutateCheckAllRegisteredGroups?: () => void, | ||
| status?: string |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a more specific type for the status parameter.
Same issue as in other hooks — status?: string should be constrained to valid status values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/Employees/hooks.ts` at line 378, The
parameter currently typed as status?: string in
user/src/components/Applications/Employees/hooks.ts is too broad; change it to a
constrained type (e.g., an existing EmployeeStatus enum/union or create one like
'type EmployeeStatus = "active" | "inactive" | "terminated"') and replace
status?: string with status?: EmployeeStatus in the hook signature(s) that
accept status (search for functions/hooks in this file that declare a status
parameter), and update any callers or tests to use the specific values or cast
accordingly.
| import { venueMapLabels } from '../label'; | ||
|
|
||
| export const useVenueMapHooks = (groupId: number) => { | ||
| export const useVenueMapHooks = (groupId: number, status?: string) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a more specific type for the status parameter.
The status parameter is typed as string, but it should be constrained to valid application status values. This would prevent invalid status strings from being passed and improve type safety.
📐 Suggested improvement
-export const useVenueMapHooks = (groupId: number, status?: string) => {
+export const useVenueMapHooks = (groupId: number, status?: 'waiting_resubmission' | 'approved' | 'unapproved') => {Or better yet, define and import a type:
// In a shared types file
export type ApplicationStatus = 'waiting_resubmission' | 'approved' | 'unapproved';
// In this file
export const useVenueMapHooks = (groupId: number, status?: ApplicationStatus) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useVenueMapHooks = (groupId: number, status?: string) => { | |
| export const useVenueMapHooks = (groupId: number, status?: 'waiting_resubmission' | 'approved' | 'unapproved') => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/VenueMap/hooks.ts` at line 11, The status
parameter on useVenueMapHooks is currently typed as string; define a constrained
union type (e.g., ApplicationStatus = 'waiting_resubmission' | 'approved' |
'unapproved') in a shared types file or nearby and change the signature to
useVenueMapHooks(groupId: number, status?: ApplicationStatus) so only valid
statuses are allowed; update any imports/usages of useVenueMapHooks accordingly
and run TypeScript checks to fix resulting type errors.
| const { healthCenterSubmissionStatus, mutateHealthCenterSubmissionStatus } = | ||
| useGetHealthCenterSubmissionStatus(groupId); | ||
| const { trigger: patchHealthCenterSubmissionStatus } = | ||
| useUpdateHealthCenterSubmissionStatus()(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the signature of useUpdateHealthCenterSubmissionStatus
ast-grep --pattern 'export const useUpdateHealthCenterSubmissionStatus = $$$'
# Also check other similar update hooks for comparison
rg -nP 'useUpdate\w+\(\)\(\)' --type=ts -C2Repository: NUTFes/group-manager-2
Length of output: 4687
🏁 Script executed:
# Find the definition of useAuthenticatedPatchWithId
rg -n 'export.*useAuthenticatedPatchWithId|const useAuthenticatedPatchWithId' --type=ts -A5Repository: NUTFes/group-manager-2
Length of output: 505
🏁 Script executed:
# Find createMutationHookWithId definition
rg -n 'function createMutationHookWithId|const createMutationHookWithId|export.*createMutationHookWithId' --type=ts -A10Repository: NUTFes/group-manager-2
Length of output: 2124
🏁 Script executed:
# Get full definition of useUpdateHealthCenterSubmissionStatus
rg -n 'export const useUpdateHealthCenterSubmissionStatus' --type=ts -A5Repository: NUTFes/group-manager-2
Length of output: 537
Remove the double invocation pattern — it incorrectly calls the hook twice.
useUpdateHealthCenterSubmissionStatus() already returns the hook from useAuthenticatedPatchWithId(endpoint). The second () attempts to invoke the hook itself, which is incorrect. Use:
const { trigger: patchHealthCenterSubmissionStatus } = useUpdateHealthCenterSubmissionStatus();
This same pattern appears in at least 5 other files (FoodProduct, Employees, PurchaseLists, CookingProcessOrder, FireEquipment) and should be corrected across the codebase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/VenueMap/hooks.ts` at line 33, The hook is
being invoked twice—change the double-call pattern
useUpdateHealthCenterSubmissionStatus()() to a single hook call and destructure
its trigger; e.g. call const { trigger: patchHealthCenterSubmissionStatus } =
useUpdateHealthCenterSubmissionStatus();; locate
useUpdateHealthCenterSubmissionStatus (which wraps
useAuthenticatedPatchWithId(endpoint)) and replace any instances of the trailing
() invocation (also fix the same pattern for the analogous hooks in FoodProduct,
Employees, PurchaseLists, CookingProcessOrder, and FireEquipment) so the hook is
only called once and its trigger is used.
| const updateStatus = async (status: 'unapproved') => { | ||
| const venueMapSubmission = healthCenterSubmissionStatus.find( | ||
| (submission) => submission.applicationType === 'venue_map' | ||
| ); | ||
|
|
||
| if (!venueMapSubmission?.id) { | ||
| throw new Error('Venue map submission status id not found'); | ||
| } | ||
|
|
||
| await patchHealthCenterSubmissionStatus({ | ||
| id: venueMapSubmission.id, | ||
| body: { status }, | ||
| }); | ||
|
|
||
| await mutateHealthCenterSubmissionStatus(); | ||
| }; |
There was a problem hiding this comment.
Add error handling for the status update flow.
The updateStatus function throws an error when the submission is not found (line 41), and is called without a try-catch in handleFormSubmitted (line 56). This will cause unhandled promise rejections if the submission record is missing.
🛡️ Add error handling
const handleFormSubmitted = async () => {
if (status === 'waiting_resubmission') {
- // status更新処理
- await updateStatus('unapproved');
+ try {
+ await updateStatus('unapproved');
+ } catch (error) {
+ console.error('Failed to update submission status:', error);
+ // Consider showing a user-facing error message
+ }
}
setIsEditing(false);
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/VenueMap/hooks.ts` around lines 35 - 50, The
updateStatus function can throw when no venue_map submission exists and
handleFormSubmitted calls it without catching errors; wrap the call to
updateStatus inside handleFormSubmitted with a try-catch (or alternatively make
updateStatus return a success/failure value instead of throwing) and on failure
log the error and surface a user-friendly message or safe fallback; ensure you
reference healthCenterSubmissionStatus, patchHealthCenterSubmissionStatus and
mutateHealthCenterSubmissionStatus so any
cleanup/mutateHealthCenterSubmissionStatus is still called on success or handled
appropriately on failure.
| groupId: number; | ||
| isDeadline?: boolean; | ||
| isRegistered?: boolean; | ||
| status?: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a more specific type for the status prop.
Same issue as in hooks.ts — the status prop should use a union type or enum rather than the broad string type.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/components/Applications/VenueMap/VenueMap.tsx` at line 11, The
status prop on VenueMap (currently declared as status?: string) should be
narrowed to a specific union or enum to match hooks.ts; define a VenueStatus
union (e.g. 'open' | 'closed' | 'pending' etc.) or an exported enum and change
the prop signature in the VenueMapProps/interface from status?: string to
status?: VenueStatus, then update any usages and imports of VenueMap and related
hooks to use the same VenueStatus type so the types remain consistent across
VenueMap and hooks.ts.
| isLoading: isLoadingGroupByUserId, | ||
| } = useGetGroupByUserId(userId); | ||
| const groupId = groupUserIdAndGroupCategoryId?.id ?? 0; | ||
| const groupId = groupUserIdAndGroupCategoryId?.id ?? undefined; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if useGetCheckAllRegisteredGroups handles undefined groupId
ast-grep --pattern 'export const useGetCheckAllRegisteredGroups = ($$$) => { $$$ }'
# Check if useGetHealthCenterSubmissionStatus handles undefined groupId
ast-grep --pattern 'export const useGetHealthCenterSubmissionStatus = ($$$) => { $$$ }'
# Find all places where groupId is used as a required number
rg -nP 'groupId:\s*number[^?|]' --type=ts user/src/components/Applications/ -C2Repository: NUTFes/group-manager-2
Length of output: 34062
🏁 Script executed:
# First, get the file and check around line 351
wc -l user/src/pages/home/index.tsxRepository: NUTFes/group-manager-2
Length of output: 100
🏁 Script executed:
# View the context around line 351 to see how groupId is used after the assignment
sed -n '345,400p' user/src/pages/home/index.tsxRepository: NUTFes/group-manager-2
Length of output: 2235
🏁 Script executed:
# Check what components receive groupId in home/index.tsx
rg -n 'groupId=' user/src/pages/home/index.tsx -A2 -B2Repository: NUTFes/group-manager-2
Length of output: 8837
🏁 Script executed:
# Check the exact type definitions for components that receive groupId without || 0 fallback
rg -n 'type VenueApplicationProps|type RentItemsProps|type PowerProps|type PublicRelationsProps|type VenueMapProps|type FoodProductProps|type PurchaseListsProps|type CookingProcessOrderProps|type FireEquipmentProps|type EmployeesProps' user/src/components/Applications/ -A5 | head -100Repository: NUTFes/group-manager-2
Length of output: 5514
🏁 Script executed:
# Check the full context around lines 106-158 to see if groupId is guarded
sed -n '95,160p' user/src/pages/home/index.tsxRepository: NUTFes/group-manager-2
Length of output: 2493
🏁 Script executed:
# Check if there's any conditional check that guards the rendering of these components
sed -n '80,110p' user/src/pages/home/index.tsxRepository: NUTFes/group-manager-2
Length of output: 1314
🏁 Script executed:
# Check what happens when groupUserIdAndGroupCategoryId is null/undefined
sed -n '348,365p' user/src/pages/home/index.tsxRepository: NUTFes/group-manager-2
Length of output: 920
Pass groupId || 0 to components requiring number type, or update component props to accept number | undefined.
Changing from ?? 0 to ?? undefined at line 351 makes groupId potentially undefined. However, components at lines 106–158 (VenueApplications, RentItems, Power, PublicRelations, Employees, VenueMap, FoodProduct, PurchaseLists, CookingProcessOrder, FireEquipment) receive groupId directly without fallback, but all require groupId: number per their type definitions. This creates a type mismatch that will cause pnpm run type-check to fail, violating the Next.js coding guidelines.
Either:
- Apply the
|| 0fallback pattern already used at lines 367, 378, 388 to all component passes, or - Update component prop types to accept
number | undefinedand add internal guards
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@user/src/pages/home/index.tsx` at line 351, The change made groupId to be
possibly undefined (const groupId = groupUserIdAndGroupCategoryId?.id ??
undefined), which breaks type expectations for components that require groupId:
number (VenueApplications, RentItems, Power, PublicRelations, Employees,
VenueMap, FoodProduct, PurchaseLists, CookingProcessOrder, FireEquipment). Fix
by either passing a numeric fallback when rendering those components (use
groupId || 0 in the JSX props where each component is invoked) or change each
component's prop type to accept number | undefined and add internal guards in
their props-handling logic; prefer the former to match existing fallback usage
(see renders around lines where groupId is passed) to restore type compatibility
without changing component internals.
…roup-manager-2 into feat/riririn/2038-user-resubmission
対応Issue
resolve #2038
概要
実装詳細
画面スクリーンショット等
テスト項目
備考
火気使用申請のみ受付期間の切り替えができなかったので動作確認できてません
Summary by CodeRabbit
New Features
Localization
Style