Skip to content

Survey details re-design #172

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
ihsankahveci wants to merge 3 commits into
base: kc-pit-2026-test
Choose a base branch
from
Open

Survey details re-design #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c479c0
Refactor survey initialization in edit mode and update SurveyDetails …
ihsankahveci Jan 26, 2026
203812d
Update client/src/pages/SurveyDetails/SurveyDetails.tsx
ihsankahveci Jan 26, 2026
20dd8d6
Fix performance issue: fetch single survey instead of all surveys
ihsankahveci Jan 26, 2026
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
31 changes: 20 additions & 11 deletions client/src/hooks/useApi.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,38 +352,47 @@ export const useApi = () => {
type SurveyWithUser = SurveyDocument & {
employeeName: string;
employeeId: string;
locationName: string;
};

const useSurveyWithUser = (
surveyObjectId: string
): SWRResponse<SurveyWithUser> => {
surveyObjectId: string | undefined
): SWRResponse<SurveyWithUser | null> => {
return useSWR(
surveyObjectId ? `/api/surveys/${surveyObjectId}` : null,
() => fetchSurveyWithUser(surveyObjectId)
() => fetchSurveyWithUser(surveyObjectId!)
);
};

const fetchSurveyWithUser = async (
surveyObjectId: string
): Promise<SurveyWithUser> => {
): Promise<SurveyWithUser | null> => {
const survey = await fetchAndDeserialize<SurveyDocument>(
`/api/surveys/${surveyObjectId}`
);
if (!survey) {
throw new Error(
'Survey not found or you do not have permission to view it'
);
return null;
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The error handling has been changed in a way that loses important information. Previously, the API would throw an error with a specific message ("Survey not found or you do not have permission to view it") that would be captured in the error variable from SWR. Now, the function returns null without distinguishing between a 404 (not found) and a 403 (no permission). This makes debugging more difficult and provides less helpful feedback. Consider checking the response status in fetchSurveyWithUser and throwing appropriate errors for different failure cases, or at minimum preserve the error information that SWR naturally provides.

Suggested change
return null;
throw new Error('Survey not found or you do not have permission to view it');

Copilot uses AI. Check for mistakes.
}
const user = await fetchAndDeserialize<UserDocument>(
`/api/users/${survey.createdByUserObjectId}`
);

// Fetch user and location in parallel for efficiency
const [user, location] = await Promise.all([
fetchAndDeserialize<UserDocument>(
`/api/users/${survey.createdByUserObjectId}`
),
survey.locationObjectId
? fetchAndDeserialize<LocationDocument>(
`/api/locations/${survey.locationObjectId}`
)
: Promise.resolve(null)
Comment on lines +381 to +386
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The parallel fetch of user and location doesn't handle potential errors gracefully. If either fetchAndDeserialize call fails (e.g., user or location not found), the entire Promise.all will reject, causing the whole function to fail even if the survey itself was retrieved successfully. Consider adding error handling within the Promise.all to catch individual fetch failures and return null for that specific field while still returning the survey data with "Unknown" values for failed fetches.

Suggested change
),
survey.locationObjectId
? fetchAndDeserialize<LocationDocument>(
`/api/locations/${survey.locationObjectId}`
)
: Promise.resolve(null)
).catch(() => null),
survey.locationObjectId
? fetchAndDeserialize<LocationDocument>(
`/api/locations/${survey.locationObjectId}`
).catch(() => null)
: Promise.resolve<LocationDocument | null>(null)

Copilot uses AI. Check for mistakes.
]);

return {
...survey,
employeeName: user
? `${user.firstName} ${user.lastName}`
: 'Unknown',
employeeId: user?._id ?? 'Unknown'
employeeId: user?._id ?? 'Unknown',
locationName: location?.hubName ?? 'Unknown'
};
};

Expand Down
8 changes: 7 additions & 1 deletion client/src/pages/Survey/utils/surveyUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ export const initializeSurvey = (
if (isEditMode) {
// Edit mode only uses first 3 pages: volunteer-pre-screen, consent, survey-validation
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The comment on line 19 is outdated and incorrect. It states "Edit mode only uses first 3 pages: volunteer-pre-screen, consent, survey-validation" but the code now includes pages 0-3 (4 pages total) plus pages 16 and 17 (giftCards and giftCards2). The comment should be updated to accurately reflect the new behavior: "Edit mode includes volunteer-pre-screen, age_check, consent, survey-validation, giftCards, and giftCards2 pages"

Suggested change
// Edit mode only uses first 3 pages: volunteer-pre-screen, consent, survey-validation
// Edit mode includes volunteer-pre-screen, age_check, consent, survey-validation, giftCards, and giftCards2 pages

Copilot uses AI. Check for mistakes.
surveyJson.title = 'Homelessness Experience Survey (Edit Mode)';
surveyJson.pages = surveyJson.pages.slice(0, 3);
// surveyJson.pages = surveyJson.pages.slice(0, 4);
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

There is commented-out code on line 21 that should be removed. Commented code in production codebase can cause confusion about intent and makes the code harder to maintain. If this line is no longer needed, it should be deleted entirely.

Suggested change
// surveyJson.pages = surveyJson.pages.slice(0, 4);

Copilot uses AI. Check for mistakes.
surveyJson.pages = [
...surveyJson.pages.slice(0, 4),
surveyJson.pages[16],
surveyJson.pages[17]
].filter(page => page !== undefined);
Comment on lines 19 to +26
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

Hardcoded page indices (16 and 17) are fragile and will break if pages are reordered in survey.json. The comment says "Edit mode only uses first 3 pages" but the code now includes 6 pages (0-3, plus 16-17). Consider using page names instead of indices for better maintainability, or update the comment to accurately reflect the new behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +22 to +26
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The code uses hardcoded indices 16 and 17 to access specific survey pages without validating that these indices exist in the array. While the filter removes undefined values, if the survey.json structure changes and these pages are moved or removed, this code will silently exclude them without any error or warning. Consider adding validation to ensure these pages exist, or better yet, select pages by their name property rather than hardcoded indices (e.g., surveyJson.pages.filter(page => ['volunteer-pre-screen', 'age_check', 'consent', 'survey-validation', 'giftCards', 'giftCards2'].includes(page.name))).

Copilot uses AI. Check for mistakes.

// Remove any early stop triggers to allow full editing of survey
// Without this, the survey will stop early if consent is revoked, not allowing any edits to consecutive pages
if (surveyJson.triggers) {
Expand Down
151 changes: 57 additions & 94 deletions client/src/pages/SurveyDetails/SurveyDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export default function SurveyDetails() {
const { surveyService } = useApi();
const {
data: survey,
isLoading: loading,
error
} = surveyService.useSurveyWithUser(id ?? '') || {};
isLoading: loading
} = surveyService.useSurveyWithUser(id) || {};
const error = !loading && !survey;
const navigate = useNavigate();
const qrRefs = useRef<(HTMLDivElement | null)[]>([]);
const ability = useAbility();
Expand All @@ -33,40 +33,23 @@ export default function SurveyDetails() {
const canEdit = survey
? ability.can(ACTIONS.CASL.UPDATE, subject(SUBJECTS.SURVEY, survey))
: false;
// Fields to display in survey responses section
const displayFields = [
'first_two_letters_fname',
'first_two_letters_lname',
'date_of_birth',
'email_phone_consent',
'email',
'phone'
];

const labelMap: Record<string, string> = {
first_two_letters_fname: 'First two letters of first name',
first_two_letters_lname: 'First two letters of last name',
year_born: 'Year born',
month_born: 'Month born',
location: 'Location',
interpreter: 'Using interpreter?',
language: 'Language (if using interpreter)',
phone_number: 'Phone number',
first_two_letters_fname: 'First name initials',
first_two_letters_lname: 'Last name initials',
date_of_birth: 'Date of Birth',
email_phone_consent: 'Email/Phone Consent',
email: 'Email',
email_consent: 'Consent to email',
age_for_consent: 'Age 18 or over?',
consent_given: 'Oral consent given?',
homeless_people_count:
'Number of people experiencing homelessness you know',
people_you_know: 'People you know experiencing homelessness',
sleeping_location_last_night: 'Sleeping Location Last Night',
homeless_duration_since_housing: 'Homeless Duration Since Housing',
homeless_occurrences_past_3_years: 'Homeless Occurrences Past 3 Years',
months_homeless: 'Months Homeless',
age: 'Age',
hispanic_latino: 'Hispanic/Latino',
veteran: 'Veteran',
fleeing_dv: 'Fleeing Domestic Violence',
disability: 'Disability',
mental_illness: 'Mental Illness',
substance_abuse: 'Substance Abuse',
city_lasthoused: 'City Last Housed',
minutes_traveled: 'Minutes Traveled',
events_conditions: 'Events/Conditions',
shelter_preferences: 'Shelter Preferences',
person_name: 'Name',
relationship: 'Relationship',
current_sleeping_location: 'Current Sleeping Location'
phone: 'Phone'
};

if (loading) return <p>Loading...</p>;
Expand Down Expand Up @@ -119,17 +102,53 @@ export default function SurveyDetails() {

<div className="survey-info">
<p>
<strong>Employee ID:</strong> {survey.employeeId}
<strong>Survey Code:</strong> {survey.surveyCode}
</p>
<p>
<strong>Staff Name:</strong> {survey.employeeName}
</p>
<p>
<strong>Employee Name:</strong> {survey.employeeName}
<strong>Location:</strong> {survey.locationName}
</p>
<p>
<strong>Submitted At:</strong>{' '}
<strong>Date and Time:</strong>{' '}
{new Date(survey.createdAt).toLocaleString()}
</p>
</div>

{/* Gift Card Information */}
<div className="responses-section">
<h3>Gift Card Information</h3>
<pre>
{survey.responses &&
displayFields
.filter(field => field in survey.responses)
.map(field => {
const answer = survey.responses[field];
const label = labelMap[field] || field;
return `${label}: ${answer ?? 'N/A'}`;
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The field 'email_phone_consent' is a checkbox type that returns an array of values (either ['email'], ['phone'], ['email', 'phone'], or an empty array/null). The current display code treats it as a simple value and will display it as comma-separated array items or "[object Object]" rather than in a user-friendly format. Consider adding special handling to display this as "Email, Phone", "Email", "Phone", or "None" based on the array contents.

Suggested change
return `${label}: ${answer ?? 'N/A'}`;
let formattedAnswer: string;
if (field === 'email_phone_consent') {
const value = answer as unknown;
if (
!value ||
(Array.isArray(value) && value.length === 0)
) {
formattedAnswer = 'None';
} else if (Array.isArray(value)) {
const parts: string[] = [];
if (value.includes('email')) {
parts.push('Email');
}
if (value.includes('phone')) {
parts.push('Phone');
}
formattedAnswer = parts.join(', ') || 'None';
} else {
formattedAnswer = String(value);
}
} else {
formattedAnswer = String(answer ?? 'N/A');
}
return `${label}: ${formattedAnswer}`;

Copilot uses AI. Check for mistakes.
})
.join('\n\n')}
</pre>
</div>
{/* Edit Pre-screen Questions Button */}
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The comment on line 134 is outdated. It says "Edit Pre-screen Questions Button" but the button text has been changed to "Edit Gift Card Information" and its purpose has been updated to edit gift card related pages. The comment should be updated to reflect this: "Edit Gift Card Information Button"

Suggested change
{/* Edit Pre-screen Questions Button */}
{/* Edit Gift Card Information Button */}

Copilot uses AI. Check for mistakes.
<Tooltip
title={
!canEdit
? "You don't have permission to edit this survey"
: ''
}
>
<span>
<button
className="edit-button"
disabled={!canEdit}
onClick={() => navigate(`/survey/${id}/edit`)}
>
Edit Gift Card Information
</button>
</span>
</Tooltip>
{/* Coupon Code Information */}
<div className="referral-info">
<h3>Referral Information</h3>
Comment on lines 152 to 154
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

Inconsistent terminology: The comment says "Coupon Code Information" but the actual heading in the UI is "Referral Information" and uses terms like "Referred By Code" and "Generated Referral Codes". For consistency with the terminology change from "referral" to "coupon" throughout this PR, consider updating this section to use "Coupon" terminology (e.g., "Coupon Information", "Received From Coupon Code", "Generated Coupon Codes").

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -198,62 +217,6 @@ export default function SurveyDetails() {
</button>
</div>
</div>

{/* Survey Responses */}
<div className="responses-section">
<h3>Survey Responses</h3>
<pre>
{survey.responses &&
Object.entries(survey.responses)
.map(([question, answer]) => {
const label =
labelMap[question] || question;

if (
question === 'people_you_know' &&
Array.isArray(answer)
) {
return (
`\n${label}:\n` +
answer
.map((person, index) => {
return (
` Person ${index + 1}:\n` +
Object.entries(person)
.map(
([key, val]) =>
` ${key}: ${val}`
)
.join('\n')
);
})
.join('\n\n')
);
} else {
return `${label}: ${answer}`;
}
})
.join('\n\n')}
</pre>
</div>
{/* Edit Pre-screen Questions Button */}
<Tooltip
title={
!canEdit
? "You don't have permission to edit this survey"
: ''
}
>
<span>
<button
className="edit-button"
disabled={!canEdit}
onClick={() => navigate(`/survey/${id}/edit`)}
>
Edit Prescreen Responses
</button>
</span>
</Tooltip>
</div>
</div>
);
Expand Down
1 change: 0 additions & 1 deletion client/src/styles/SurveyDetailsCss.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ h3 {

/* Edit Pre-Screen Responses Button */
.edit-button {
width: 100%;
padding: 14px;
background-color: #3e236e;
color: white;
Expand Down
Loading