Skip to content
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.

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

LJ-366 Add Attachments sql models #5784

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

JadeCara
Copy link
Contributor

@JadeCara JadeCara commented Feb 19, 2025

Closes LJ-366

Description Of Changes

Added Attachments and Attachments references tables with alembic migration to create them. Added tests for basic functionality.

Code Changes

  • Added Attachments table definition to sql_model.py
  • Added AttachmentReference table definition to sql_model.py
  • Added Enum models for Attachment Types and Attachment Reference Types
  • Added to AuditLog Enums for attachment activities.
  • Added unit tests for:
    • create, and delete functionality
    • foriegn key and null constraints

Steps to Confirm

  1. There should be no functionality changes.
  2. Verify tests run as expected
  3. Verify new tables are created through alembic migration.

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
  • Followup issues:
    • Followup issues created (include link)
    • 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

Copy link

vercel bot commented Feb 19, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Feb 20, 2025 7:58pm

Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.21%. Comparing base (18cc518) to head (6f0a65e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5784      +/-   ##
==========================================
+ Coverage   86.91%   87.21%   +0.30%     
==========================================
  Files         394      394              
  Lines       24432    24455      +23     
  Branches     2641     2641              
==========================================
+ Hits        21235    21329      +94     
+ Misses       2628     2549      -79     
- Partials      569      577       +8     

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

@JadeCara JadeCara marked this pull request as ready for review February 20, 2025 13:00
@JadeCara JadeCara added the db-migration This indicates that a change includes a database migration label Feb 20, 2025
@JadeCara JadeCara changed the title WIP LJ-366 Add Attachments sql models LJ-366 Add Attachments sql models Feb 20, 2025
@JadeCara JadeCara marked this pull request as draft February 20, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db-migration This indicates that a change includes a database migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant