Move DSR attachments into main clickme.html

[jpople]

Ticket ENG-1398

Description Of Changes

Moves the files previously in ./attachments.html in DSR access packages to be in the main clickme.html.

Steps to Confirm

1. Have a storage and email provider configured
2. With PATCH /config, set execution.require_manual_request_approval to true
3. Create a new access request
4. Add an attachment to the request with attachment_type of include_with_access_package
5. Approve the request
6. In the access package sent to your email, you should see the attachments with links on the main page of clickme.html and there should be no attachments folder

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

[vercel]

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

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
fides-plus-nightly	Ignored	Preview		Dec 4, 2025 7:24pm
fides-privacy-center	Ignored			Dec 4, 2025 7:24pm

[greptile-apps]

Greptile Overview

Greptile Summary

This PR consolidates DSR access package attachments from a separate attachments/index.html file into the main clickme.html page, simplifying the package structure.

Key Changes:

• Removed separate attachments index page; attachments now display directly in clickme.html
• Refactored _generate_attachment_url_from_index to _generate_attachment_url_from_clickme with updated URL paths
• Changed _add_comprehensive_attachments_index to _get_all_attachments_for_clickme to return attachment data instead of creating a separate file
• Updated all URL generation to use clickme.html as the base instead of attachments/index.html
• Updated extensive test coverage to verify attachments appear in clickme.html

Issues Found:

• Unrelated change in messaging_endpoints.py: Exception handler order was swapped, changing logging behavior for messaging config validation errors. This appears to be an accidental change from a merge conflict and should be reverted.

Confidence Score: 3/5

• PR has a logical error in an unrelated file that should be fixed before merging
• The core attachment consolidation logic is well-implemented with comprehensive tests, but the unrelated exception handling change in messaging_endpoints.py introduces a behavior change that affects error logging and appears to be accidental
• Pay close attention to src/fides/api/api/v1/endpoints/messaging_endpoints.py - the exception handler swap needs to be reverted

Important Files Changed

File Analysis

Filename	Score	Overview
src/fides/api/api/v1/endpoints/messaging_endpoints.py	2/5	Swapped exception handler order - ValidationError now caught before ValueError/KeyError, changing logging behavior unrelated to PR purpose
src/fides/api/service/privacy_request/dsr_package/dsr_report_builder.py	4/5	Refactored to consolidate attachments into clickme.html; renamed methods and updated URL generation logic from attachments/index.html to clickme.html as base

[greptile-apps]

Additional Comments (1)

1. src/fides/api/service/privacy_request/dsr_package/dsr_report_builder.py, line 419-472 (link)

   logic: tests still expect attachments/index.html to exist in the zip file but this PR removes the code that creates it

   multiple tests in test_dsr_attachments.py explicitly check for and read from attachments/index.html:

   • line 22 in EXPECTED_FILES
   • lines 51, 56, 96, 133, 169, 327, 401, 405, 707, 883, 959, 1037

   these tests will fail because the file is no longer generated

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.24%. Comparing base (6310fab) to head (e630d9d).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
.../fides/api/api/v1/endpoints/messaging_endpoints.py	66.66%	0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (92.30%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7033   +/-   ##
=======================================
  Coverage   87.24%   87.24%           
=======================================
  Files         532      532           
  Lines       34900    34896    -4     
  Branches     4036     4037    +1     
=======================================
- Hits        30448    30446    -2     
+ Misses       3570     3567    -3     
- Partials      882      883    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jpople]

@greptileai

[greptile-apps]

Additional Comments (1)

1. src/fides/api/api/v1/endpoints/messaging_endpoints.py, line 326-336 (link)

   logic: exception handler order was swapped, which changes the logging behavior. Before: ValueError/KeyError exceptions were logged with error details. After: ValidationError exceptions are caught first and not logged. This change appears unrelated to moving DSR attachments and may have been introduced during merge conflict resolution. The original order should be restored:

_{6 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[johnewart]

I just have a few comments / questions; I don't see anything blocking so I will approve it but, if they're easy to do and make sense, you might be able to reduce a lot of overhead 😄

[johnewart]

Can we fold this into the other errors and have one except block? Looks like the behavior is identical (except the logger) for ValueError, KeyError and ValidationError

[johnewart]

Is there a reason to not use extra_template_data: Optional[dict[str, Any]]? Taking a splat means if you need a new argument later, it becomes extra work (and then it's also clear what the data is used for).

[johnewart]

These test changes make sense but it also looks like we could reduce the number of nearly-duplicated tests if we had a few parameterized tests - it looks they all do (more-or-less) the exact same thing only looking for different strings. If it's reasonable to do, consider consolidating them to cut down on complexity 😄