Skip to content

Assessment detail page UI#7434

Merged
lucanovera merged 105 commits intomainfrom
ENG-2371-UI-Individual-answer-view-and-edit-interface
Feb 26, 2026
Merged

Assessment detail page UI#7434
lucanovera merged 105 commits intomainfrom
ENG-2371-UI-Individual-answer-view-and-edit-interface

Conversation

@lucanovera
Copy link
Copy Markdown
Contributor

@lucanovera lucanovera commented Feb 19, 2026

Ticket ENG-2371

Description Of Changes

Implements a detail page for assessment where you can view answers and update them.

Code Changes

  • Adds assessment detail page and components to build it
  • Exports notificationApi in fidesui
  • Updates types
  • Adds Slack icon

Steps to Confirm

  1. Go to settings and enable alpha flag "Data protection assessments"
  2. Go to assessments page
  3. Click on Resume on an Assessment that shows progress
  4. Try opening answer groups, editing answers
  5. Confirm deletion of assessment works by clicking the trash icon in the header

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

lucanovera and others added 30 commits February 3, 2026 15:56
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…thyca/fides into ENG-2370-ui-assessment-dashboard-with-progress-tracking
Co-authored-by: Cursor <cursoragent@cursor.com>
…' of github.com:ethyca/fides into ENG-2370-ui-assessment-dashboard-with-progress-tracking
@lucanovera lucanovera marked this pull request as ready for review February 25, 2026 02:36
@lucanovera lucanovera requested a review from a team as a code owner February 25, 2026 02:36
@lucanovera lucanovera requested review from speaker-ender and removed request for a team February 25, 2026 02:36
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

This PR implements a detail page for privacy assessments, allowing users to view and edit individual assessment answers. The implementation adds several new React components (AssessmentDetail, QuestionCard, QuestionGroupPanel, EditableTextBlock) with proper state management, error handling, and loading states. The code also exports notificationApi from fidesui for future notification needs.

Key changes:

  • Added detail page at /privacy-assessments/[id] with loading and error states
  • Implemented editable answer interface with optimistic UI updates via RTK Query
  • Added collapse panels for question groups with completion tracking
  • Included proper feature flag checks and breadcrumb navigation
  • Added Slack icon component for team collaboration features (not yet implemented)

Style observations:

  • Several components use div elements where semantic HTML or Ant Design's Flex component would be more appropriate per the project's style guide

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - implements UI-only changes with proper error handling
  • The code is well-structured with proper TypeScript types, error handling, and follows most project patterns. The RTK Query integration correctly invalidates tags for cache updates. Minor style guide deviations regarding div usage don't affect functionality. No security concerns or logical errors found.
  • Files with div elements should consider using Flex components per style guide: QuestionGroupPanel.tsx, AssessmentDetail.tsx, QuestionCard.tsx

Important Files Changed

Filename Overview
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx Main detail component with deletion and completion logic. Uses divs appropriately within Flex components.
clients/admin-ui/src/features/privacy-assessments/EditableTextBlock.tsx Clean editable text component with proper state management and loading states.
clients/admin-ui/src/features/privacy-assessments/QuestionCard.tsx Question display and editing component with proper error handling.
clients/admin-ui/src/features/privacy-assessments/QuestionGroupPanel.tsx Collapse panel header with status display. Uses absolute positioning for tag placement.
clients/admin-ui/src/pages/privacy-assessments/[id].tsx Detail page with proper loading, error states, and feature flag checks.

Last reviewed commit: 7ccf872

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

19 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

return (
<>
<Flex gap="large" align="flex-start" className="min-w-0 flex-1">
<div className="flex-1">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use semantic element or Flex component instead of div

Suggested change
<div className="flex-1">
<Flex className="flex-1" direction="vertical">

Context Used: Rule from dashboard - Avoid using div elements when possible. Use semantic HTML elements or component library alternativ... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor

@speaker-ender speaker-ender left a comment

Choose a reason for hiding this comment

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

Minor nits/questions but otherwise looks good!

Comment on lines +32 to +51
const [isEditing, setIsEditing] = useState(false);
const [editValue, setEditValue] = useState(value);

useEffect(() => {
if (!isEditing) {
setEditValue(value);
}
}, [value, isEditing]);

const handleSave = async () => {
await onSave?.(editValue);
setIsEditing(false);
};

const handleCancel = () => {
setEditValue(value);
setIsEditing(false);
};

if (isEditing) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What problem is this boilerplate solving that ant form cannot do?
This seems very similar to just a form that manages a value + has a callback on submission.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great catch! I've refactored it to use Ant Form to handle the state and it's much cleaner.

Comment on lines +41 to +42
const [notificationApi, notificationContextHolder] =
notification.useNotification();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Higher level question that doesn't need to be solved here:
Is the only reason we are adding all of this to the provider in fidesui because it doesn't pick up styles for messages/modals/notifications?

Copy link
Copy Markdown
Contributor Author

@lucanovera lucanovera Feb 25, 2026

Choose a reason for hiding this comment

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

I think yes, that's how we can get the modals/messages/notification to use our theme without needing to render the {modalContext} manually. I think Jeremy work on this solution.

@lucanovera lucanovera enabled auto-merge February 25, 2026 17:06
@lucanovera lucanovera added this pull request to the merge queue Feb 25, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@lucanovera lucanovera added this pull request to the merge queue Feb 25, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@lucanovera lucanovera force-pushed the ENG-2371-UI-Individual-answer-view-and-edit-interface branch from 60b1b53 to 44a04e6 Compare February 25, 2026 19:04
@lucanovera lucanovera enabled auto-merge February 25, 2026 19:44
@lucanovera lucanovera added this pull request to the merge queue Feb 25, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@lucanovera lucanovera added this pull request to the merge queue Feb 26, 2026
Merged via the queue into main with commit 9db995b Feb 26, 2026
84 of 94 checks passed
@lucanovera lucanovera deleted the ENG-2371-UI-Individual-answer-view-and-edit-interface branch February 26, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants