fix: preserve navigation guard on failed questionnaire submission#15857
fix: preserve navigation guard on failed questionnaire submission#15857KshitijShukla30 wants to merge 2 commits intoohcnetwork:developfrom
Conversation
…gation guard on failed submission Fixes ohcnetwork#15855 setIsDirty(false) was called at the top of handleSubmit() before validation and before submitBatch resolved. This prematurely disabled the useNavigationPrompt guard, allowing users to navigate away without warning after a failed submission, silently losing clinical data. Move setIsDirty(false) into the submitBatch mutation's onSuccess callback so the dirty flag is only cleared after confirmed successful submission. This is consistent with createDraft and updateDraft which already correctly set isDirty in their onSuccess callbacks.
|
WalkthroughCleared early resetting of the form dirty flag: Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🚀 Preview Deployment Ready!🔗 Preview URL: https://pr-15857.care-preview-a7w.pages.dev 📱 Mobile Access: This preview will be automatically updated when you push new commits to this PR. |
|
Did you read our contributors guide? |
1 similar comment
|
Did you read our contributors guide? |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/facility/patient/questionnaire-dirty-state.spec.ts`:
- Line 43: Replace the two hard-coded page.waitForTimeout calls in the
questionnaire-dirty-state.spec tests with deterministic, event-driven waits:
after rendering the form, wait for a stable UI signal such as the submit button
becoming enabled or a required-field label to appear using page.waitForSelector
or locator.waitFor (reference the test block and the submit
button/required-field locator used in that spec), and after triggering
validation, wait for the validation error element to appear with
page.waitForSelector or locator.waitFor instead of sleeping; update the waits
used around the actions that currently call page.waitForTimeout(1000) and
page.waitForTimeout(500) to these selector/condition-based waits so the test
becomes deterministic.
- Around line 19-20: After clicking "View Encounter" and "Update Encounter" the
test does not wait for navigation/loads, causing races; wrap those clicks with
an explicit navigation/load wait (e.g., use Promise.all with
page.waitForNavigation() or await page.waitForLoadState('networkidle') or await
page.waitForURL(...) immediately after the click) so the page settles before the
subsequent combobox interaction; update the calls around page.getByText("View
Encounter").first().click() and page.getByRole("link", { name: "Update
Encounter" }).click() to include one of these wait patterns.
- Around line 38-40: The test currently clicks the firstOption
(page.locator("[cmdk-item]").first()) which may select a form without required
fields; instead either pin the chooser to a known questionnaire item (select by
its visible label/text) so the chosen form is guaranteed to have required
fields, or after selecting firstOption submit and assert that a validation error
appears (i.e., that submission did not succeed and setIsDirty(false) was not
called) before proceeding to the navigation guard check; reference the selection
step (firstOption/page.locator("[cmdk-item]")) and the dirty-state side effect
(setIsDirty(false)/navigation guard) when updating the test.
- Around line 62-76: The test incorrectly mixes two mechanisms and vacuously
passes; split into two focused tests: for the in-app raviger guard, replace the
page.goto("/") navigation with a user action that triggers history-API
navigation (e.g., click the in-app nav link element found via page.getByRole or
similar) and then assert unconditionally that page.getByText(/unsaved changes/i)
(unsavedChangesDialog) is visible; for the native beforeunload path, keep using
page.goto("/") but rely on the existing page.on('dialog') handler to catch the
beforeunload dialog and assert that the dialog was fired (or assert that
navigation was prevented via the dialog handler), ensuring you remove the
current conditional isVisible() branching so each test verifies exactly one
mechanism (reference: unsavedChangesDialog, page.goto, page.getByText,
page.on('dialog'), and the app's useNavigationPrompt behavior).
- Around line 48-77: The test currently gates the entire assertion body behind a
silent runtime check using if (await submitButton.isVisible()), which causes the
test to pass vacuously if the submit button is missing; replace that conditional
with a hard assertion (e.g., await expect(submitButton).toBeVisible()) and
remove the surrounding if block so the test fails immediately when the
precondition (submitButton visibility) is not met; keep the rest of the logic
(click, dialog handler, navigation and in-app prompt checks) intact and located
in the same spec (questionnaire-dirty-state.spec.ts) so failures are visible and
informative.
- Around line 56-59: The test currently attaches a fire-and-forget handler via
page.on("dialog", ...) so the expect inside that callback can be lost and the
dialog may never be observed in headless mode; change to awaiting the dialog
promise with page.waitForEvent("dialog") where you trigger the navigation, then
assert dialog.type() === "beforeunload" and call dialog.dismiss() — update the
test around the existing page.on usage to use page.waitForEvent("dialog") and
ensure the promise is awaited so assertion failures propagate to the test
runner.
- Around line 35-38: Replace the fragile cmdk library selectors in the test:
change the page.locator("[cmdk-input]") waitFor and the
page.locator("[cmdk-item]").first() usage so they use the stable data-slot
attributes exposed by the wrapper (use "[data-slot=\"command-input\"]" instead
of "[cmdk-input]" and "[data-slot=\"command-item\"]" instead of "[cmdk-item]");
update the two places where those selectors are referenced (the waitFor call and
the firstOption locator assignment) so the test targets
data-slot="command-input" and data-slot="command-item" respectively.
- Use data-slot selectors instead of cmdk internal attributes - Replace waitForTimeout with deterministic waits (networkidle, toBeVisible) - Add navigation wait after View Encounter and Update Encounter clicks - Use unconditional assertions instead of vacuous if-guards - Test the in-app raviger navigation guard (unsaved changes dialog) - Gracefully skip if selected form has no required fields
f0ab091 to
56d6cd7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/facility/patient/questionnaire-dirty-state.spec.ts`:
- Around line 56-64: The current use of .catch(() => test.skip()) on await
expect(validationError).toBeVisible() swallows all errors; wrap the await in a
try/catch instead, and in the catch inspect the thrown Error from
expect(...).toBeVisible() — if the message indicates a timeout/no matching
element (i.e., the locator never matched) then call test.skip(), otherwise
rethrow the error so real failures surface; update the code around the
validationError locator, the await expect(validationError).toBeVisible() call,
and the test.skip() invocation accordingly.
- Around line 67-76: The navLink selector (variable navLink) is too generic and
uses a regex (/home|dashboard|facility/i) that doesn't match any links on the
facility encounter page; update the selector to target a sidebar link that
actually exists on that route (e.g., the facility "Overview" link) by using
getByRole("link", { name: /overview/i }) or an equivalent exact label for the
FacilityNav overview link (remove .first() if not needed) so the click finds a
visible element and the unsaved changes dialog assertion no longer times out.
| // Wait for validation to produce an error (deterministic wait) | ||
| const validationError = page.getByText(/required|invalid|error/i).first(); | ||
| await expect(validationError) | ||
| .toBeVisible({ timeout: 5_000 }) | ||
| .catch( | ||
| // If no validation error appears (form has no required fields), | ||
| // the form may have submitted successfully — skip this test case | ||
| () => test.skip(), | ||
| ); |
There was a problem hiding this comment.
.catch(() => test.skip()) swallows all errors indiscriminately
Any rejection from expect().toBeVisible() — network timeout, infrastructure failure, unexpected DOM state — is silently converted to test.skip(), masking real failures as "no required fields". Additionally, calling test.skip() inside a .catch() callback is not officially documented by Playwright; SkipError propagation relies on implementation details that may change across versions.
🐛 Proposed fix — explicit conditional skip
- // Wait for validation to produce an error (deterministic wait)
- const validationError = page.getByText(/required|invalid|error/i).first();
- await expect(validationError)
- .toBeVisible({ timeout: 5_000 })
- .catch(
- // If no validation error appears (form has no required fields),
- // the form may have submitted successfully — skip this test case
- () => test.skip(),
- );
+ // Confirm a validation error appeared; skip only when the form has no
+ // required fields (i.e., submission succeeded and dirty state was correctly cleared).
+ const validationError = page.getByText(/required|invalid|error/i).first();
+ const hasValidationError = await validationError
+ .isVisible({ timeout: 5_000 })
+ .catch(() => false);
+ if (!hasValidationError) {
+ test.skip();
+ return;
+ }
+ await expect(validationError).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/patient/questionnaire-dirty-state.spec.ts` around lines 56 -
64, The current use of .catch(() => test.skip()) on await
expect(validationError).toBeVisible() swallows all errors; wrap the await in a
try/catch instead, and in the catch inspect the thrown Error from
expect(...).toBeVisible() — if the message indicates a timeout/no matching
element (i.e., the locator never matched) then call test.skip(), otherwise
rethrow the error so real failures surface; update the code around the
validationError locator, the await expect(validationError).toBeVisible() call,
and the test.skip() invocation accordingly.
| const navLink = page | ||
| .getByRole("link", { name: /home|dashboard|facility/i }) | ||
| .first(); | ||
| await expect(navLink).toBeVisible(); | ||
| await navLink.click(); | ||
|
|
||
| // The unsaved changes dialog should appear | ||
| await expect(page.getByText(/unsaved changes/i)).toBeVisible({ | ||
| timeout: 5_000, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the test file
find . -name "questionnaire-dirty-state.spec.ts" -type fRepository: ohcnetwork/care_fe
Length of output: 120
🏁 Script executed:
# Once found, read the test file around lines 67-76
cat -n tests/facility/patient/questionnaire-dirty-state.spec.ts | head -100Repository: ohcnetwork/care_fe
Length of output: 3512
🏁 Script executed:
# Search for encounter/questionnaire update page implementations
fd -t f "encounter.*update\|questionnaire.*form" --type ts --type tsx | grep -v node_modules | head -20Repository: ohcnetwork/care_fe
Length of output: 231
🏁 Script executed:
# Find where breadcrumbs or navigation links are rendered
rg "getByRole.*navigation\|Breadcrumb\|BreadCrumb" --type=tsx -g "!node_modules" -l | head -10Repository: ohcnetwork/care_fe
Length of output: 89
🏁 Script executed:
# Check for useNavigationPrompt usage in questionnaire/encounter forms
rg "useNavigationPrompt\|navigationPrompt" --type=ts --type=tsx -g "!node_modules" -B 2 -A 2 | head -50Repository: ohcnetwork/care_fe
Length of output: 89
🏁 Script executed:
# Search for "Update Encounter" route/page
rg "Update Encounter" --type-list | head -5
rg "Update Encounter" -g "!node_modules" -B 2 -A 2 | head -50Repository: ohcnetwork/care_fe
Length of output: 3524
🏁 Script executed:
# Find QuestionnaireForm or encounter update page
rg -l "QuestionnaireForm\|UpdateEncounter" -g "!node_modules" --max-count=20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Search for navigation/breadcrumb rendering
rg "getByRole.*navigation\|Breadcrumb" -g "!node_modules" -l | head -15Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check for useNavigationPrompt in the codebase
rg "useNavigationPrompt" -g "!node_modules" -B 3 -A 5Repository: ohcnetwork/care_fe
Length of output: 4757
🏁 Script executed:
# Find the encounter route definition
rg "encounters.*update\|encounter.*update" -g "!node_modules" -i -B 2 -A 2 | head -60Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Find encounter-related routes
rg "encounters.*update\|/facility.*encounter" -g "!node_modules" -i -l | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Find where the "Update Encounter" link is rendered
rg "Update Encounter" -g "!node_modules" -B 5 -A 5 | grep -E "\.(tsx|ts):" | head -30Repository: ohcnetwork/care_fe
Length of output: 554
🏁 Script executed:
# Look for the encounter show/view page
find . -path ./node_modules -prune -o -name "*ncounter*.tsx" -type f -print | head -20Repository: ohcnetwork/care_fe
Length of output: 1200
🏁 Script executed:
# Check src/pages structure for encounter pages
find ./src/pages -name "*ncounter*" -o -name "*Encounter*" | head -20Repository: ohcnetwork/care_fe
Length of output: 1062
🏁 Script executed:
# Get structure of Facility pages
find ./src/pages/Facility -type f -name "*.tsx" | grep -i encounter | head -20Repository: ohcnetwork/care_fe
Length of output: 332
🏁 Script executed:
# Check EncounterShow.tsx to understand the page structure
cat -n ./src/pages/Encounters/EncounterShow.tsx | head -150Repository: ohcnetwork/care_fe
Length of output: 6257
🏁 Script executed:
# Search for breadcrumb or navigation elements in EncounterShow
rg "Breadcrumb\|navigation\|getByRole.*link" ./src/pages/Encounters/EncounterShow.tsx -A 2 -B 2Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if there are navigation/breadcrumb components used in encounter pages
rg "Breadcrumb\|NavBar\|Header" ./src/pages/Encounters -g "*.tsx" -lRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Look at EncounterQuestionnaire component since that's likely where questionnaire is rendered
cat -n ./src/components/Patient/EncounterQuestionnaire.tsx | head -100Repository: ohcnetwork/care_fe
Length of output: 3360
🏁 Script executed:
# Check QuestionnaireForm to see what page/layout it's embedded in
cat -n ./src/components/Questionnaire/QuestionnaireForm.tsx | head -100Repository: ohcnetwork/care_fe
Length of output: 4318
🏁 Script executed:
# Check PatientHeader to see what links/navigation it renders
cat -n ./src/components/Patient/PatientHeader.tsx | head -200Repository: ohcnetwork/care_fe
Length of output: 4369
🏁 Script executed:
# Check Page component to see if it renders breadcrumbs or navigation
rg "Page.*component\|export.*Page" ./src/components/Common/Page.tsx -A 20 | head -50Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if there's a breadcrumb or navigation wrapper
rg "Breadcrumb\|breadcrumb\|Navigation" ./src/components -l | head -10Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Look for all links rendered on the page containing "facility" or "dashboard" text
rg "facility\|dashboard\|home" ./src/components/Common -i -lRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check what BackButton does - it might be the key to understanding navigation
cat -n ./src/components/Common/BackButton.tsxRepository: ohcnetwork/care_fe
Length of output: 913
🏁 Script executed:
# Look for the rest of QuestionnaireForm to see all the navigation elements
cat -n ./src/components/Questionnaire/QuestionnaireForm.tsx | sed -n '100,200p'Repository: ohcnetwork/care_fe
Length of output: 4174
🏁 Script executed:
# Check if QuestionnaireForm renders BackButton or any other navigation links
rg "BackButton\|Link\|navigate\|href" ./src/components/Questionnaire/QuestionnaireForm.tsx -n | head -30Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Look for where "home" or "dashboard" text might appear on the questionnaire page
rg "home\|dashboard" ./src/components/Questionnaire -i -A 2 -B 2 | head -50Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the Page component and common layouts to see if they render breadcrumbs/nav
cat -n ./src/components/Common/Page.tsxRepository: ohcnetwork/care_fe
Length of output: 1384
🏁 Script executed:
# Check if there's a sidebar or main layout that renders navigation with these labels
rg "getByRole.*link.*home\|getByRole.*link.*dashboard\|getByRole.*link.*facility" ./src -i -lRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check for sidebar or layout components that render navigation
find ./src/components -name "*Sidebar*" -o -name "*Layout*" -o -name "*Navigation*" | head -20Repository: ohcnetwork/care_fe
Length of output: 298
🏁 Script executed:
# Look at the main app layout/structure
rg "Sidebar\|navbar\|nav" ./src/components -i -l | head -10Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the App or index component structure
find ./src -maxdepth 2 -name "App.*" -o -name "index.*" -o -name "main.*"Repository: ohcnetwork/care_fe
Length of output: 166
🏁 Script executed:
# Search for any actual links with text containing "facility", "home", or "dashboard"
rg "name.*=.*facility|name.*=.*dashboard|name.*=.*home" ./src -i -g "*.tsx" | head -20Repository: ohcnetwork/care_fe
Length of output: 2600
🏁 Script executed:
# Check if there are any BreadCrumb-like components or navigation within encounter pages
rg "Link.*facility|Link.*dashboard|Link.*home" ./src/pages/Encounters -iRepository: ohcnetwork/care_fe
Length of output: 315
🏁 Script executed:
# Let me check what links are actually on the encounter form page by looking at EncounterQuestionnaire more carefully
cat -n ./src/components/Patient/EncounterQuestionnaire.tsx | grep -E "Link|href|navigate|<a|button" -iRepository: ohcnetwork/care_fe
Length of output: 242
🏁 Script executed:
# Check the main App.tsx to see the overall layout structure
cat -n ./src/App.tsxRepository: ohcnetwork/care_fe
Length of output: 3130
🏁 Script executed:
# Check if there's a sidebar component that might render navigation with these links
find ./src/components -name "*sidebar*" -o -name "*Sidebar*" -type fRepository: ohcnetwork/care_fe
Length of output: 165
🏁 Script executed:
# Let me check what the actual layout looks like - look for components that might wrap pages
rg "Sidebar\|Navigation\|Header" ./src/components -i -l | head -15Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if QuestionnaireForm renders any links internally
cat -n ./src/components/Questionnaire/QuestionnaireForm.tsx | sed -n '1,500p' | grep -E "Link|href|<a|navigate|button" -iRepository: ohcnetwork/care_fe
Length of output: 436
🏁 Script executed:
# Let me search for where facility/home/dashboard links might be in a breadcrumb or sidebar
rg "role=\"navigation\"|role=\"link\"" ./src -g "*.tsx" -B 2 -A 2 | head -60Repository: ohcnetwork/care_fe
Length of output: 746
🏁 Script executed:
# Check the app-sidebar to see what navigation links it renders
cat -n ./src/components/ui/sidebar/app-sidebar.tsx | head -200Repository: ohcnetwork/care_fe
Length of output: 7899
🏁 Script executed:
# Check the main sidebar component
cat -n ./src/components/ui/sidebar.tsx | head -100Repository: ohcnetwork/care_fe
Length of output: 3750
🏁 Script executed:
# Look for where navigation items are defined
rg "facility\|dashboard\|home" ./src/components/ui/sidebar -i -g "*.tsx" -B 2 -A 2 | head -80Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check for a breadcrumb component being used in encounter or questionnaire pages
rg "Breadcrumb" ./src/components/Patient -i -lRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Look at the test more carefully - what page state is it in when trying to find the link?
sed -n '10,30p' ./tests/facility/patient/questionnaire-dirty-state.spec.tsRepository: ohcnetwork/care_fe
Length of output: 857
🏁 Script executed:
# Let me check what actually renders on the Update Encounter page
rg "Update Encounter" ./src -g "*.tsx" -B 5 -A 5 | head -100Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check PatientNav to see what links it renders
rg "PatientNav" ./src/components -lRepository: ohcnetwork/care_fe
Length of output: 190
🏁 Script executed:
# Find the PatientNav file
find ./src/components -name "*PatientNav*"Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check FacilityNav to see what navigation options it provides
find ./src/components -path "*sidebar*" -name "*facility*nav*"Repository: ohcnetwork/care_fe
Length of output: 115
🏁 Script executed:
# Let me verify: does the sidebar Link component use raviger?
grep -n "from.*raviger" ./src/components/ui/sidebar/app-sidebar.tsxRepository: ohcnetwork/care_fe
Length of output: 114
🏁 Script executed:
# Check if there's a specific "facility" or "dashboard" link in the sidebar
rg "href.*facility|href.*dashboard|href.*home" ./src/components/ui/sidebar -i | head -20Repository: ohcnetwork/care_fe
Length of output: 186
🏁 Script executed:
# Check PatientNav to see what links it renders
cat -n ./src/components/ui/sidebar/patient-nav.tsx | head -100Repository: ohcnetwork/care_fe
Length of output: 2035
🏁 Script executed:
# Check what facility-nav renders
cat -n ./src/components/ui/sidebar/facility/facility-nav.tsx | head -150Repository: ohcnetwork/care_fe
Length of output: 5204
🏁 Script executed:
# Now let's understand: the test is navigating through facility → encounters → patient
# Let's check what sidebar is shown in encounter pages
rg "sidebarFor\|AppSidebar" ./src/pages/Encounters -i -B 2 -A 2 | head -50Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the routes to understand the full path context
cat -n ./src/Routers/index.tsx | head -150Repository: ohcnetwork/care_fe
Length of output: 345
🏁 Script executed:
# Find where the encounter page is rendered and with what sidebar
rg "EncounterShow\|EncounterQuestionnaire" ./src/Routers -B 5 -A 5 | head -60Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if there's a breadcrumb component on encounter pages
rg "Breadcrumb" ./src/pages/Encounters -g "*.tsx" -iRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Let me verify the actual text of the sidebar dashboard link
rg "view_dashboard" ./src -g "*.json" -B 1 -A 1Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Now let's verify: does the sidebar render on the encounter questionnaire page?
# Check AppRouter for the route pattern
cat -n ./src/Routers/AppRouter.tsx | grep -i "encounter\|questionnaire" -A 3 -B 3 | head -100Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if the sidebar is visible on questionnaire/encounter pages by looking at AppRouter
cat -n ./src/Routers/AppRouter.tsx | head -300Repository: ohcnetwork/care_fe
Length of output: 7456
🏁 Script executed:
# Now let's understand what links actually appear on the encounter questionnaire page
# The page renders EncounterQuestionnaire component which has:
# - PatientHeader
# - QuestionnaireForm
# And the sidebar renders FacilityNav
# Let's check: does the PatientHeader render any links?
rg "Link\|href" ./src/components/Patient/PatientHeader.tsxRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Let's check what text the facility overview link would have
rg "t\(\"overview\"\)" ./src/components/ui/sidebar/facility/facility-nav.tsx -B 3 -A 3Repository: ohcnetwork/care_fe
Length of output: 216
🏁 Script executed:
# Now the critical question: does clicking a raviger Link trigger useNavigationPrompt?
# Let's check the raviger documentation/usage by examining how it's used
rg "useNavigationPrompt" ./src -g "*.tsx" -B 5 -A 2 | head -80Repository: ohcnetwork/care_fe
Length of output: 4111
🏁 Script executed:
# Verify the sidebar is on encounter questionnaire pages - check if path matches exclusions
python3 << 'EOF'
import re
# Paths where sidebar is hidden
paths_without_sidebar = [
"/",
"/login",
"/session-expired",
r"^\/facility\/[^/]+\/service_requests\/[^/]+$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/service_requests\/[^/]+$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/internal_transfers\/to_receive\/[^/]+$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/internal_transfers\/to_dispatch\/[^/]+$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/internal_transfers\/create_delivery$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/internal_transfers\/requests\/[^/]+$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/internal_transfers\/requests\/[^/]+\/edit$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/external_supply\/purchase_orders\/new$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/external_supply\/purchase_orders\/[^/]+\/edit$",
r"^\/facility\/[^/]+\/locations\/[^/]+\/external_supply\/deliveries\/[^/]+$",
r"^\/facility\/[^/]+\/queues\/[^/]+\/tokens\/[^/]+$",
]
# Test path - encounter questionnaire
test_path = "/facility/123/patient/456/encounter/789/questionnaire"
sidebar_hidden = False
for path in paths_without_sidebar:
if isinstance(path, str):
if path == test_path:
sidebar_hidden = True
break
else:
if re.match(path, test_path):
sidebar_hidden = True
break
print(f"Path: {test_path}")
print(f"Sidebar hidden: {sidebar_hidden}")
print(f"Sidebar visible: {not sidebar_hidden}")
EOFRepository: ohcnetwork/care_fe
Length of output: 165
🏁 Script executed:
# Check what text would appear in sidebar links on a facility-based encounter page
# The regex is /home|dashboard|facility/i
# Let's see what would actually match
rg "name.*t\(" ./src/components/ui/sidebar/facility/facility-nav.tsx | head -20Repository: ohcnetwork/care_fe
Length of output: 678
🏁 Script executed:
# Check what the English translations are for the navigation links
rg "overview|appointments|queues|patients|services|resource|users|billing|settings" ./public/locale/en.json | head -20Repository: ohcnetwork/care_fe
Length of output: 1361
🏁 Script executed:
# Check if there's any link that would match "facility", "home", or "dashboard"
python3 << 'EOF'
import re
# The regex from the test
test_regex = re.compile(r"home|dashboard|facility", re.IGNORECASE)
# Sidebar link text examples (from English translations)
sidebar_texts = [
"Overview",
"Appointments",
"Queues",
"Patients",
"Services",
"Resource",
"Users",
"Billing",
"Settings",
"Search Patients",
"All Encounters",
"Locations",
]
matches = []
for text in sidebar_texts:
if test_regex.search(text):
matches.append(text)
print("Sidebar links that match /home|dashboard|facility/i:")
for match in matches:
print(f" - {match}")
if not matches:
print(" [NONE - regex won't match any sidebar links on encounter page]")
EOFRepository: ohcnetwork/care_fe
Length of output: 179
🏁 Script executed:
# Let's also check: are there any breadcrumbs or other UI elements that might render "Facility"?
rg "Facility.*Link\|Link.*Facility" ./src/components -i -g "*.tsx" | head -10Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the actual en.json to see what text appears
cat ./public/locale/en.json | grep -o '"overview":\|"appointments":\|"facility":' | head -10Repository: ohcnetwork/care_fe
Length of output: 101
🏁 Script executed:
# Check what "view_dashboard" translates to
rg '"view_dashboard"' ./public/locale/en.jsonRepository: ohcnetwork/care_fe
Length of output: 99
🏁 Script executed:
# Check if there are any breadcrumbs or other elements that render "facility" text
rg "facility" ./src/pages/Encounters -i -g "*.tsx" | grep -i "text\|name\|label" | head -10Repository: ohcnetwork/care_fe
Length of output: 450
🏁 Script executed:
# Let's trace through: when on a facility encounter page, what sidebar renders?
# Looking at app-sidebar.tsx, if facilitySidebar is true, it renders FacilityNav
# FacilityNav renders links with names from t() function calls
# Let's check the complete link names from facility-nav
python3 << 'EOF'
import re
# Extract all the t() calls from facility-nav.tsx
with open('./src/components/ui/sidebar/facility/facility-nav.tsx', 'r') as f:
content = f.read()
# Find all t("...") calls
pattern = r't\("([^"]+)"\)'
matches = re.findall(pattern, content)
print("Translation keys used in FacilityNav:")
for match in sorted(set(matches)):
print(f" - {match}")
EOFRepository: ohcnetwork/care_fe
Length of output: 588
🏁 Script executed:
# Now check what those translate to in English
python3 << 'EOF'
import json
# Translation keys from FacilityNav
keys = ['overview', 'appointments', 'queues', 'patients', 'search_patients',
'all_encounters', 'encounter_class_encounters', 'locations', 'services',
'resource', 'users', 'billing', 'accounts', 'invoices', 'payments',
'settings', 'general', 'departments', 'locations', 'devices']
with open('./public/locale/en.json', 'r') as f:
translations = json.load(f)
# Test if any text contains "facility", "dashboard", or "home"
test_regex = r'facility|dashboard|home'
import re
regex = re.compile(test_regex, re.IGNORECASE)
print("English translations of FacilityNav links:")
matches = []
for key in keys:
if key in translations:
text = translations[key]
print(f" {key}: {text}")
if regex.search(text):
matches.append((key, text))
print("\nMatches for /facility|dashboard|home/i:")
if matches:
for key, text in matches:
print(f" {key}: {text}")
else:
print(" [NONE]")
EOFRepository: ohcnetwork/care_fe
Length of output: 645
🏁 Script executed:
# Also check: is there a breadcrumb or title that renders "Facility"?
rg "Facility\|facility" ./src/pages/Encounters/EncounterShow.tsx -B 2 -A 2 | head -30Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Finally, let's check what the dashboard link text is
python3 << 'EOF'
import json
with open('./public/locale/en.json', 'r') as f:
translations = json.load(f)
print("view_dashboard:", translations.get('view_dashboard'))
EOFRepository: ohcnetwork/care_fe
Length of output: 92
🏁 Script executed:
# Let me check if the "View Dashboard" link is actually visible on the encounter page
# by checking if the condition for showing it is met
cat -n ./src/components/ui/sidebar/app-sidebar.tsx | sed -n '133,157p'Repository: ohcnetwork/care_fe
Length of output: 1317
🏁 Script executed:
# Let me also check if there are any other elements that render "facility" or "home" or "dashboard"
rg "home|dashboard" ./src/pages/Encounters -i -g "*.tsx" | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Let me check if there's a back/cancel button or other navigation on the questionnaire form
rg "BackButton\|onCancel\|goBack" ./src/components/Questionnaire/QuestionnaireForm.tsx -B 2 -A 2 | head -50Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check what the onCancel handler does in EncounterQuestionnaire
grep -n "onCancel" ./src/components/Patient/EncounterQuestionnaire.tsx -B 2 -A 2Repository: ohcnetwork/care_fe
Length of output: 191
The navLink selector won't match any visible elements on the encounter questionnaire page, causing the test to fail with a timeout
The regex /home|dashboard|facility/i with .first() won't find any navigation links on this page. When on a facility-based encounter route, the sidebar renders only the FacilityNav links (Overview, Appointments, Queues, Patients, Services, Resource, Users, Billing, Settings) — none of which match the regex. The "View Dashboard" link only appears when not on a facility route.
Use a specific sidebar link that's actually rendered on the encounter page, such as the facility overview link:
🐛 Proposed fix — target a specific rendered sidebar link
- // Navigate away using an in-app link to trigger the raviger guard
- const navLink = page
- .getByRole("link", { name: /home|dashboard|facility/i })
- .first();
- await expect(navLink).toBeVisible();
+ // Navigate away via the facility overview sidebar link (raviger, triggers guard)
+ const navLink = page
+ .getByRole("link", { name: /overview/i })
+ .first();
+ await expect(navLink).toBeVisible({ timeout: 5_000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/patient/questionnaire-dirty-state.spec.ts` around lines 67 -
76, The navLink selector (variable navLink) is too generic and uses a regex
(/home|dashboard|facility/i) that doesn't match any links on the facility
encounter page; update the selector to target a sidebar link that actually
exists on that route (e.g., the facility "Overview" link) by using
getByRole("link", { name: /overview/i }) or an equivalent exact label for the
FacilityNav overview link (remove .first() if not needed) so the click finds a
visible element and the unsaved changes dialog assertion no longer times out.
adarsh-priydarshi-5646
left a comment
There was a problem hiding this comment.
@KshitijShukla30 Please do not raise a PR unless an issue has been assigned to you.
Proposed Changes
Fixes #15855
setIsDirty(false)from the beginning ofhandleSubmitto thesubmitBatchmutation'sonSuccesscallback.useNavigationPromptguard remains active during client-side validation failures and server-side submission errors.This change preserves existing happy-path behavior while preventing unintended data loss.
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
questionnaire-dirty-state.spec.ts— verifies navigation guard stays active after failed submission)Summary by CodeRabbit