Skip to content

feat: 86extdm6t #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rachelishteren wants to merge 1 commit into
base: main
Choose a base branch
from
+23 −14
Open

feat: 86extdm6t #89

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions src/pages/home/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { useState } from 'react'
import { useQuery } from '@tanstack/react-query'
import { ProgressCard } from '@/pages/home/progress/ProgressCard'
import { BottomSheetDialog } from '@/components/BottomSheetDialog/BottomSheetDialog'
import type { Appointment } from '@/components/BottomSheetDialog/types'
import { DailyTips } from './dailyTips/DailyTips'
import { CheckIn } from './checkIn/CheckIn'
import { HomeButtomDesktop, HomeButtomMobile, HomeTopDesktop, HomeTopMobile } from './styles'
Expand All @@ -10,17 +9,27 @@ import { SGLTypography } from '@/components/UI/Typography/SGLTypography'
import { useTranslation } from 'react-i18next'
import { useIsMobile } from '@/hooks/useIsMobile'
import { Welcome } from './welcome/Welcome'
import { apiService } from '@/services/api/api.service'
import { useState } from 'react'
import type { Appointment } from '@/components/BottomSheetDialog/types'

export const Home = () => {
const [selectedAppointment, setSelectedAppointment] = useState<Appointment | undefined>({
appointmentId: '1',
patientId: 1622017,
time: '10:12:50',
date: '2026-05-15',
chamber: 'appointment.chamber.hyperbaric',
chairNumber: 9,
treatmentNumber: 2,
status: 'confirmed',
const [selectedAppointment, setSelectedAppointment] = useState<boolean>(true)

@coderabbitai coderabbitai Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Dialog auto-opens on mount due to initial true state.

Initializing selectedAppointment to true causes the dialog to attempt opening immediately when the component mounts, creating an auto-popup experience. Users typically expect dialogs to open in response to a deliberate action (e.g., clicking a button), not automatically on page load.

🔧 Recommended fix 
-  const [selectedAppointment, setSelectedAppointment] = useState<boolean>(true)
+  const [selectedAppointment, setSelectedAppointment] = useState<boolean>(false)

Then add a UI element (e.g., a button or card click handler) that calls setSelectedAppointment(true) when the user wants to view their appointment.

📝 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.

Suggested change
const [selectedAppointment, setSelectedAppointment] = useState<boolean>(true)
const [selectedAppointment, setSelectedAppointment] = useState<boolean>(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 `@src/pages/home/home.tsx` at line 17, The selectedAppointment state is
initialized to true, causing the dialog to auto-open when the component mounts
instead of waiting for user interaction. Change the initial value of the
selectedAppointment useState hook from true to false so the dialog does not open
automatically. Then add a UI element such as a button or clickable card with an
onClick handler that calls setSelectedAppointment(true) to allow users to
deliberately open the appointment dialog when they choose to view their
appointment.


const patientID = '1622017'
const date = '07-06-2026'
Comment on lines +19 to +20

@coderabbitai coderabbitai Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find how user/patient ID is accessed elsewhere in the codebase

# Search for user context, auth context, or patient context patterns
rg -n -C3 --type=tsx --type=ts 'useUser|useAuth|usePatient|user.*context|patient.*context' -i

# Search for existing patterns of getting current user/patient ID
rg -n -C3 --type=tsx --type=ts 'currentUser|userId|patientId|patient.*id' -i | head -50

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 135

🏁 Script executed:

# Read the home.tsx file to understand the context
cat -n src/pages/home/home.tsx | head -40

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 1789

🏁 Script executed:

# Search for user/auth context patterns with corrected rg syntax
rg -n -C3 'useUser|useAuth|usePatient|useContext.*user|useContext.*auth' -t ts -t tsx

# Also search for how patientId/userId is typically accessed
rg -n -C3 'patient.*id|user.*id|currentUser' -t ts -t tsx | head -50

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 135

🏁 Script executed:

# Check if there are any imports or hooks related to authentication in the codebase
fd -e ts -e tsx | xargs grep -l 'useAuth\|useUser\|user.*context' | head -20

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 149

🏁 Script executed:

# Read the useAuth hook to understand how to access user context
cat -n src/hooks/useAuth.ts

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 258

🏁 Script executed:

# Check ProtectedRoute to understand auth patterns
cat -n src/components/ProtectedRoute/ProtectedRoute.tsx

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 380

🏁 Script executed:

# Search for other uses of patientID in the codebase to understand patterns
rg -n 'patientID|patientId' --max-count=20

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 1163

🏁 Script executed:

# Look for user/session/auth context setup (try without type filters)
rg -n 'createContext|useContext|useState.*user|useState.*auth' | head -30

Repository: ColmanDevClubORG/Sagol360Management

Length of output: 61

Hardcoded test data must be replaced with dynamic user context and date logic.

These hardcoded values will cause all users to see the same appointment data (patient 1622017 on 07-06-2026), which is a critical data integrity and privacy issue. The authentication system is currently incomplete (see the TODO in useAuth.ts), but once it provides user context, patientID must be retrieved from the authenticated user instead of hardcoded here.

Before merging:

  • Implement proper user authentication in useAuth.ts to provide authenticated user/patient context
  • Replace the hardcoded patientID with the actual value from the authenticated user
  • Replace the hardcoded date with dynamic logic (e.g., current date or next available appointment date)
🤖 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 `@src/pages/home/home.tsx` around lines 19 - 20, Replace the hardcoded
patientID and date values in the home component with dynamic, user-specific data
to ensure data integrity and privacy. First, complete the authentication
implementation in useAuth.ts (address the existing TODO) to provide
authenticated user context. Then in home.tsx, replace the hardcoded patientID
constant with the actual patient identifier from the authenticated user returned
by useAuth, and replace the hardcoded date constant with dynamic logic such as
retrieving the current date or the next available appointment date. This ensures
each user sees only their own appointment data.

Comment on lines +19 to +20

@GilHeller GilHeller Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a comment for MOCK DATA


const { data: appointment } = useQuery({
queryKey: ['nextAppointment', patientID, date],
queryFn: async () => {
const response = await apiService.get<Appointment>('/api/appointment/nextAppointment', {

@GilHeller GilHeller Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the URL path should be a constant

params: {
patientID,
date,
},
})
return response
},
})
Comment on lines +22 to 33

@coderabbitai coderabbitai Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error and loading state handling for better user experience.

The query result is destructured to only data, ignoring isLoading and isError. If the API call is slow or fails, the user receives no feedback—the dialog simply never appears.

Consider destructuring and handling these states:

💡 Suggested enhancement 
-  const { data: appointment } = useQuery({
+  const { data: appointment, isLoading, isError, error } = useQuery({
     queryKey: ['nextAppointment', patientID, date],
     queryFn: async () => {
       const response = await apiService.get<Appointment>('/api/appointment/nextAppointment', {
         params: {
           patientID,
           date,
         },
       })
       return response
     },
   })
+
+  if (isLoading) {
+    return <div>Loading appointment...</div> // Or a proper loading skeleton
+  }
+
+  if (isError) {
+    console.error('Failed to fetch appointment:', error)
+    // Show error UI or toast notification
+  }
📝 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.

Suggested change
const { data: appointment } = useQuery({
queryKey: ['nextAppointment', patientID, date],
queryFn: async () => {
const response = await apiService.get<Appointment>('/api/appointment/nextAppointment', {
params: {
patientID,
date,
},
})
return response
},
})
const { data: appointment, isLoading, isError, error } = useQuery({
queryKey: ['nextAppointment', patientID, date],
queryFn: async () => {
const response = await apiService.get<Appointment>('/api/appointment/nextAppointment', {
params: {
patientID,
date,
},
})
return response
},
})
if (isLoading) {
return <div>Loading appointment...</div> // Or a proper loading skeleton
}
if (isError) {
console.error('Failed to fetch appointment:', error)
// Show error UI or toast notification
}
🤖 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 `@src/pages/home/home.tsx` around lines 22 - 33, The useQuery hook for fetching
the next appointment is only destructuring the data property and ignoring
isLoading and isError states. Modify the destructuring in the useQuery call to
also extract isLoading and isError along with data, then add conditional
rendering logic to display appropriate loading indicators when isLoading is true
and error messages or fallback UI when isError is true, ensuring the user
receives feedback during slow API calls or when the appointment fetch fails.

Comment on lines +22 to 33

@GilHeller GilHeller Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

manage the appointment logic inside the component. This component should tell us what we will see in the HomePage with minimal logic as possible


const { t } = useTranslation()
Expand All @@ -29,9 +38,9 @@ export const Home = () => {
return (
<>
<BottomSheetDialog
appointment={selectedAppointment}
isOpen={!!selectedAppointment}
onClose={() => setSelectedAppointment(undefined)}
appointment={appointment}
isOpen={selectedAppointment && !!appointment}
onClose={() => setSelectedAppointment(false)}

@GilHeller GilHeller Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

define a function outside the return and call it.

@GilHeller GilHeller Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

consider to implement it inside the component and allow override or add extra logic to the onClose.

/>
Comment on lines 40 to 44

@coderabbitai coderabbitai Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

No mechanism to reopen the dialog after closing.

Once the user closes the dialog, selectedAppointment is set to false (line 43), but there is no UI element in this component that allows the user to set it back to true. This makes the appointment information inaccessible until the page is refreshed, breaking a core user workflow.

🔧 Recommended solution

Add a clickable UI element (e.g., an appointment card or "View Appointment" button) that calls setSelectedAppointment(true) when clicked. For example:

<AppointmentCard 
  appointment={appointment}
  onClick={() => setSelectedAppointment(true)}
/>

or

{appointment && (
  <button onClick={() => setSelectedAppointment(true)}>
    View Next Appointment
  </button>
)}
🤖 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 `@src/pages/home/home.tsx` around lines 40 - 44, The BottomSheetDialog
component in the home component can be closed via the onClose handler which sets
selectedAppointment to false, but there is no UI element that allows reopening
it by setting selectedAppointment back to true. Add a clickable UI element (such
as an appointment card or button) that calls setSelectedAppointment(true) when
clicked to allow users to reopen the dialog and access the appointment
information again after closing it.

<div style={isMobile ? HomeTopMobile : HomeTopDesktop}>
<Welcome />
Expand Down
Loading