Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

[SaaS Connector] SaaS fixture cleanup #335

Merged
merged 29 commits into from
Apr 1, 2022
Merged

[SaaS Connector] SaaS fixture cleanup #335

merged 29 commits into from
Apr 1, 2022

Conversation

galvana
Copy link
Collaborator

@galvana galvana commented Mar 27, 2022

Purpose

To separate the saas_example setup used for unit tests from the rest of the SaaS fixtures. The new Mailchimp fixtures also serve as an example for other SaaS integration tests.

Addresses a comment from an earlier ticket.
#278 (comment)

Changes

  • Creating separate fixtures for SaaS examples (used for unit tests) and Mailchimp

Checklist

  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Base automatically changed from 289-saas-connector-headers to main March 28, 2022 18:51
params={},
body=json.dumps(member),
)
connector.create_client().send(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome, thanks @galvana

path: str = self.assign_placeholders(current_request.path, param_values)

headers: Dict[str, Any] = {}
for header in current_request.headers or []:
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a case to say we should update SaaSRequest to have sane defaults as opposed to optional values. That way we'd not need to make these or [] checks for instance. It's not something that ought to happen here. What do you think?

Copy link
Contributor

@seanpreston seanpreston left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes @galvana

@galvana galvana added the run unsafe ci checks Triggers running of unsafe CI checks label Apr 1, 2022
@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels Apr 1, 2022
@galvana galvana merged commit 0f69648 into main Apr 1, 2022
@galvana galvana deleted the test-cleanup branch April 1, 2022 19:41
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
Separating SaaS example and Mailchimp fixtures

Co-authored-by: Adrian Galvan <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants