Skip to content

Fix assessment detail questionnaire progress and source labels#7571

Merged
lucanovera merged 4 commits intomainfrom
misc-assessment-fixes
Mar 5, 2026
Merged

Fix assessment detail questionnaire progress and source labels#7571
lucanovera merged 4 commits intomainfrom
misc-assessment-fixes

Conversation

@galvana
Copy link
Copy Markdown
Contributor

@galvana galvana commented Mar 5, 2026

Summary

  • Fix questionnaire progress bar showing 0/11 instead of actual answered/total counts by using API response data directly instead of broken getSlackQuestions utility
  • Add team_input variant to frontend AnswerSource enum so Slack-answered questions display the "Team input" label correctly
  • Remove unused utils.ts file that contained the broken filtering logic

Test plan

  • Verify questionnaire progress bar shows correct counts (e.g., 3/5 answered)
  • Verify Slack-answered questions show "Team input" source label
  • Verify no TypeScript build errors

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added "Team input" as a distinct answer source and renamed previous "Team input" label to "User input"; tag coloring updated.
  • Refactor

    • Question status counts in assessment detail now come from questionnaire-provided totals.
    • Removed legacy Slack-specific handling and reduced exported utilities surface.
  • Bug Fixes

    • Assessment questions now display in a consistent, defined order.

- Use questionnaire API response for progress counts instead of
  recomputing from question_groups (which excluded answered questions)
- Add team_input to AnswerSource enum so Slack answers show labels
- Remove broken getSlackQuestions utility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@galvana galvana requested a review from a team as a code owner March 5, 2026 04:38
@galvana galvana requested review from jpople and removed request for a team March 5, 2026 04:38
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Mar 5, 2026 4:36pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 5, 2026 4:36pm

Request Review

@galvana galvana changed the base branch from main to celery-worker-hot-reload March 5, 2026 04:39
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b4532fbc-658a-494a-bb70-362cba4b2653

📥 Commits

Reviewing files that changed from the base of the PR and between 95c1f8b and 5b0842f.

📒 Files selected for processing (4)
  • clients/admin-ui/src/features/privacy-assessments/index.ts
  • clients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts
  • clients/admin-ui/src/features/privacy-assessments/types.ts
  • clients/admin-ui/src/features/privacy-assessments/utils.ts
💤 Files with no reviewable changes (1)
  • clients/admin-ui/src/features/privacy-assessments/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • clients/admin-ui/src/features/privacy-assessments/types.ts
  • clients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts

📝 Walkthrough

Walkthrough

Removed Slack-specific helper and memoization; introduced a TEAM_INPUT answer source (constants/types); switched QuestionnaireStatusBar to use assessment.questionnaire counts; removed a public utils re-export; changed update-answer API route; added ordering to AssessmentTemplate.questions.

Changes

Cohort / File(s) Summary
UI: Assessment detail
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Removed Slack-specific import and local memoization; now uses assessment.questionnaire.answered_questions and total_questions for status counts.
Constants & Types
clients/admin-ui/src/features/privacy-assessments/constants.ts, clients/admin-ui/src/features/privacy-assessments/types.ts
Added TEAM_INPUT to AnswerSource; renamed USER_INPUT label to "User input"; added tag color mapping for TEAM_INPUT.
Utilities & Public API
clients/admin-ui/src/features/privacy-assessments/utils.ts, clients/admin-ui/src/features/privacy-assessments/index.ts
Removed getSlackQuestions helper and removed its re-export from the module index; added formatSystems and formatTypes utilities.
Redux slice / API route
clients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts
Updated endpoints: .../answers/${questionId}.../questions/${questionId} for single and bulk update routes.
Backend model ordering
src/fides/api/models/privacy_assessment.py
Added order_by="AssessmentQuestion.group_order, AssessmentQuestion.question_order" to AssessmentTemplate.questions relationship.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐇 I scurried past Slack's old trail,
A TEAM_INPUT hops into the tale,
Counts now read from questionnaire art,
Routes and order play a tidy part,
Tiny paws, a tidy grail. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes a clear summary of changes and test plan, but is missing required template sections like Ticket number, Code Changes list, and Pre-Merge Checklist. Fill out the complete PR template including Ticket number, detailed Code Changes list, and all Pre-Merge Checklist items to meet repository requirements.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main changes: fixing the questionnaire progress bar and adding the team_input source label to display correct labels.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch misc-assessment-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR fixes two display bugs in the privacy assessment detail questionnaire view: the progress bar was always showing 0/N because getSlackQuestions was incorrectly filtering by AnswerStatus.NEEDS_INPUT (pending questions) rather than Slack-answered ones, and Slack-answered questions were missing a "Team input" label because the team_input API value had no corresponding frontend enum variant.

Key changes:

  • Replaced the broken client-side getSlackQuestions filtering logic with direct use of assessment.questionnaire.answered_questions / total_questions from the API response — the correct, authoritative source of truth
  • Added TEAM_INPUT = "team_input" to the AnswerSource enum and wired it into the label/colour constant maps so Slack-answered questions display correctly
  • Deleted the now-unused utils.ts file and its barrel export to keep the module clean

Confidence Score: 5/5

  • Safe to merge — targeted, well-scoped bug fixes with no regressions identified.
  • The changes are minimal and correct: the non-null assertion on assessment.questionnaire! is protected by the questionnaireSentAt render guard (which is only truthy when questionnaire is non-null), the new enum variant aligns with the documented backend API value, and the deleted utility had demonstrably wrong filtering logic. No new complexity is introduced.
  • No files require special attention.

Important Files Changed

Filename Overview
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx Replaced broken client-side getSlackQuestions filtering with direct API response fields (answered_questions/total_questions). The non-null assertion on assessment.questionnaire! is safe because the render guard questionnaireSentAt is only truthy when assessment.questionnaire?.sent_at is set, which guarantees questionnaire is non-null.
clients/admin-ui/src/features/privacy-assessments/types.ts Added TEAM_INPUT = "team_input" to the AnswerSource enum to match the backend API value for Slack-answered questions.
clients/admin-ui/src/features/privacy-assessments/constants.ts Added TEAM_INPUT mappings to both ANSWER_SOURCE_LABELS and ANSWER_SOURCE_TAG_COLORS records, mapping it to "Team input" label and DEFAULT color — consistent with USER_INPUT and SLACK.
clients/admin-ui/src/features/privacy-assessments/utils.ts File deleted. The getSlackQuestions utility was filtering by AnswerStatus.NEEDS_INPUT (questions pending input) instead of Slack-answered questions — completely wrong logic, correctly removed in favour of API-provided counts.
clients/admin-ui/src/features/privacy-assessments/index.ts Removed the export * from "./utils" line to match the deleted utils.ts file.

Last reviewed commit: 7d94a57

@galvana galvana changed the base branch from celery-worker-hot-reload to main March 5, 2026 04:40
@galvana galvana requested review from lucanovera and removed request for jpople March 5, 2026 06:20
@galvana galvana requested a review from a team as a code owner March 5, 2026 07:26
@galvana galvana requested review from erosselli and removed request for a team March 5, 2026 07:26
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/fides/api/models/privacy_assessment.py (1)

163-163: Add a deterministic tie-breaker to the relationship ordering.

On Line 163, ordering by only group_order and question_order can still produce nondeterministic row order when values tie. Add a final stable key (e.g., id) to keep ordering consistent across queries.

Suggested change
-        order_by="AssessmentQuestion.group_order, AssessmentQuestion.question_order",
+        order_by="AssessmentQuestion.group_order, AssessmentQuestion.question_order, AssessmentQuestion.id",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/fides/api/models/privacy_assessment.py` at line 163, The
relationship/query ordering for AssessmentQuestion currently uses only
"AssessmentQuestion.group_order, AssessmentQuestion.question_order", which can
yield nondeterministic order when those fields tie; update the ordering to
append a stable tie-breaker such as "AssessmentQuestion.id" (with the same sort
direction, e.g., ascending) so the relationship in privacy_assessment.py (the
AssessmentQuestion ordering clause) becomes deterministic across queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/fides/api/models/privacy_assessment.py`:
- Line 163: The relationship/query ordering for AssessmentQuestion currently
uses only "AssessmentQuestion.group_order, AssessmentQuestion.question_order",
which can yield nondeterministic order when those fields tie; update the
ordering to append a stable tie-breaker such as "AssessmentQuestion.id" (with
the same sort direction, e.g., ascending) so the relationship in
privacy_assessment.py (the AssessmentQuestion ordering clause) becomes
deterministic across queries.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bb24cf6e-c365-4811-9f85-4deb93ce5df6

📥 Commits

Reviewing files that changed from the base of the PR and between ae177ac and 95c1f8b.

📒 Files selected for processing (3)
  • clients/admin-ui/src/features/privacy-assessments/constants.ts
  • clients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts
  • src/fides/api/models/privacy_assessment.py

@lucanovera lucanovera enabled auto-merge March 5, 2026 16:31
@lucanovera lucanovera added this pull request to the merge queue Mar 5, 2026
Merged via the queue into main with commit 47179ff Mar 5, 2026
30 of 42 checks passed
@lucanovera lucanovera deleted the misc-assessment-fixes branch March 5, 2026 18:04
mfbrown pushed a commit that referenced this pull request Mar 12, 2026
Co-authored-by: Adrian Galvan <galvana@uci.edu>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Lucano Vera <lucanovera@live.com.ar>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants