Skip to content

Adding from_connection_config helper#7492

Merged
adamsachs merged 1 commit intoasachs/add-systems-connection-endpointfrom
centralized-mapping
Feb 25, 2026
Merged

Adding from_connection_config helper#7492
adamsachs merged 1 commit intoasachs/add-systems-connection-endpointfrom
centralized-mapping

Conversation

@galvana
Copy link
Copy Markdown
Contributor

@galvana galvana commented Feb 25, 2026

Description Of Changes

Suggestion for centralized linked system mapping

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

@galvana galvana requested a review from a team as a code owner February 25, 2026 22:33
@galvana galvana requested review from JadeCara and removed request for a team February 25, 2026 22:33
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 25, 2026

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

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Feb 25, 2026 10:33pm
fides-privacy-center Ignored Ignored Feb 25, 2026 10:33pm

Request Review

@galvana galvana requested review from adamsachs and removed request for JadeCara February 25, 2026 22:33
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

Centralized connection config response mapping by adding from_connection_config() classmethod to the schema, replacing scattered transformation logic in endpoints.

  • Removed _enrich_linked_systems() helper function from connection_endpoints.py in favor of schema-layer method
  • Updated get_connections(), get_connection_detail(), and get_system_connections() to use new classmethod
  • Properly used TYPE_CHECKING to avoid circular imports between schema and model
  • Logic for populating linked_systems now centralized in one location

This refactoring aligns with the architectural principle of delegating business logic to service/schema layers rather than handling it in endpoints (rule 9e0596fc).

Confidence Score: 4/5

  • This PR is safe to merge with low risk - it's a clean refactoring that centralizes transformation logic
  • Clean refactoring that follows architectural best practices and properly handles circular imports. However, there's a subtle behavior change: system_key now uses the ConnectionConfig property which falls back to connection_config.name when no system is linked, whereas the old code set it to None. This aligns with the property's documented behavior but represents a change worth verifying.
  • No files require special attention - the changes are straightforward and well-structured

Important Files Changed

Filename Overview
src/fides/api/schemas/connection_configuration/connection_config.py Added from_connection_config() classmethod to centralize ORM-to-schema mapping with proper TYPE_CHECKING imports to avoid circular dependencies
src/fides/api/api/v1/endpoints/connection_endpoints.py Removed _enrich_linked_systems() helper and updated endpoints to use new from_connection_config() method, simplifying code
src/fides/api/api/v1/endpoints/system.py Updated get_system_connections() to use from_connection_config() instead of inline transformation logic

Last reviewed commit: e392ee4

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.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@adamsachs adamsachs merged commit 6aeaa68 into asachs/add-systems-connection-endpoint Feb 25, 2026
28 of 30 checks passed
@adamsachs adamsachs deleted the centralized-mapping branch February 25, 2026 22:39
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